[NativeAOT] Fix GCDesc computation (#88927)

* Fix GCDesc computation

https://github.com/dotnet/runtime/pull/86877 appears to have introduced a bug in the GCDesc computation.

Consider the following structure layout (we are on 32 bit):

 struct {
     int X1;
     int X2;
     Object Obj;
     int X3;
 }

Crucially, the object reference in this struct is placed at a non-zero offset, which means that in an
array GCDesc, sizeof(X1 + X2) aka 8 will be added to the "base size" of the object. Since we have one
and only series (of GC pointers), it will also be the last. Its "skip" was computed as:

 bitfield.Count (4) - last (3) = 1

Which is clearly incorrect, as we need to skip 3 pointers when considering the shifted array layout:

 <Obj, X3][X1, X2, Obj, X3][X1, X2, Obj, X3]...
     |            |
     [Correct skip]

In effect, for the last series, we must consider the skip to include the delta we have included into
the base size, which code before #86877 did, although wrongly - for MD arrays - as well. This change
restores a fixed version of it.

* Add a test

Verified to fail (hit a GC assert) before and pass after.

* (actually make it compile)

diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
index 9e44b2e..7a259c8 100644 (file)
--- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
+++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
@@ -526,17 +526,17 @@ namespace Internal.Runtime.TypeLoader
             int numSeries = 0;
             int i = 0;
 
-            bool first = true;
+            int first = -1;
             int last = 0;
             short numPtrs = 0;
             while (i < bitfield.Count)
             {
                 if (bitfield[i])
                 {
-                    if (first)
+                    if (first == -1)
                     {
-                        baseOffset += i;
-                        first = false;
+                        first = i;
+                        baseOffset += first;
                     }
                     else if (gcdesc != null)
                     {
@@ -565,7 +565,7 @@ namespace Internal.Runtime.TypeLoader
             {
                 if (numSeries > 0)
                 {
-                    *ptr-- = (short)((bitfield.Count - last) * IntPtr.Size);
+                    *ptr-- = (short)((first + bitfield.Count - last) * IntPtr.Size);
                     *ptr-- = numPtrs;
 
                     *(void**)gcdesc = (void*)-numSeries;

diff --git a/src/tests/nativeaot/SmokeTests/DynamicGenerics/B282745.cs b/src/tests/nativeaot/SmokeTests/DynamicGenerics/B282745.cs
index 7e9ea6a..3b4579b 100644 (file)
--- a/src/tests/nativeaot/SmokeTests/DynamicGenerics/B282745.cs
+++ b/src/tests/nativeaot/SmokeTests/DynamicGenerics/B282745.cs
@@ -6,6 +6,7 @@ using System.Collections;
 using System.Collections.Generic;
 using CoreFXTestLibrary;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Reflection;
@@ -166,6 +167,53 @@ public static class B282745
         }
     }
 
+    unsafe struct StructWithNonGCValuesAtZeroOffset<T>
+    {
+        // Generic structs cannot have explicit layout. We make do with a non-generic one.
+        public StructWithNonGCValuesAtZeroOffsetImpl v;
+    }
+
+    [StructLayout(LayoutKind.Explicit)]
+    struct StructWithNonGCValuesAtZeroOffsetImpl
+    {
+        [FieldOffset(0)]
+        public int i;
+        [FieldOffset(8)]
+        public object o;
+        [FieldOffset(16)]
+        public long l;
+    }
+
+    public class GenericTypeForStructWithNonGCValuesAtZeroOffset<T>
+    {
+        public static void test()
+        {
+            int[] lengths = { 1, 2, 3 };
+            StructWithNonGCValuesAtZeroOffset<T>[,,] array = (StructWithNonGCValuesAtZeroOffset<T>[,,])Array.CreateInstance(typeof(StructWithNonGCValuesAtZeroOffset<T>), lengths);
+
+            array[0, 0, 0].v.o = null;
+            array[0, 0, 0].v.i = GetIntPtrOnHeapAsInt();
+            array[0, 0, 0].v.l = GetIntPtrOnHeapAsLong();
+
+            array[0, 1, 2].v.o = null;
+            array[0, 1, 2].v.i = GetIntPtrOnHeapAsInt();
+            array[0, 1, 2].v.l = GetIntPtrOnHeapAsLong();
+
+            array[0, 1, 1].v.o = null;
+            array[0, 1, 1].v.i = GetIntPtrOnHeapAsInt();
+            array[0, 1, 1].v.l = GetIntPtrOnHeapAsLong();
+
+            GC.Collect();
+
+            GC.KeepAlive(array);
+
+            RuntimeTypeHandle arrayTypeHandle = array.GetType().TypeHandle;
+#if INTERNAL_CONTRACTS
+            Assert.IsTrue(RuntimeAugments.IsDynamicType(arrayTypeHandle));
+#endif
+        }
+    }
+
     [MethodImpl(MethodImplOptions.NoInlining)]
     [TestMethod]
     public static void testMDArrayWithPointerLikeValuesOfKnownStructTypeLargerType()
@@ -175,6 +223,15 @@ public static class B282745
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     [TestMethod]
+    public static void testMDArrayWithPointerLikeValuesOfUnknownStructTypeWithNonGCValuesAtZeroOffset()
+    {
+        Type genType = typeof(GenericTypeForStructWithNonGCValuesAtZeroOffset<>).MakeGenericType(TypeOf.String);
+        MethodInfo m = genType.GetTypeInfo().GetDeclaredMethod("test");
+        m.Invoke(null, new object[] { });
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    [TestMethod]
     public static void testMDArrayWithPointerLikeValuesOfUnknownStructReferenceType()
     {
         Type genType = typeof(GenericType<>).MakeGenericType(TypeOf.String);

diff --git a/src/tests/nativeaot/SmokeTests/DynamicGenerics/DynamicGenerics.main.cs b/src/tests/nativeaot/SmokeTests/DynamicGenerics/DynamicGenerics.main.cs
index ea56825..c1b5906 100644 (file)
--- a/src/tests/nativeaot/SmokeTests/DynamicGenerics/DynamicGenerics.main.cs
+++ b/src/tests/nativeaot/SmokeTests/DynamicGenerics/DynamicGenerics.main.cs
@@ -145,6 +145,7 @@ new CoreFXTestLibrary.Internal.TestInfo("B282745.testIntMDArrayWithPointerLikeVa
 new CoreFXTestLibrary.Internal.TestInfo("B282745.testLongMDArrayWithPointerLikeValues", () => global::B282745.testLongMDArrayWithPointerLikeValues(), null),
 new CoreFXTestLibrary.Internal.TestInfo("B282745.testMDArrayWithPointerLikeValuesOfKnownStructType", () => global::B282745.testMDArrayWithPointerLikeValuesOfKnownStructType(), null),
 new CoreFXTestLibrary.Internal.TestInfo("B282745.testMDArrayWithPointerLikeValuesOfKnownStructTypeLargerType", () => global::B282745.testMDArrayWithPointerLikeValuesOfKnownStructTypeLargerType(), null),
+new CoreFXTestLibrary.Internal.TestInfo("B282745.testMDArrayWithPointerLikeValuesOfUnknownStructTypeWithNonGCValuesAtZeroOffset", () => global::B282745.testMDArrayWithPointerLikeValuesOfUnknownStructTypeWithNonGCValuesAtZeroOffset(), null),
 new CoreFXTestLibrary.Internal.TestInfo("B282745.testMDArrayWithPointerLikeValuesOfUnknownStructReferenceType", () => global::B282745.testMDArrayWithPointerLikeValuesOfUnknownStructReferenceType(), null),
 new CoreFXTestLibrary.Internal.TestInfo("B282745.testMDArrayWithPointerLikeValuesOfUnknownStructPrimitiveType", () => global::B282745.testMDArrayWithPointerLikeValuesOfUnknownStructPrimitiveType(), null),
 new CoreFXTestLibrary.Internal.TestInfo("B282745.testMDArrayWith3Dimensions", () => global::B282745.testMDArrayWith3Dimensions(), null),