diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs index de747cff457d3e..1b6677e22ce444 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs @@ -50,6 +50,16 @@ public MethodDesc TargetMethod } } + // The target method might be constrained if this was a "constrained ldftn" IL instruction. + // The real target can be computed after resolving the constraint. + public MethodDesc PossiblyUnresolvedTargetMethod + { + get + { + return _targetMethod; + } + } + private bool TargetMethodIsUnboxingThunk { get 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 906bcc79d2a619..0ec69a4f29f76a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -344,6 +344,8 @@ public sealed override IEnumerable GetConditionalSt { factory.MetadataManager.NoteOverridingMethod(decl, impl); } + + factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, decl, impl); } Debug.Assert( @@ -389,6 +391,8 @@ public sealed override IEnumerable GetConditionalSt } factory.MetadataManager.NoteOverridingMethod(interfaceMethod, implMethod); + + factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, interfaceMethod, implMethod); } else { @@ -414,6 +418,8 @@ public sealed override IEnumerable GetConditionalSt result.Add(new CombinedDependencyListEntry(factory.MethodEntrypoint(defaultIntfMethod), factory.VirtualMethodUse(interfaceMethod), "Interface method")); factory.MetadataManager.NoteOverridingMethod(interfaceMethod, implMethod); + + factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, interfaceMethod, implMethod); } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs index a12f2db56ce2ca..9ec56a62733590 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs @@ -215,6 +215,12 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact dependencies.Add(new DependencyListEntry(dependency, "GenericLookupResultDependency")); } + if (_id == ReadyToRunHelperId.DelegateCtor) + { + MethodDesc targetMethod = ((DelegateCreationInfo)_target).PossiblyUnresolvedTargetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencies, factory, targetMethod); + } + return dependencies; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs index 62d120bf46c477..3787039d53edb1 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs @@ -142,23 +142,27 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact } else if (_id == ReadyToRunHelperId.DelegateCtor) { + DependencyList dependencyList = null; + var info = (DelegateCreationInfo)_target; if (info.NeedsVirtualMethodUseTracking) { MethodDesc targetMethod = info.TargetMethod; - DependencyList dependencyList = new DependencyList(); #if !SUPPORT_JIT factory.MetadataManager.GetDependenciesDueToVirtualMethodReflectability(ref dependencyList, factory, targetMethod); if (!factory.VTable(info.TargetMethod.OwningType).HasFixedSlots) { + dependencyList ??= new DependencyList(); dependencyList.Add(factory.VirtualMethodUse(info.TargetMethod), "ReadyToRun Delegate to virtual method"); } #endif - - return dependencyList; } + + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencyList, factory, info.PossiblyUnresolvedTargetMethod); + + return dependencyList; } return null; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index 3906b11d7ca090..5ebf7f1ed3a247 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -430,6 +430,22 @@ public virtual void GetDependenciesDueToLdToken(ref DependencyList dependencies, // RuntimeFieldHandle data structure. } + /// + /// This method is an extension point that can provide additional metadata-based dependencies to delegate targets. + /// + public virtual void GetDependenciesDueToDelegateCreation(ref DependencyList dependencies, NodeFactory factory, MethodDesc target) + { + // MetadataManagers can override this to provide additional dependencies caused by the construction + // of a delegate to a method. + } + + /// + /// This method is an extension point that can provide additional dependencies for overriden methods on constructed types. + /// + public virtual void GetDependenciesForOverridingMethod(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc decl, MethodDesc impl) + { + } + /// /// This method is an extension point that can provide additional metadata-based dependencies to generated method bodies. /// diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index ef615ac558bd18..4d1f784b224528 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -456,6 +456,46 @@ public override void GetDependenciesDueToLdToken(ref DependencyList dependencies dependencies.Add(factory.ReflectableMethod(method), "LDTOKEN method"); } + public override void GetDependenciesDueToDelegateCreation(ref DependencyList dependencies, NodeFactory factory, MethodDesc target) + { + if (!IsReflectionBlocked(target)) + { + dependencies = dependencies ?? new DependencyList(); + dependencies.Add(factory.ReflectableMethod(target), "Target of a delegate"); + } + } + + public override void GetDependenciesForOverridingMethod(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc decl, MethodDesc impl) + { + Debug.Assert(decl.IsVirtual && MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(decl) == decl); + + // If a virtual method slot is reflection visible, all implementations become reflection visible. + // + // We could technically come up with a weaker position on this because the code below just needs to + // to ensure that delegates to virtual methods can have their GetMethodInfo() called. + // Delegate construction introduces a ReflectableMethod for the slot defining method; it doesn't need to. + // We could have a specialized node type to track that specific thing and introduce a conditional dependency + // on that. + // + // class Base { abstract Boo(); } + // class Derived1 : Base { override Boo() { } } + // class Derived2 : Base { override Boo() { } } + // + // typeof(Derived2).GetMethods(...) + // + // In the above case, we don't really need Derived1.Boo to become reflection visible + // but the below code will do that because ReflectableMethodNode tracks all reflectable methods, + // without keeping information about subtleities like "reflectable delegate". + if (!IsReflectionBlocked(decl) && !IsReflectionBlocked(impl)) + { + dependencies ??= new CombinedDependencyList(); + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + factory.ReflectableMethod(impl.GetCanonMethodTarget(CanonicalFormKind.Specific)), + factory.ReflectableMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), + "Virtual method declaration is reflectable")); + } + } + protected override void GetDependenciesDueToMethodCodePresenceInternal(ref DependencyList dependencies, NodeFactory factory, MethodDesc method, MethodIL methodIL) { bool scanReflection = (_generationOptions & UsageBasedMetadataGenerationOptions.ReflectionILScanning) != 0; diff --git a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs index ab19c6f35d5102..53a2100b287960 100644 --- a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs +++ b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs @@ -16,7 +16,7 @@ [assembly: TestAssembly] [module: TestModule] -internal class ReflectionTest +internal static class ReflectionTest { private static int Main() { @@ -27,6 +27,8 @@ private static int Main() // // Tests for dependency graph in the compiler // + TestSimpleDelegateTargets.Run(); + TestVirtualDelegateTargets.Run(); TestRunClassConstructor.Run(); #if !OPTIMIZED_MODE_WITHOUT_SCANNER TestContainment.Run(); @@ -1656,6 +1658,104 @@ public static void Run() } } + class TestSimpleDelegateTargets + { + class TestClass + { + public static void StaticMethod() { } + public void InstanceMethod() { } + public static void SimplyCalledMethod() { } + } + + class TestClass + { + public static void StaticMethod() { } + } + + static void CheckGeneric() + { + Action staticMethod = TestClass.StaticMethod; + if (staticMethod.GetMethodInfo().Name != nameof(TestClass.StaticMethod)) + throw new Exception(); + } + + public static void Run() + { + Console.WriteLine("Testing delegate targets are reflectable..."); + + Action staticMethod = TestClass.StaticMethod; + if (staticMethod.GetMethodInfo().Name != nameof(TestClass.StaticMethod)) + throw new Exception(); + + Action instanceMethod = new TestClass().InstanceMethod; + if (instanceMethod.GetMethodInfo().Name != nameof(TestClass.InstanceMethod)) + throw new Exception(); + + TestClass.SimplyCalledMethod(); + + Assert.Equal( +#if REFLECTION_FROM_USAGE + 3, +#else + 2, +#endif + typeof(TestClass).CountMethods()); + + CheckGeneric(); + } + } + + class TestVirtualDelegateTargets + { + abstract class Base + { + public virtual void VirtualMethod() { } + public abstract void AbstractMethod(); + } + + class Derived : Base, IBar + { + public override void AbstractMethod() { } + public override void VirtualMethod() { } + void IFoo.InterfaceMethod() { } + } + + interface IFoo + { + void InterfaceMethod(); + void DefaultInterfaceMethod() { } + } + + interface IBar : IFoo + { + void IFoo.DefaultInterfaceMethod() { } + } + + static Base s_baseInstance = new Derived(); + static IFoo s_ifooInstance = new Derived(); + + public static void Run() + { + Console.WriteLine("Testing virtual delegate targets are reflectable..."); + + Action abstractMethod = s_baseInstance.AbstractMethod; + if (abstractMethod.GetMethodInfo().Name != nameof(Derived.AbstractMethod)) + throw new Exception(); + + Action virtualMethod = s_baseInstance.VirtualMethod; + if (virtualMethod.GetMethodInfo().Name != nameof(Derived.VirtualMethod)) + throw new Exception(); + + Action interfaceMethod = s_ifooInstance.InterfaceMethod; + if (!interfaceMethod.GetMethodInfo().Name.EndsWith("IFoo.InterfaceMethod")) + throw new Exception(); + + Action defaultMethod = s_ifooInstance.DefaultInterfaceMethod; + if (!defaultMethod.GetMethodInfo().Name.EndsWith("IFoo.DefaultInterfaceMethod")) + throw new Exception(); + } + } + class TestRunClassConstructor { static class TypeWithNoStaticFieldsButACCtor @@ -1709,6 +1809,11 @@ private static bool HasTypeHandle(Type type) return true; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "That's the point")] + public static int CountMethods(this Type t) + => t.GetMethods(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly).Length; + class Assert { public static void Equal(T expected, T actual)