Differentiate MethodTables that are/aren't visible to reflection

src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs

@@ -202,6 +202,8 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType
         }
 
 #if !READYTORUN
+        public virtual bool IsGenericDefinitionMethodTableReflectionVisible(TypeDesc type) => true;
+
         /// <summary>
         /// Gets a value indicating whether it might be possible to obtain a constructed type data structure for the given type
         /// in this compilation (i.e. is it possible to reference a constructed MethodTable symbol for this).

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs

@@ -101,7 +101,8 @@ public AnalysisBasedMetadataManager(
                 Debug.Assert((GetMetadataCategory(refType.Entity.GetTypeDefinition()) & MetadataCategory.Description)
                     == (GetMetadataCategory(refType.Entity) & MetadataCategory.Description));
 
-                Debug.Assert(!(refType.Entity is MetadataType) || moduleHash.Contains(((MetadataType)refType.Entity).Module));
+                Debug.Assert((refType.Category & MetadataCategory.Description) == 0 ||
+                    !(refType.Entity is MetadataType) || moduleHash.Contains(((MetadataType)refType.Entity).Module));
             }
 
             foreach (var refMethod in reflectableMethods)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs

@@ -266,9 +266,15 @@ public bool NeedsRuntimeLookup(ReadyToRunHelperId lookupKind, object targetOfLoo
 
         public ReadyToRunHelperId GetLdTokenHelperForType(TypeDesc type)
         {
-            return (ConstructedEETypeNode.CreationAllowed(type) && NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type.NormalizeInstantiation()))
-                ? ReadyToRunHelperId.TypeHandle
-                : ReadyToRunHelperId.NecessaryTypeHandle;
+            bool canPotentiallyConstruct = ConstructedEETypeNode.CreationAllowed(type)
+                && NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type.NormalizeInstantiation());
+            if (canPotentiallyConstruct)
+                return ReadyToRunHelperId.TypeHandle;
+
+            if (type.IsGenericDefinition && NodeFactory.DevirtualizationManager.IsGenericDefinitionMethodTableReflectionVisible(type))
+                return ReadyToRunHelperId.TypeHandle;
+
+            return ReadyToRunHelperId.NecessaryTypeHandle;
         }
 
         public static MethodDesc GetConstructorForCreateInstanceIntrinsic(TypeDesc type)

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

@@ -272,7 +272,7 @@ private bool CanonFormTypeMayExist
             }
         }
 
-        public sealed override bool HasConditionalStaticDependencies
+        public override bool HasConditionalStaticDependencies
         {
             get
             {
@@ -323,7 +323,7 @@ public sealed override bool HasConditionalStaticDependencies
             }
         }
 
-        public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
+        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
         {
             List<CombinedDependencyListEntry> result = new List<CombinedDependencyListEntry>();
 
@@ -353,7 +353,7 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
                     "Information about static bases for type with template"));
             }
 
-            if (!_type.IsGenericDefinition && !_type.IsCanonicalSubtype(CanonicalFormKind.Any))
+            if (!_type.IsCanonicalSubtype(CanonicalFormKind.Any))
             {
                 foreach (DefType iface in _type.RuntimeInterfaces)
                 {
@@ -1135,7 +1135,8 @@ protected void OutputGenericInstantiationDetails(NodeFactory factory, ref Object
             {
                 if (!_type.IsTypeDefinition)
                 {
-                    IEETypeNode typeDefNode = factory.NecessaryTypeSymbol(_type.GetTypeDefinition());
+                    IEETypeNode typeDefNode = factory.MaximallyConstructableType(_type) == this ?
+                        factory.ConstructedTypeSymbol(_type.GetTypeDefinition()) : factory.NecessaryTypeSymbol(_type.GetTypeDefinition());
                     if (factory.Target.SupportsRelativePointers)
                         objData.EmitReloc(typeDefNode, RelocType.IMAGE_REL_BASED_RELPTR32);
                     else
@@ -1312,7 +1313,8 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
                 // If the whole program view contains a reference to a preallocated RuntimeType
                 // instance for this type, generate a reference to it.
                 // Otherwise, generate as zero to save size.
-                if (!_type.Type.IsCanonicalSubtype(CanonicalFormKind.Any)
+                if (!relocsOnly
+                    && !_type.Type.IsCanonicalSubtype(CanonicalFormKind.Any)
                     && _type.GetFrozenRuntimeTypeNode(factory) is { Marked: true } runtimeTypeObject)
                 {
                     builder.EmitPointerReloc(runtimeTypeObject);

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

@@ -1,33 +1,29 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Generic;
 using Internal.Runtime;
+using Internal.Text;
 using Internal.TypeSystem;
 
 using Debug = System.Diagnostics.Debug;
 
 namespace ILCompiler.DependencyAnalysis
 {
-    internal sealed class GenericDefinitionEETypeNode : EETypeNode
+    internal abstract class GenericDefinitionEETypeNode : EETypeNode
     {
         public GenericDefinitionEETypeNode(NodeFactory factory, TypeDesc type) : base(factory, type)
         {
             Debug.Assert(type.IsGenericDefinition);
         }
 
-        public override bool ShouldSkipEmittingObjectNode(NodeFactory factory)
-        {
-            return false;
-        }
+        public override bool HasConditionalStaticDependencies => false;
 
-        protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
-        {
-            DependencyList dependencyList = null;
+        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
 
-            // Ask the metadata manager if we have any dependencies due to the presence of the EEType.
-            factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencyList, factory, _type);
-
-            return dependencyList;
+        public override ISymbolNode NodeForLinkage(NodeFactory factory)
+        {
+            return factory.NecessaryTypeSymbol(_type);
         }
 
         protected override ObjectData GetDehydratableData(NodeFactory factory, bool relocsOnly = false)
@@ -63,7 +59,57 @@ protected override ObjectData GetDehydratableData(NodeFactory factory, bool relo
 
             return dataBuilder.ToObjectData();
         }
+    }
+
+    internal sealed class ReflectionInvisibleGenericDefinitionEETypeNode : GenericDefinitionEETypeNode
+    {
+        public ReflectionInvisibleGenericDefinitionEETypeNode(NodeFactory factory, TypeDesc type) : base(factory, type)
+        {
+        }
+
+        public override bool ShouldSkipEmittingObjectNode(NodeFactory factory)
+        {
+            return factory.ConstructedTypeSymbol(_type).Marked;
+        }
+
+        protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
+        {
+            return new DependencyList();
+        }
+
+        public override int ClassCode => -287423988;
+    }
+
+    internal sealed class ReflectionVisibleGenericDefinitionEETypeNode : GenericDefinitionEETypeNode
+    {
+        public ReflectionVisibleGenericDefinitionEETypeNode(NodeFactory factory, TypeDesc type) : base(factory, type)
+        {
+        }
+
+        public override bool ShouldSkipEmittingObjectNode(NodeFactory factory)
+        {
+            return false;
+        }
+
+        protected override FrozenRuntimeTypeNode GetFrozenRuntimeTypeNode(NodeFactory factory)
+        {
+            return factory.SerializedConstructedRuntimeTypeObject(_type);
+        }
+
+        protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler) + " reflection visible";
+
+        protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
+        {
+            var dependencyList = new DependencyList();
+
+            dependencyList.Add(factory.NecessaryTypeSymbol(_type), "Reflection invisible type for a visible type");
+
+            // Ask the metadata manager if we have any dependencies due to the presence of the EEType.
+            factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencyList, factory, _type);
+
+            return dependencyList;
+        }
 
-        public override int ClassCode => -160325006;
+        public override int ClassCode => 983279111;
     }
 }

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

@@ -569,7 +569,8 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
                     //            A necessary EEType might be enough for some cases.
                     //            But we definitely need constructed if this is e.g. layout for a typehandle.
                     //            Measurements show this doesn't amount to much (0.004% - 0.3% size cost vs Necessary).
-                    new DependencyListEntry(context.MaximallyConstructableType(_type), "NativeLayoutEETypeVertexNode containing type signature")
+                    new DependencyListEntry(_type.IsGenericDefinition ? context.NecessaryTypeSymbol(_type) : context.MaximallyConstructableType(_type),
+                    "NativeLayoutEETypeVertexNode containing type signature")
                 };
             }
             public override Vertex WriteVertex(NodeFactory factory)

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

@@ -591,7 +591,7 @@ private IEETypeNode CreateNecessaryTypeNode(TypeDesc type)
             {
                 if (type.IsGenericDefinition)
                 {
-                    return new GenericDefinitionEETypeNode(this, type);
+                    return new ReflectionInvisibleGenericDefinitionEETypeNode(this, type);
                 }
                 else if (type.IsCanonicalDefinitionType(CanonicalFormKind.Any))
                 {
@@ -620,7 +620,11 @@ private IEETypeNode CreateConstructedTypeNode(TypeDesc type)
 
             if (_compilationModuleGroup.ContainsType(type))
             {
-                if (type.IsCanonicalSubtype(CanonicalFormKind.Any))
+                if (type.IsGenericDefinition)
+                {
+                    return new ReflectionVisibleGenericDefinitionEETypeNode(this, type);
+                }
+                else if (type.IsCanonicalSubtype(CanonicalFormKind.Any))
                 {
                     return new CanonicalEETypeNode(this, type);
                 }
@@ -703,7 +707,7 @@ public IEETypeNode ConstructedTypeSymbol(TypeDesc type)
 
         public IEETypeNode MaximallyConstructableType(TypeDesc type)
         {
-            if (ConstructedEETypeNode.CreationAllowed(type))
+            if (ConstructedEETypeNode.CreationAllowed(type) || type.IsGenericDefinition)
                 return ConstructedTypeSymbol(type);
             else
                 return NecessaryTypeSymbol(type);
@@ -1379,7 +1383,7 @@ public SerializedFrozenObjectNode SerializedFrozenObject(MetadataType owningType
 
         public FrozenRuntimeTypeNode SerializedMaximallyConstructableRuntimeTypeObject(TypeDesc type)
         {
-            if (ConstructedEETypeNode.CreationAllowed(type))
+            if (ConstructedEETypeNode.CreationAllowed(type) || type.IsGenericDefinition)
                 return SerializedConstructedRuntimeTypeObject(type);
             return SerializedNecessaryRuntimeTypeObject(type);
         }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs

@@ -436,6 +436,7 @@ public override DictionaryLayoutNode GetLayout(TypeSystemEntity methodOrType)
         private sealed class ScannedDevirtualizationManager : DevirtualizationManager
         {
             private HashSet<TypeDesc> _constructedMethodTables = new HashSet<TypeDesc>();
+            private HashSet<TypeDesc> _reflectionVisibleGenericDefinitionMethodTables = new HashSet<TypeDesc>();
             private HashSet<TypeDesc> _canonConstructedMethodTables = new HashSet<TypeDesc>();
             private HashSet<TypeDesc> _canonConstructedTypes = new HashSet<TypeDesc>();
             private HashSet<TypeDesc> _unsealedTypes = new HashSet<TypeDesc>();
@@ -456,6 +457,11 @@ public ScannedDevirtualizationManager(NodeFactory factory, ImmutableArray<Depend
                         _generatedVirtualMethods.Add(virtualMethodBody.Method);
                     }
 
+                    if (node is ReflectionVisibleGenericDefinitionEETypeNode reflectionVisibleMT)
+                    {
+                        _reflectionVisibleGenericDefinitionMethodTables.Add(reflectionVisibleMT.Type);
+                    }
+
                     TypeDesc type = node switch
                     {
                         ConstructedEETypeNode eetypeNode => eetypeNode.Type,
@@ -736,6 +742,12 @@ public override bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc t
                 return _constructedMethodTables.Contains(type) || _canonConstructedMethodTables.Contains(type);
             }
 
+            public override bool IsGenericDefinitionMethodTableReflectionVisible(TypeDesc type)
+            {
+                Debug.Assert(type.IsGenericDefinition);
+                return _reflectionVisibleGenericDefinitionMethodTables.Contains(type);
+            }
+
             public override TypeDesc[] GetImplementingClasses(TypeDesc type)
             {
                 if (_disqualifiedTypes.Contains(type))

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MultiFileCompilationModuleGroup.cs

@@ -97,7 +97,7 @@ public override bool ShouldProduceFullVTable(TypeDesc type)
 
         public override bool ShouldPromoteToFullType(TypeDesc type)
         {
-            return ShouldProduceFullVTable(type);
+            return ShouldProduceFullVTable(type) || type.IsGenericDefinition;
         }
 
         public override bool PresenceOfEETypeImpliesAllMethodsOnType(TypeDesc type)

src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs

@@ -72,22 +72,16 @@ public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type)
             // information proving that it isn't, give RyuJIT the constructed symbol even
             // though we just need the unconstructed one.
             // https://github.com/dotnet/runtimelab/issues/1128
-            bool canPotentiallyConstruct = ConstructedEETypeNode.CreationAllowed(type)
-                && NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type);
-            if (canPotentiallyConstruct)
-                return _nodeFactory.MaximallyConstructableType(type);
-
-            return _nodeFactory.NecessaryTypeSymbol(type);
+            return GetLdTokenHelperForType(type) == ReadyToRunHelperId.TypeHandle
+                ? _nodeFactory.ConstructedTypeSymbol(type)
+                : _nodeFactory.NecessaryTypeSymbol(type);
         }
 
         public FrozenRuntimeTypeNode NecessaryRuntimeTypeIfPossible(TypeDesc type)
         {
-            bool canPotentiallyConstruct = ConstructedEETypeNode.CreationAllowed(type)
-                && NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type);
-            if (canPotentiallyConstruct)
-                return _nodeFactory.SerializedMaximallyConstructableRuntimeTypeObject(type);
-
-            return _nodeFactory.SerializedNecessaryRuntimeTypeObject(type);
+            return GetLdTokenHelperForType(type) == ReadyToRunHelperId.TypeHandle
+                ? _nodeFactory.SerializedConstructedRuntimeTypeObject(type)
+                : _nodeFactory.SerializedNecessaryRuntimeTypeObject(type);
         }
 
         protected override void CompileInternal(string outputFile, ObjectDumper dumper)

src/tests/nativeaot/SmokeTests/Preinitialization/Preinitialization.cs

@@ -56,6 +56,7 @@ private static int Main()
         TestStaticInterfaceMethod.Run();
         TestConstrainedCall.Run();
         TestTypeHandles.Run();
+        TestPreinitDefinition.Run();
         TestIsValueType.Run();
         TestIndirectLoads.Run();
         TestInitBlock.Run();
@@ -1335,6 +1336,22 @@ public static void Run()
     }
 }
 
+class TestPreinitDefinition
+{
+    class Gen<T>;
+
+    class PreinitHolder
+    {
+        public readonly static Type TheType = typeof(Gen<>);
+    }
+
+    public static void Run()
+    {
+        Assert.IsPreinitialized(typeof(PreinitHolder));
+        Assert.AreEqual("Gen`1", PreinitHolder.TheType.Name);
+    }
+}
+
 class TestIsValueType
 {
     class IsValueTypeTests

src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs

@@ -32,6 +32,7 @@ public static int Run()
         TestUnmodifiableInstanceFieldOptimization.Run();
         TestGetMethodOptimization.Run();
         TestTypeOfCodegenBranchElimination.Run();
+        TestInvisibleGenericsTrimming.Run();
 
         return 100;
     }
@@ -1047,6 +1048,39 @@ static void Consume(object o) { }
         }
     }
 
+    class TestInvisibleGenericsTrimming
+    {
+        class NotPresentType1<T>;
+        class NotPresentType2<T>;
+
+        class PresentType<T>;
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static bool IsNotPresentType1(object o) => o is NotPresentType1<object>;
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static bool IsNotPresentType2(object o) => o.GetType() == typeof(NotPresentType2<object>);
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static bool IsPresentType(object o) => o.GetType() == typeof(PresentType<object>);
+
+        public static void Run()
+        {
+            IsNotPresentType1(new object());
+#if !DEBUG
+            ThrowIfPresent(typeof(TestInvisibleGenericsTrimming), "NotPresentType1`1");
+#endif
+
+            IsNotPresentType2(new object());
+#if !DEBUG
+            ThrowIfPresent(typeof(TestInvisibleGenericsTrimming), "NotPresentType2`1");
+#endif
+
+            IsPresentType(new PresentType<object>());
+            ThrowIfNotPresent(typeof(TestInvisibleGenericsTrimming), "PresentType`1");
+        }
+    }
+
     [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
         Justification = "That's the point")]
     private static Type GetTypeSecretly(Type testType, string typeName) => testType.GetNestedType(typeName, BindingFlags.NonPublic | BindingFlags.Public);