From 12351e11ed7b2cf76b3517130201acaa12e73c09 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Fri, 9 Oct 2026 10:27:30 +0200 Subject: [PATCH 1/2] Keep larger field alignment for Align8 auto-layout structs On FEATURE_64BIT_ALIGNMENT targets, an auto-layout value type that is an Align8 candidate always got an alignment requirement of 8, even when a field required more. On wasm, Int128/UInt128, Vector128 and Decimal128 require 16, so an auto-layout struct containing them was 8-aligned by the runtime but 16-aligned by crossgen2 and by the runtime's own sequential layout. With composite ReadyToRun, crossgen2 bakes the field offsets of such a struct inside a class, so R2R code wrote past the end of the object and corrupted the GC heap. In BenchmarkDotNet runs this surfaced as "Invalid Program: attempted to call a UnmanagedCallersOnly method from managed code". Use max(8, largest field alignment) for Align8 candidates without GC pointers. ARM32 is unaffected since no type there requires more than 8. Re-enable readytorun/fieldlayout on browser CoreCLR and add a case for an auto-layout Int128 struct in a derived class. Fixes #135368 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05ba961f-8f2d-42b8-ac78-8f138880e420 --- src/coreclr/vm/methodtablebuilder.cpp | 4 +- .../fieldlayout/fieldlayouttests.cs | 38 ++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 9f23c2f433fbc6..8ffdf8468c7b44 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -8683,7 +8683,9 @@ VOID MethodTableBuilder::HandleAutoLayout(MethodTable ** pByValueClassCache) #ifdef FEATURE_64BIT_ALIGNMENT if (GetHalfBakedClass()->IsAlign8Candidate()) { - minAlign = 8; + // Align8 raises the requirement to at least 8; it must not lower a larger requirement + // of a field (e.g. 16 for Int128 on wasm), matching sequential layout and crossgen2. + minAlign = containsGCPointers ? 8 : max(8u, (unsigned)largestAlignmentRequirement); } else #endif // FEATURE_64BIT_ALIGNMENT diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs index 949ab4414e27e9..6bf0c1fc959684 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -1,17 +1,17 @@ using System; +using System.Runtime.InteropServices; using System.Runtime.Intrinsics; using Xunit; -using TestLibrary; public class Test { // This test uses the same set of types as the type system unittests use, and attempts to validate that the R2R usage of said types works well. // This is done by touching the various types, and then relying on the verification logic in R2R images to detect failures. [ActiveIssue("These tests are not supposed to be run with mono.", TestRuntimes.Mono)] - [ActiveIssue("https://github.com/dotnet/runtime/pull/131421", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsCoreCLR))] [Fact] public static void TestEntryPoint() { + AutoStructWithInt128InDerivedClassTest.Test(); ContainsGCPointersFieldsTest.Test(); // ExplicitTest.Test(); // Explicit layout is known to not quite match the runtime, and if enabled this set of tests will fail. SequentialTest.Test(); @@ -23,6 +23,40 @@ public static void TestEntryPoint() } } +// An auto layout struct with Int128 fields placed in a class whose base class has fields. The field +// offset compiled into R2R code must match the offset the runtime uses (observed via reflection). +class AutoStructWithInt128InDerivedClassTest +{ + [StructLayout(LayoutKind.Auto)] + public struct Int128Pair + { + public Int128 First; + public Int128 Second; + } + + public class Base + { + public char[] Chars = new char[1]; + } + + public sealed class Derived : Base + { + public Int128Pair Pair; + } + + public static void Test() + { + Derived derived = new Derived(); + derived.Pair.First = 0x1111; + derived.Pair.Second = -0x2222; + + Int128Pair pair = (Int128Pair)typeof(Derived).GetField(nameof(Derived.Pair))!.GetValue(derived)!; + Assert.Equal((Int128)0x1111, pair.First); + Assert.Equal((Int128)(-0x2222), pair.Second); + Assert.Equal(1, derived.Chars.Length); + } +} + class EnumAlignmentTest { static EnumAlignment.LongIntEnumStruct _fld1; From 6493e24a9b1e9c0d5601f618336ecfb281126259 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Fri, 9 Oct 2026 11:47:07 +0200 Subject: [PATCH 2/2] Address review: tighten comment, add Wasm32 crossgen2 layout rows - Shorten the Align8 comment in HandleAutoLayout to the invariant. - Drop an assertion in the fieldlayout test that did not exercise the bug. - Add Wasm32 rows to TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence so crossgen2's 16-byte alignment for auto-layout structs containing Int128/UInt128/Decimal128 is pinned as the layout the runtime must match. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05ba961f-8f2d-42b8-ac78-8f138880e420 --- .../ArchitectureSpecificFieldLayoutTests.cs | 14 ++++++++++++++ src/coreclr/vm/methodtablebuilder.cpp | 3 +-- .../readytorun/fieldlayout/fieldlayouttests.cs | 1 - 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs index 1ffab5e6120f8c..4653c32a042bfa 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs @@ -22,6 +22,8 @@ public class ArchitectureSpecificFieldLayoutTests private ModuleDesc _testModuleARM; private TestTypeSystemContext _contextARM64; private ModuleDesc _testModuleARM64; + private TestTypeSystemContext _contextWasm32; + private ModuleDesc _testModuleWasm32; public ArchitectureSpecificFieldLayoutTests() { @@ -60,6 +62,12 @@ public ArchitectureSpecificFieldLayoutTests() _contextARM64.SetSystemModule(systemModuleARM64); _testModuleARM64 = systemModuleARM64; + + _contextWasm32 = new TestTypeSystemContext(TargetArchitecture.Wasm32, TargetOS.Browser); + var systemModuleWasm32 = _contextWasm32.CreateModuleForSimpleName("CoreTestAssembly"); + _contextWasm32.SetSystemModule(systemModuleWasm32); + + _testModuleWasm32 = systemModuleWasm32; } [Fact] @@ -501,11 +509,13 @@ public void TestAlignmentBehavior_AutoAlignmentRules(string wrapperType, int[] a [InlineData("StructStructByte_Int128StructAuto", "X86", 16, 32)] [InlineData("StructStructByte_Int128StructAuto", "X64Linux", 16, 32)] [InlineData("StructStructByte_Int128StructAuto", "X64Windows", 16, 32)] + [InlineData("StructStructByte_Int128StructAuto", "Wasm32", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "ARM64", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "ARM", 8, 24)] [InlineData("StructStructByte_UInt128StructAuto", "X86", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "X64Linux", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "X64Windows", 16, 32)] + [InlineData("StructStructByte_UInt128StructAuto", "Wasm32", 16, 32)] [InlineData("StructStructByte_Decimal32StructAuto", "ARM64", 4, 8)] [InlineData("StructStructByte_Decimal32StructAuto", "ARM", 4, 8)] [InlineData("StructStructByte_Decimal32StructAuto", "X86", 4, 8)] @@ -521,6 +531,7 @@ public void TestAlignmentBehavior_AutoAlignmentRules(string wrapperType, int[] a [InlineData("StructStructByte_Decimal128StructAuto", "X86", 16, 32)] [InlineData("StructStructByte_Decimal128StructAuto", "X64Linux", 16, 32)] [InlineData("StructStructByte_Decimal128StructAuto", "X64Windows", 16, 32)] + [InlineData("StructStructByte_Decimal128StructAuto", "Wasm32", 16, 32)] // Variation of TestAlignmentBehavior_AutoAlignmentRules above that is able to deal with os specific behavior public void TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence(string wrapperType, string osArch, int alignment, int size) { @@ -545,6 +556,9 @@ public void TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence(string wrap case "X86": testModule = _testModuleX86; break; + case "Wasm32": + testModule = _testModuleWasm32; + break; default: throw new Exception(); } diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 8ffdf8468c7b44..11e48fc70bb370 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -8683,8 +8683,7 @@ VOID MethodTableBuilder::HandleAutoLayout(MethodTable ** pByValueClassCache) #ifdef FEATURE_64BIT_ALIGNMENT if (GetHalfBakedClass()->IsAlign8Candidate()) { - // Align8 raises the requirement to at least 8; it must not lower a larger requirement - // of a field (e.g. 16 for Int128 on wasm), matching sequential layout and crossgen2. + // Align8 is a minimum; keep any larger field alignment requirement. minAlign = containsGCPointers ? 8 : max(8u, (unsigned)largestAlignmentRequirement); } else diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs index 6bf0c1fc959684..fed1f075c3d766 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -53,7 +53,6 @@ public static void Test() Int128Pair pair = (Int128Pair)typeof(Derived).GetField(nameof(Derived.Pair))!.GetValue(derived)!; Assert.Equal((Int128)0x1111, pair.First); Assert.Equal((Int128)(-0x2222), pair.Second); - Assert.Equal(1, derived.Chars.Length); } }