Allow reference types for pinned GC.AllocateArray() (#89293)

* Relaxing constraints of System.GC.AllocateArray<T>.

It's now possible to allocate pinned and default-initialized arrays that contain references.

Fix #48703

* Adding coverage of GC.AllocateArray with ref types.

Added a test that pins a reference type array and resolves references to indices with pointer arithmetic, checking that modifications are visible back in the original array.

Test #48703

* Mono/NativeAOT relaxing of System.GC.AllocateArray<T>.

* Relaxed GC.AllocateUninitializedArray for ref types as well.

This was done by deferring reference types to GC.AllocateArray to avoid potential memory issues with the GC + uninitialized memory. The API only promises to avoid initialization if possible, anyway.
Refactored tests to parametrically exercise these new relaxed constraints.

* Simplifiy conditionals in AllocateUninitializedArray.

Relying on internal implementation zeroing refs if necessary.

* Also simplify path for NativeOAT.

Mono already piggybacks on the AllocateArray path anyway.

* Simplify pinning paths.

All conditional paths in GC.Allocate*Array now handle pinning unconditionally out of the main branch.

* Don't use `var` if type name doesn't exist explicitly on right-hand side

* PR feedback for using terse method tables and JIT intrinsics for GC array allocation that should be slightly faster.

* Changed EmbeddedValueType and added comment.

After a longer discussion, settled on a slightly augmented suggestion that isn't as controversial as the prior one.

* Fixing signature for `RhAllocateNewArray` in NativeAOT to directly use a `MethodTable*`

* Adding explicit structlayout to silence warning for EmbeddedValueType in GCTests, and improved the comment.

---------

Co-authored-by: Jan Kotas <[email protected]>

Author: jthorborg

src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

@@ -741,9 +741,6 @@ internal static void UnregisterMemoryLoadChangeNotification(Action notification)
         /// <typeparam name="T">Specifies the type of the array element.</typeparam>
         /// <param name="length">Specifies the length of the array.</param>
         /// <param name="pinned">Specifies whether the allocated array must be pinned.</param>
-        /// <remarks>
-        /// If pinned is set to true, <typeparamref name="T"/> must not be a reference type or a type that contains object references.
-        /// </remarks>
         [MethodImpl(MethodImplOptions.AggressiveInlining)] // forced to ensure no perf drop for small memory buffers (hot path)
         public static unsafe T[] AllocateUninitializedArray<T>(int length, bool pinned = false) // T[] rather than T?[] to match `new T[length]` behavior
         {
@@ -766,11 +763,8 @@ public static unsafe T[] AllocateUninitializedArray<T>(int length, bool pinned =
 
 #endif
             }
-            else if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
-            {
-                ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(T));
-            }
 
+            // Runtime overrides GC_ALLOC_ZEROING_OPTIONAL if the type contains references, so we don't need to worry about that.
             GC_ALLOC_FLAGS flags = GC_ALLOC_FLAGS.GC_ALLOC_ZEROING_OPTIONAL;
             if (pinned)
                 flags |= GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP;
@@ -784,22 +778,16 @@ public static unsafe T[] AllocateUninitializedArray<T>(int length, bool pinned =
         /// <typeparam name="T">Specifies the type of the array element.</typeparam>
         /// <param name="length">Specifies the length of the array.</param>
         /// <param name="pinned">Specifies whether the allocated array must be pinned.</param>
-        /// <remarks>
-        /// If pinned is set to true, <typeparamref name="T"/> must not be a reference type or a type that contains object references.
-        /// </remarks>
         public static T[] AllocateArray<T>(int length, bool pinned = false) // T[] rather than T?[] to match `new T[length]` behavior
         {
             GC_ALLOC_FLAGS flags = GC_ALLOC_FLAGS.GC_ALLOC_NO_FLAGS;
 
             if (pinned)
             {
-                if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
-                    ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(T));
-
                 flags = GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP;
             }
 
-            return Unsafe.As<T[]>(AllocateNewArray(typeof(T[]).TypeHandle.Value, length, flags));
+            return Unsafe.As<T[]>(AllocateNewArray(RuntimeTypeHandle.ToIntPtr(typeof(T[]).TypeHandle), length, flags));
         }
 
         [MethodImpl(MethodImplOptions.InternalCall)]

src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs

@@ -795,9 +795,6 @@ internal static ulong GetSegmentSize()
         /// <typeparam name="T">Specifies the type of the array element.</typeparam>
         /// <param name="length">Specifies the length of the array.</param>
         /// <param name="pinned">Specifies whether the allocated array must be pinned.</param>
-        /// <remarks>
-        /// If pinned is set to true, <typeparamref name="T"/> must not be a reference type or a type that contains object references.
-        /// </remarks>
         [MethodImpl(MethodImplOptions.AggressiveInlining)] // forced to ensure no perf drop for small memory buffers (hot path)
         public static unsafe T[] AllocateUninitializedArray<T>(int length, bool pinned = false)
         {
@@ -819,10 +816,6 @@ public static unsafe T[] AllocateUninitializedArray<T>(int length, bool pinned =
                 }
 #endif
             }
-            else if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
-            {
-                ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(T));
-            }
 
             // kept outside of the small arrays hot path to have inlining without big size growth
             return AllocateNewUninitializedArray(length, pinned);
@@ -837,7 +830,7 @@ static T[] AllocateNewUninitializedArray(int length, bool pinned)
                     throw new OverflowException();
 
                 T[]? array = null;
-                RuntimeImports.RhAllocateNewArray(EETypePtr.EETypePtrOf<T[]>().RawValue, (uint)length, (uint)flags, Unsafe.AsPointer(ref array));
+                RuntimeImports.RhAllocateNewArray(MethodTable.Of<T[]>(), (uint)length, (uint)flags, Unsafe.AsPointer(ref array));
                 if (array == null)
                     throw new OutOfMemoryException();
 
@@ -851,26 +844,20 @@ static T[] AllocateNewUninitializedArray(int length, bool pinned)
         /// <typeparam name="T">Specifies the type of the array element.</typeparam>
         /// <param name="length">Specifies the length of the array.</param>
         /// <param name="pinned">Specifies whether the allocated array must be pinned.</param>
-        /// <remarks>
-        /// If pinned is set to true, <typeparamref name="T"/> must not be a reference type or a type that contains object references.
-        /// </remarks>
         public static unsafe T[] AllocateArray<T>(int length, bool pinned = false)
         {
             GC_ALLOC_FLAGS flags = GC_ALLOC_FLAGS.GC_ALLOC_NO_FLAGS;
 
             if (pinned)
             {
-                if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
-                    ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(T));
-
                 flags = GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP;
             }
 
             if (length < 0)
                 throw new OverflowException();
 
             T[]? array = null;
-            RuntimeImports.RhAllocateNewArray(EETypePtr.EETypePtrOf<T[]>().RawValue, (uint)length, (uint)flags, Unsafe.AsPointer(ref array));
+            RuntimeImports.RhAllocateNewArray(MethodTable.Of<T[]>(), (uint)length, (uint)flags, Unsafe.AsPointer(ref array));
             if (array == null)
                 throw new OutOfMemoryException();
 

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs

@@ -249,7 +249,7 @@ internal struct GCHeapHardLimitInfo
         internal static extern void RhGetMemoryInfo(ref byte info, GCKind kind);
 
         [LibraryImport(RuntimeLibrary)]
-        internal static unsafe partial void RhAllocateNewArray(IntPtr pArrayEEType, uint numElements, uint flags, void* pResult);
+        internal static unsafe partial void RhAllocateNewArray(MethodTable* pArrayEEType, uint numElements, uint flags, void* pResult);
 
         [LibraryImport(RuntimeLibrary)]
         internal static unsafe partial void RhAllocateNewObject(IntPtr pEEType, uint flags, void* pResult);

src/libraries/System.Runtime/tests/System/GCTests.cs

@@ -1058,11 +1058,60 @@ private static void AllocateArrayTooLarge()
             Assert.Throws<OutOfMemoryException>(() => GC.AllocateUninitializedArray<double>(int.MaxValue, pinned: true));
         }
 
-        [Fact]
-        private static void AllocateArrayRefType()
+        [StructLayout(LayoutKind.Sequential)]
+        struct EmbeddedValueType<T>
+        {
+            // There's a few extra fields here to ensure any reads and writes to Value reasonably are not just offset 0.
+            // This is a low-level smoke check that can give a bit of confidence in pointer arithmetic.
+            // The CLR is permitted to reorder fields and ignore the sequential consistency of value types if they contain
+            // managed references, but it will never hurt the test.
+            object _1;
+            byte _2;
+            public T Value;
+            int _3;
+            string _4;
+        }
+
+        [Theory]
+        [InlineData(false), InlineData(true)]
+        private static void AllocateArray_UninitializedOrNot_WithManagedType_DoesNotThrow(bool pinned)
+        {
+            void TryType<T>()
+            {
+                GC.AllocateUninitializedArray<T>(100, pinned);
+                GC.AllocateArray<T>(100, pinned);
+
+                GC.AllocateArray<EmbeddedValueType<T>>(100, pinned);
+                GC.AllocateUninitializedArray<EmbeddedValueType<T>>(100, pinned);
+            }
+
+            TryType<string>();
+            TryType<object>();
+        }
+
+        [Theory]
+        [InlineData(false), InlineData(true)]
+        private unsafe static void AllocateArrayPinned_ManagedValueType_CanRoundtripThroughPointer(bool uninitialized)
         {
-            GC.AllocateUninitializedArray<string>(100);
-            Assert.Throws<ArgumentException>(() => GC.AllocateUninitializedArray<string>(100, pinned: true));
+            const int length = 100;
+            var rng = new Random(0xAF);
+
+            EmbeddedValueType<string>[] array = uninitialized ? GC.AllocateUninitializedArray<EmbeddedValueType<string>>(length, pinned: true) : GC.AllocateArray<EmbeddedValueType<string>>(length, pinned: true);
+            byte* pointer = (byte*)Unsafe.AsPointer(ref array[0]);
+            var size = Unsafe.SizeOf<EmbeddedValueType<string>>();
+
+            for(int i = 0; i < length; ++i)
+            {
+                int idx = rng.Next(length);
+                ref EmbeddedValueType<string> evt = ref Unsafe.AsRef<EmbeddedValueType<string>>(pointer + size * idx);
+
+                string stringValue = rng.NextSingle().ToString();
+                evt.Value = stringValue;
+
+                Assert.Equal(evt.Value, array[idx].Value);
+            }
+
+            GC.KeepAlive(array);
         }
 
         [Fact]

src/mono/System.Private.CoreLib/src/System/GC.Mono.cs

@@ -281,8 +281,6 @@ public static T[] AllocateUninitializedArray<T>(int length, bool pinned = false)
         public static T[] AllocateArray<T>(int length, bool pinned = false)
         {
             if (pinned) {
-                if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
-                    ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(T));
                 return Unsafe.As<T[]>(AllocPinnedArray(typeof(T[]), length));
             }