Fix type layout with explicitly sized structs and classes (#38632)

- Mimic the alignment and other algorithms present in coreclr - Add tests to crossgen2smoke to cover these scenarios - Update type system unit tests so that they can run in VS - Fix issue #38450
dotnet · Jul 1, 2020 · a5198a7 · a5198a7
1 parent c9c6cd8
commit a5198a7
Show file tree

Hide file tree

Showing 5 changed files with 310 additions and 22 deletions.
diff --git a/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs
@@ -103,6 +103,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp
                     type,
                     type.Context.Target.GetWellKnownTypeSize(type),
                     type.Context.Target.GetWellKnownTypeAlignment(type),
+                    0,
                     out instanceByteSizeAndAlignment
                     );
 
@@ -358,13 +359,8 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata
                 fieldOrdinal++;
             }
 
-            if (type.IsValueType)
-            {
-                instanceSize = LayoutInt.Max(new LayoutInt(layoutMetadata.Size), instanceSize);
-            }
-
             SizeAndAlignment instanceByteSizeAndAlignment;
-            var instanceSizeAndAlignment = ComputeInstanceSize(type, instanceSize, largestAlignmentRequired, out instanceByteSizeAndAlignment);
+            var instanceSizeAndAlignment = ComputeInstanceSize(type, instanceSize, largestAlignmentRequired, layoutMetadata.Size, out instanceByteSizeAndAlignment);
 
             ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
             computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment;
@@ -407,13 +403,8 @@ protected static ComputedInstanceFieldLayout ComputeSequentialFieldLayout(Metada
                 fieldOrdinal++;
             }
 
-            if (type.IsValueType)
-            {
-                cumulativeInstanceFieldPos = LayoutInt.Max(cumulativeInstanceFieldPos, new LayoutInt(layoutMetadata.Size));
-            }
-
             SizeAndAlignment instanceByteSizeAndAlignment;
-            var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, largestAlignmentRequirement, out instanceByteSizeAndAlignment);
+            var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment);
 
             ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
             computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment;
@@ -675,7 +666,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
             }
 
             SizeAndAlignment instanceByteSizeAndAlignment;
-            var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, out instanceByteSizeAndAlignment);
+            var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment);
 
             ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
             computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment;
@@ -788,21 +779,40 @@ private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata lay
                 return layoutMetadata.PackingSize;
         }
 
-        private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, out SizeAndAlignment byteCount)
+        private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount)
         {
             SizeAndAlignment result;
-
-            TargetDetails target = type.Context.Target;
-
+
             // Pad the length of structs to be 1 if they are empty so we have no zero-length structures
             if (type.IsValueType && instanceSize == LayoutInt.Zero)
             {
                 instanceSize = LayoutInt.One;
             }
 
+            TargetDetails target = type.Context.Target;
+
+            if (classLayoutSize != 0)
+            {
+                LayoutInt parentSize;
+                if (type.IsValueType)
+                    parentSize = new LayoutInt(0);
+                else
+                    parentSize = type.BaseType.InstanceByteCountUnaligned;
+
+                LayoutInt specifiedInstanceSize = parentSize + new LayoutInt(classLayoutSize);
+
+                instanceSize = LayoutInt.Max(specifiedInstanceSize, instanceSize);
+            }
+            else
+            {
+                if (type.IsValueType)
+                {
+                    instanceSize = LayoutInt.AlignUp(instanceSize, alignment, target);
+                }
+            }
+
             if (type.IsValueType)
             {
-                instanceSize = LayoutInt.AlignUp(instanceSize, alignment, target);
                 result.Size = instanceSize;
                 result.Alignment = alignment;
             }

diff --git a/.../tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/InstanceFieldLayout.cs b/.../tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/InstanceFieldLayout.cs
@@ -79,7 +79,7 @@ class Class2 : Class1
     }
 
     [StructLayout(LayoutKind.Explicit, Size = 40)]
-    class ExplicitSize : Class1
+    class ExplicitSize
     {
         [FieldOffset(0)]
         int Lol;
@@ -92,6 +92,11 @@ public class ExplicitEmptyClass
     {
     }
 
+    [StructLayout(LayoutKind.Explicit, Size = 0)]
+    public class ExplicitEmptyClassSize0
+    {
+    }
+
     [StructLayout(LayoutKind.Explicit)]
     public struct ExplicitEmptyStruct
     {

diff --git a/...coreclr/src/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/GenericTypeAndMethodTests.cs b/...coreclr/src/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/GenericTypeAndMethodTests.cs
@@ -110,12 +110,11 @@ public void TestFinalize()
         /// Testing lookup up of a method in an instantiated type.
         /// </summary>
         [Fact]
-        [ActiveIssue("")]
         public void TestMethodLookup()
         {
             MetadataType t = _testModule.GetType("GenericTypes", "GenericClass`1").MakeInstantiatedType(_context.GetWellKnownType(WellKnownType.Int32));
 
-            MethodSignature sig = new MethodSignature(MethodSignatureFlags.None, 0, t.Instantiation[0], new TypeDesc[0] { });
+            MethodSignature sig = new MethodSignature(MethodSignatureFlags.None, 0, _context.GetSignatureVariable(0, false), new TypeDesc[0] { });
             MethodDesc fooMethod = t.GetMethod("Foo", sig);
             Assert.NotNull(fooMethod);
         }

diff --git a/.../aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj b/.../aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <AssemblyName>ILCompiler.TypeSystem.ReadyToRun.Tests</AssemblyName>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
     <Configurations>Debug;Release;Checked</Configurations>
     <!-- This seems to be required for supporting assemblies to be copied into the output -->
     <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
@@ -11,6 +11,7 @@
     <!-- By default the subdirectories containing CoreTestAssembly and ILTestAssembly would be 
          included in compilation of this project -->
     <EnableDefaultItems>false</EnableDefaultItems>
+    <Platforms>AnyCPU;x64</Platforms>
   </PropertyGroup>
 
   <ItemGroup>