From a73abd2966d75393a2baf52dfcbcfda4d8ba0f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 21 Feb 2025 12:34:06 +0100 Subject: [PATCH] Differentiate `MethodTable`s that are/aren't visible to reflection Fixes #112029 --- .../Compiler/DevirtualizationManager.cs | 2 + .../ConstructedEETypeNode.cs | 4 +- .../Compiler/DependencyAnalysis/EETypeNode.cs | 6 +- .../GenericDefinitionEETypeNode.cs | 69 +++++++++++++++---- .../DependencyAnalysis/NodeFactory.cs | 8 ++- .../ILCompiler.Compiler/Compiler/ILScanner.cs | 12 ++++ .../Compiler/RyuJitCompilation.cs | 6 ++ 7 files changed, 87 insertions(+), 20 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs b/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs index 5574a9f8aa100f..2728cb113d3a89 100644 --- 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; + /// /// 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/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ConstructedEETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ConstructedEETypeNode.cs index 1905d09e64ef0a..d564640f4d2091 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ConstructedEETypeNode.cs +++ b/src/coreclr/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 index 70d9bd026c68b7..345d4edb6e1b70 100644 --- 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/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs index 22cd3c46497340..8521dbf9fab206 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs +++ b/src/coreclr/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 index d66f9fcc413a65..90f21bc2a992a5 100644 --- 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 index 4d9328f1a34eef..d0773ba4806468 100644 --- 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 _constructedMethodTables = new HashSet(); + private HashSet _reflectionVisibleGenericDefinitionMethodTables = new HashSet(); private HashSet _canonConstructedMethodTables = new HashSet(); private HashSet _canonConstructedTypes = new HashSet(); private HashSet _unsealedTypes = new HashSet(); @@ -456,6 +457,11 @@ public ScannedDevirtualizationManager(NodeFactory factory, ImmutableArray 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 index 99e4a10809d196..2f1305289f1d05 100644 --- 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); }