Allow MakeGenericType/MakeArrayType of arbitrary types (#112986)

When we worked on .NET Native, we didn't have static analysis but at least wanted to have some predictability in how dynamic code fails with AOT. We decided we want `MakeGenericType` & co. to be the thing that fails instead of subsequent operations that try to obtain a type handle (one needs a type handle to do "interesting" reflection operations).

As we refined the model and introduced static analysis, we decided it's fine if trim/AOT unsafe code can get at things that are in "undefined" state [^1] (named types that might not have type handles, MethodInfos that don't have parameter names, etc.). The static analysis warning is always at the problematic operation and what happens next can be anything.

But `MakeGenericType` failing early sometimes got in the way. For example xUnit uses `MakeGenericType` to create a new interface at runtime and check `IsAssignableFrom` with it. We don't actually need a usable type handle for this. We had an opt-out switch that we only enabled for testing to get xUnit working. But that also means we were not testing the shipping configuration whenever xUnit was involved.

This PR makes it so `MakeGeneric`/`MakeArray` can actually succeed even if we don't have the code/data structures for the type. Trying to do "interesting" reflection with it is going to throw the good old MissingMetadata-like exception - the difference is just in the timing.

I'm also changing things to allow obtaining `TypeHandle` on generic types even if they're in the "necessary" form only. This will only bring us to parity with what already happens for non-generic types. We already have blocks in places to make sure such `TypeHandle` cannot be used for much more than casting (e.g. `GetUninitializedObject` has a preexisting block for this).

Cc @dotnet/ilc-contrib 

[^1]: We say we're fine with undefined behavior for trim-unsafe code, but that does not include GC holes.

Author: MichalStrehovsky

eng/testing/tests.singlefile.targets

@@ -50,10 +50,6 @@
 
   <ItemGroup Condition="'$(TestNativeAot)' == 'true'">
     <RdXmlFile Include="$(MSBuildThisFileDirectory)default.rd.xml" />
-
-    <!-- xunit calls MakeGenericType to check if something is IEquatable -->
-    <IlcArg Include="--feature:System.Reflection.IsTypeConstructionEagerlyValidated=false" />
-
     <TrimmerRootAssembly Include="TestUtilities" />
   </ItemGroup>
 

src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Core/Execution/ExecutionEnvironment.cs

@@ -53,7 +53,8 @@ public abstract class ExecutionEnvironment
         //==============================================================================================
         // Invoke and field access support.
         //==============================================================================================
-        public abstract MethodBaseInvoker TryGetMethodInvoker(RuntimeTypeHandle declaringTypeHandle, QMethodDefinition methodHandle, RuntimeTypeHandle[] genericMethodTypeArgumentHandles);
+        public abstract void ValidateGenericMethodConstraints(MethodInfo method);
+        public abstract MethodBaseInvoker TryGetMethodInvokerNoConstraintCheck(RuntimeTypeHandle declaringTypeHandle, QMethodDefinition methodHandle, RuntimeTypeHandle[] genericMethodTypeArgumentHandles);
         public abstract FieldAccessor TryGetFieldAccessor(MetadataReader reader, RuntimeTypeHandle declaringTypeHandle, RuntimeTypeHandle fieldTypeHandle, FieldHandle fieldHandle);
 
         //==============================================================================================
@@ -108,7 +109,7 @@ internal MethodBaseInvoker GetMethodInvoker(RuntimeTypeInfo declaringType, QMeth
             {
                 genericMethodTypeArgumentHandles[i] = genericMethodTypeArguments[i].TypeHandle;
             }
-            MethodBaseInvoker methodInvoker = TryGetMethodInvoker(typeDefinitionHandle, methodHandle, genericMethodTypeArgumentHandles);
+            MethodBaseInvoker methodInvoker = TryGetMethodInvokerNoConstraintCheck(typeDefinitionHandle, methodHandle, genericMethodTypeArgumentHandles);
             if (methodInvoker == null)
                 exception = ReflectionCoreExecution.ExecutionEnvironment.CreateNonInvokabilityException(exceptionPertainant);
             return methodInvoker;

src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs

@@ -26,6 +26,7 @@
 using System.Runtime.InteropServices;
 using System.Threading;
 
+using Internal.Reflection.Core.Execution;
 using Internal.Runtime.CompilerHelpers;
 using Internal.Runtime.CompilerServices;
 
@@ -70,14 +71,33 @@ public static object RawNewObject(RuntimeTypeHandle typeHandle)
             return RuntimeImports.RhNewObject(typeHandle.ToMethodTable());
         }
 
+        internal static void EnsureMethodTableSafeToAllocate(MethodTable* mt)
+        {
+            // We might be dealing with a "necessary" MethodTable (in the ILCompiler terms).
+            // This MethodTable is okay for casting, but must not be allocated on the GC heap.
+            Debug.Assert(MethodTable.Of<object>()->NumVtableSlots > 0);
+            if (mt->NumVtableSlots == 0)
+            {
+                // This is a type without a vtable or GCDesc. We must not allow creating an instance of it
+                throw ReflectionCoreExecution.ExecutionEnvironment.CreateMissingMetadataException(Type.GetTypeFromMethodTable(mt));
+            }
+            // Paranoid check: not-meant-for-GC-heap types should be reliably identifiable by empty vtable.
+            Debug.Assert(!mt->ContainsGCPointers || RuntimeImports.RhGetGCDescSize(mt) != 0);
+        }
+
         //
         // Perform the equivalent of a "newarr" The resulting array is zero-initialized.
         //
         public static Array NewArray(RuntimeTypeHandle typeHandleForArrayType, int count)
         {
             // Don't make the easy mistake of passing in the element MethodTable rather than the "array of element" MethodTable.
             Debug.Assert(typeHandleForArrayType.ToMethodTable()->IsSzArray);
-            return RuntimeImports.RhNewArray(typeHandleForArrayType.ToMethodTable(), count);
+
+            MethodTable* mt = typeHandleForArrayType.ToMethodTable();
+
+            EnsureMethodTableSafeToAllocate(mt);
+
+            return RuntimeImports.RhNewArray(mt, count);
         }
 
         //
@@ -109,7 +129,7 @@ public static unsafe Array NewMultiDimArray(RuntimeTypeHandle typeHandleForArray
                 // We just checked above that all lower bounds are zero. In that case, we should actually allocate
                 // a new SzArray instead.
                 Type elementType = Type.GetTypeFromHandle(new RuntimeTypeHandle(typeHandleForArrayType.ToMethodTable()->RelatedParameterType))!;
-                return RuntimeImports.RhNewArray(elementType.MakeArrayType().TypeHandle.ToMethodTable(), lengths[0]);
+                return NewArray(elementType.MakeArrayType().TypeHandle, lengths[0]);
             }
 
             // Create a local copy of the lengths that cannot be modified by the caller

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

@@ -78,7 +78,7 @@ private static unsafe Array InternalCreate(RuntimeType elementType, int rank, in
 
             if (rank == 1)
             {
-                return RuntimeImports.RhNewArray(elementType.MakeArrayType().TypeHandle.ToMethodTable(), pLengths[0]);
+                return RuntimeAugments.NewArray(elementType.MakeArrayType().TypeHandle, pLengths[0]);
             }
             else
             {
@@ -112,15 +112,15 @@ private static unsafe Array InternalCreateFromArrayType(RuntimeType arrayType, i
                 }
             }
 
-            MethodTable* eeType = arrayType.TypeHandle.ToMethodTable();
             if (rank == 1)
             {
                 // Multidimensional array of rank 1 with 0 lower bounds gets actually allocated
                 // as an SzArray. SzArray is castable to MdArray rank 1.
-                if (!eeType->IsSzArray)
-                    eeType = arrayType.GetElementType().MakeArrayType().TypeHandle.ToMethodTable();
+                RuntimeTypeHandle arrayTypeHandle = arrayType.IsSZArray
+                    ? arrayType.TypeHandle
+                    : arrayType.GetElementType().MakeArrayType().TypeHandle;
 
-                return RuntimeImports.RhNewArray(eeType, pLengths[0]);
+                return RuntimeAugments.NewArray(arrayTypeHandle, pLengths[0]);
             }
             else
             {
@@ -129,6 +129,7 @@ private static unsafe Array InternalCreateFromArrayType(RuntimeType arrayType, i
                 for (int i = 0; i < rank; i++)
                     pImmutableLengths[i] = pLengths[i];
 
+                MethodTable* eeType = arrayType.TypeHandle.ToMethodTable();
                 return NewMultiDimArray(eeType, pImmutableLengths, rank);
             }
         }
@@ -662,6 +663,7 @@ internal static unsafe Array NewMultiDimArray(MethodTable* eeType, int* pLengths
             if (maxArrayDimensionLengthOverflow)
                 throw new OutOfMemoryException(); // "Array dimensions exceeded supported range."
 
+            Debug.Assert(eeType->NumVtableSlots != 0, "Compiler enforces we never have unconstructed MTs for multi-dim arrays since those can be template-constructed anytime");
             Array ret = RuntimeImports.RhNewArray(eeType, (int)totalLength);
 
             ref int bounds = ref ret.GetRawMultiDimArrayBounds();

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/TypeUnifier.cs

@@ -37,42 +37,16 @@ namespace System.Reflection.Runtime.General
 {
     internal static partial class TypeUnifier
     {
-        [FeatureSwitchDefinition("System.Reflection.IsTypeConstructionEagerlyValidated")]
-        // This can be replaced at native compile time using a feature switch.
-        internal static bool IsTypeConstructionEagerlyValidated => true;
-
         public static RuntimeTypeInfo GetArrayType(this RuntimeTypeInfo elementType)
         {
             return RuntimeArrayTypeInfo.GetArrayTypeInfo(elementType, multiDim: false, rank: 1);
         }
 
-        public static RuntimeTypeInfo GetArrayTypeWithTypeHandle(this RuntimeTypeInfo elementType)
-        {
-            return RuntimeArrayTypeInfo.GetArrayTypeInfo(elementType, multiDim: false, rank: 1).WithVerifiedTypeHandle(elementType);
-        }
-
         public static RuntimeTypeInfo GetMultiDimArrayType(this RuntimeTypeInfo elementType, int rank)
         {
             return RuntimeArrayTypeInfo.GetArrayTypeInfo(elementType, multiDim: true, rank: rank);
         }
 
-        public static RuntimeTypeInfo GetMultiDimArrayTypeWithTypeHandle(this RuntimeTypeInfo elementType, int rank)
-        {
-            return RuntimeArrayTypeInfo.GetArrayTypeInfo(elementType, multiDim: true, rank: rank).WithVerifiedTypeHandle(elementType);
-        }
-
-        private static RuntimeArrayTypeInfo WithVerifiedTypeHandle(this RuntimeArrayTypeInfo arrayType, RuntimeTypeInfo elementType)
-        {
-            // We only permit creating parameterized types if the pay-for-play policy specifically allows them *or* if the result
-            // type would be an open type.
-            RuntimeTypeHandle typeHandle = arrayType.InternalTypeHandleIfAvailable;
-            if (IsTypeConstructionEagerlyValidated
-                && typeHandle.IsNull() && !elementType.ContainsGenericParameters)
-                throw ReflectionCoreExecution.ExecutionEnvironment.CreateMissingMetadataException(arrayType.ToType());
-
-            return arrayType;
-        }
-
         public static RuntimeTypeInfo GetByRefType(this RuntimeTypeInfo targetType)
         {
             return RuntimeByRefTypeInfo.GetByRefTypeInfo(targetType);
@@ -88,29 +62,9 @@ public static RuntimeTypeInfo GetConstructedGenericTypeNoConstraintCheck(this Ru
             return RuntimeConstructedGenericTypeInfo.GetRuntimeConstructedGenericTypeInfoNoConstraintCheck(genericTypeDefinition, genericTypeArguments);
         }
 
-        public static RuntimeTypeInfo GetConstructedGenericTypeWithTypeHandle(this RuntimeTypeInfo genericTypeDefinition, RuntimeTypeInfo[] genericTypeArguments)
+        public static RuntimeTypeInfo GetConstructedGenericType(this RuntimeTypeInfo genericTypeDefinition, RuntimeTypeInfo[] genericTypeArguments)
         {
-            return RuntimeConstructedGenericTypeInfo.GetRuntimeConstructedGenericTypeInfo(genericTypeDefinition, genericTypeArguments).WithVerifiedTypeHandle(genericTypeArguments);
-        }
-
-        private static RuntimeConstructedGenericTypeInfo WithVerifiedTypeHandle(this RuntimeConstructedGenericTypeInfo genericType, RuntimeTypeInfo[] genericTypeArguments)
-        {
-            // We only permit creating parameterized types if the pay-for-play policy specifically allows them *or* if the result
-            // type would be an open type.
-            RuntimeTypeHandle typeHandle = genericType.InternalTypeHandleIfAvailable;
-            if (IsTypeConstructionEagerlyValidated && typeHandle.IsNull())
-            {
-                bool atLeastOneOpenType = false;
-                foreach (RuntimeTypeInfo genericTypeArgument in genericTypeArguments)
-                {
-                    if (genericTypeArgument.ContainsGenericParameters)
-                        atLeastOneOpenType = true;
-                }
-                if (!atLeastOneOpenType)
-                    throw ReflectionCoreExecution.ExecutionEnvironment.CreateMissingMetadataException(genericType.ToType());
-            }
-
-            return genericType;
+            return RuntimeConstructedGenericTypeInfo.GetRuntimeConstructedGenericTypeInfo(genericTypeDefinition, genericTypeArguments);
         }
 
         public static RuntimeTypeInfo GetRuntimeTypeInfoForRuntimeTypeHandle(this RuntimeTypeHandle typeHandle)

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/MethodInfos/RuntimeNamedMethodInfo.cs

@@ -144,7 +144,9 @@ public sealed override MethodInfo MakeGenericMethod(params Type[] typeArguments)
             if (typeArguments.Length != GenericTypeParameters.Length)
                 throw new ArgumentException(SR.Format(SR.Argument_NotEnoughGenArguments, typeArguments.Length, GenericTypeParameters.Length));
             RuntimeMethodInfo methodInfo = (RuntimeMethodInfo)RuntimeConstructedGenericMethodInfo.GetRuntimeConstructedGenericMethodInfo(this, genericTypeArguments);
-            MethodBaseInvoker _ = methodInfo.MethodInvoker; // For compatibility with other Make* apis, trigger any missing metadata exceptions now rather than later.
+
+            ReflectionCoreExecution.ExecutionEnvironment.ValidateGenericMethodConstraints(methodInfo);
+
             return methodInfo;
         }
 

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs

@@ -404,14 +404,14 @@ public Type MakeArrayType()
             // Do not implement this as a call to MakeArrayType(1) - they are not interchangeable. MakeArrayType() returns a
             // vector type ("SZArray") while MakeArrayType(1) returns a multidim array of rank 1. These are distinct types
             // in the ECMA model and in CLR Reflection.
-            return this.GetArrayTypeWithTypeHandle().ToType();
+            return this.GetArrayType().ToType();
         }
 
         public Type MakeArrayType(int rank)
         {
             if (rank <= 0)
                 throw new IndexOutOfRangeException();
-            return this.GetMultiDimArrayTypeWithTypeHandle(rank).ToType();
+            return this.GetMultiDimArrayType(rank).ToType();
         }
 
         public Type MakePointerType()
@@ -475,7 +475,7 @@ public Type MakeGenericType(Type[] typeArguments)
                     throw new TypeLoadException(SR.CannotUseByRefLikeTypeInInstantiation);
             }
 
-            return this.GetConstructedGenericTypeWithTypeHandle(runtimeTypeArguments!).ToType();
+            return this.GetConstructedGenericType(runtimeTypeArguments!).ToType();
         }
 
         public Type DeclaringType

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs

@@ -322,14 +322,7 @@ public static unsafe object GetUninitializedObject(
                 throw new NotSupportedException(SR.NotSupported_ByRefLike);
             }
 
-            Debug.Assert(MethodTable.Of<object>()->NumVtableSlots > 0);
-            if (mt->NumVtableSlots == 0)
-            {
-                // This is a type without a vtable or GCDesc. We must not allow creating an instance of it
-                throw ReflectionCoreExecution.ExecutionEnvironment.CreateMissingMetadataException(type);
-            }
-            // Paranoid check: not-meant-for-GC-heap types should be reliably identifiable by empty vtable.
-            Debug.Assert(!mt->ContainsGCPointers || RuntimeImports.RhGetGCDescSize(mt) != 0);
+            RuntimeAugments.EnsureMethodTableSafeToAllocate(mt);
 
             if (mt->IsNullable)
             {
@@ -364,13 +357,7 @@ public static unsafe object GetUninitializedObject(
             if (mt->ElementType == EETypeElementType.Void || mt->IsGenericTypeDefinition || mt->IsByRef || mt->IsPointer || mt->IsFunctionPointer)
                 throw new ArgumentException(SR.Arg_TypeNotSupported);
 
-            if (mt->NumVtableSlots == 0)
-            {
-                // This is a type without a vtable or GCDesc. We must not allow creating an instance of it
-                throw ReflectionCoreExecution.ExecutionEnvironment.CreateMissingMetadataException(Type.GetTypeFromHandle(type));
-            }
-            // Paranoid check: not-meant-for-GC-heap types should be reliably identifiable by empty vtable.
-            Debug.Assert(!mt->ContainsGCPointers || RuntimeImports.RhGetGCDescSize(mt) != 0);
+            RuntimeAugments.EnsureMethodTableSafeToAllocate(mt);
 
             if (!mt->IsValueType)
             {

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs

@@ -218,14 +218,14 @@ public sealed override unsafe bool TryGetConstructedGenericTypeForComponentsNoCo
             return TypeLoaderEnvironment.Instance.TryGetConstructedGenericTypeForComponents(genericTypeDefinitionHandle, genericTypeArgumentHandles, out runtimeTypeHandle);
         }
 
-        public sealed override MethodBaseInvoker TryGetMethodInvoker(RuntimeTypeHandle declaringTypeHandle, QMethodDefinition methodHandle, RuntimeTypeHandle[] genericMethodTypeArgumentHandles)
+        public sealed override void ValidateGenericMethodConstraints(MethodInfo method)
         {
-            MethodBase methodInfo = ExecutionDomain.GetMethod(declaringTypeHandle, methodHandle, genericMethodTypeArgumentHandles);
+            ConstraintValidator.EnsureSatisfiesClassConstraints(method);
+        }
 
-            // Validate constraints first. This is potentially useless work if the method already exists, but it prevents bad
-            // inputs to reach the type loader (we don't have support to e.g. represent pointer types within the type loader)
-            if (genericMethodTypeArgumentHandles != null && genericMethodTypeArgumentHandles.Length > 0)
-                ConstraintValidator.EnsureSatisfiesClassConstraints((MethodInfo)methodInfo);
+        public sealed override MethodBaseInvoker TryGetMethodInvokerNoConstraintCheck(RuntimeTypeHandle declaringTypeHandle, QMethodDefinition methodHandle, RuntimeTypeHandle[] genericMethodTypeArgumentHandles)
+        {
+            MethodBase methodInfo = ExecutionDomain.GetMethod(declaringTypeHandle, methodHandle, genericMethodTypeArgumentHandles);
 
             MethodSignatureComparer methodSignatureComparer = new MethodSignatureComparer(methodHandle);
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ArrayMapNode.cs

@@ -49,15 +49,14 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
             Section hashTableSection = writer.NewSection();
             hashTableSection.Place(typeMapHashTable);
 
-            foreach (var type in factory.MetadataManager.GetTypesWithConstructedEETypes())
+            foreach (var type in factory.MetadataManager.GetTypesWithEETypes())
             {
                 if (!type.IsArray)
                     continue;
 
                 var arrayType = (ArrayType)type;
 
-                // Look at the constructed type symbol. If a constructed type wasn't emitted, then the array map entry isn't valid for use
-                IEETypeNode arrayTypeSymbol = factory.ConstructedTypeSymbol(arrayType);
+                IEETypeNode arrayTypeSymbol = factory.NecessaryTypeSymbol(arrayType);
 
                 Vertex vertex = writer.GetUnsignedConstant(_externalReferences.GetIndex(arrayTypeSymbol));