Differentiate MethodTables that are/aren't visible to reflection

Fixes dotnet#112029
MichalStrehovsky · Feb 24, 2025 · a73abd2 · a73abd2
1 parent 69facbc
commit a73abd2
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 20 deletions.
diff --git a/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs b/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).

diff --git a/...oreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ConstructedEETypeNode.cs b/...oreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ConstructedEETypeNode.cs
@@ -121,8 +121,8 @@ public static bool CreationAllowed(TypeDesc type)
 
                 default:
                     // Generic definition EETypes can't be allocated
-                    if (type.IsGenericDefinition)
-                        return false;
+                    //if (type.IsGenericDefinition)
+                    //    return false;
 
                     // Full MethodTable of System.Canon should never be used.
                     if (type.IsCanonicalDefinitionType(CanonicalFormKind.Any))

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs
@@ -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);

diff --git a/.../tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs b/.../tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs
@@ -2,32 +2,23 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 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)
+        public override ISymbolNode NodeForLinkage(NodeFactory factory)
         {
-            return false;
-        }
-
-        protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
-        {
-            DependencyList dependencyList = 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;
+            return factory.NecessaryTypeSymbol(_type);
         }
 
         protected override ObjectData GetDehydratableData(NodeFactory factory, bool relocsOnly = false)
@@ -63,7 +54,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;
     }
 }
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/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);
                 }

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/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))

diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs
@@ -77,6 +77,9 @@ public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type)
             if (canPotentiallyConstruct)
                 return _nodeFactory.MaximallyConstructableType(type);
 
+            if (type.IsGenericDefinition && NodeFactory.DevirtualizationManager.IsGenericDefinitionMethodTableReflectionVisible(type))
+                return _nodeFactory.ConstructedTypeSymbol(type);
+
             return _nodeFactory.NecessaryTypeSymbol(type);
         }
 
@@ -87,6 +90,9 @@ public FrozenRuntimeTypeNode NecessaryRuntimeTypeIfPossible(TypeDesc type)
             if (canPotentiallyConstruct)
                 return _nodeFactory.SerializedMaximallyConstructableRuntimeTypeObject(type);
 
+            if (type.IsGenericDefinition && NodeFactory.DevirtualizationManager.IsGenericDefinitionMethodTableReflectionVisible(type))
+                return _nodeFactory.SerializedConstructedRuntimeTypeObject(type);
+
             return _nodeFactory.SerializedNecessaryRuntimeTypeObject(type);
         }