diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs new file mode 100644 index 00000000000000..ac11ee086b3d02 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs @@ -0,0 +1,39 @@ +// 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 System.Diagnostics; + +using ILCompiler.DependencyAnalysisFramework; + +using Internal.TypeSystem; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Represents a reflection-visible virtual method that is a target of a delegate. + /// + public class DelegateTargetVirtualMethodNode : DependencyNodeCore + { + private readonly MethodDesc _method; + + public DelegateTargetVirtualMethodNode(MethodDesc method) + { + Debug.Assert(method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method); + _method = method; + } + + protected override string GetName(NodeFactory factory) + { + return "Delegate target method: " + _method.ToString(); + } + + public override IEnumerable GetStaticDependencies(NodeFactory factory) => null; + public override bool InterestingForDynamicDependencyAnalysis => false; + public override bool HasDynamicDependencies => false; + public override bool HasConditionalStaticDependencies => false; + public override bool StaticDependenciesAreComputed => true; + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => null; + public override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory factory) => null; + } +} 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 5d734957ff6e75..5a91083c42725b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -303,6 +303,11 @@ private void CreateNodeCaches() return new TypeGVMEntriesNode(type); }); + _delegateTargetMethods = new NodeCache(method => + { + return new DelegateTargetVirtualMethodNode(method); + }); + _reflectedMethods = new NodeCache(method => { return new ReflectedMethodNode(method); @@ -967,6 +972,12 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type) return _gvmTableEntries.GetOrAdd(type); } + private NodeCache _delegateTargetMethods; + public DelegateTargetVirtualMethodNode DelegateTargetVirtualMethod(MethodDesc method) + { + return _delegateTargetMethods.GetOrAdd(method); + } + private NodeCache _reflectedMethods; public ReflectedMethodNode ReflectedMethod(MethodDesc method) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index c8c49f23116af8..4a42b5e6df6d51 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -541,6 +541,9 @@ public override void GetDependenciesDueToDelegateCreation(ref DependencyList dep { dependencies ??= new DependencyList(); dependencies.Add(factory.ReflectedMethod(target), "Target of a delegate"); + + if (target.IsVirtual) + dependencies.Add(factory.DelegateTargetVirtualMethod(target.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Target of a delegate"); } } @@ -548,29 +551,14 @@ public override void GetDependenciesForOverridingMethod(ref CombinedDependencyLi { 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 ReflectedMethodNode tracks all reflectable methods, - // without keeping information about subtleities like "reflectable delegate". + // If a virtual method slot is a target of a delegate, all implementations become reflection visible + // to support Delegate.GetMethodInfo(). if (!IsReflectionBlocked(decl) && !IsReflectionBlocked(impl)) { dependencies ??= new CombinedDependencyList(); dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( factory.ReflectedMethod(impl.GetCanonMethodTarget(CanonicalFormKind.Specific)), - factory.ReflectedMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), + factory.DelegateTargetVirtualMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Virtual method declaration is reflectable")); } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index e0bce1b1eb82c7..0f263dca61b11b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -393,6 +393,7 @@ + diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/StaticInterfaceMethodDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/StaticInterfaceMethodDataflow.cs index 2f922940a3d739..e73372bca51b11 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/StaticInterfaceMethodDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/StaticInterfaceMethodDataflow.cs @@ -16,6 +16,7 @@ public class StaticInterfaceMethodDataflow public static void Main () { DamOnGenericKeepsMethod.Test (); + DamOnGenericKeepsMethod_UseImplType.Test (); DamOnMethodParameter.Test (); } @@ -33,7 +34,9 @@ public static virtual void VirtualMethod () { } [KeptInterface (typeof (IFoo))] class ImplIFoo : IFoo { - [Kept] + // NativeAOT correctly finds out that the method is not actually used by anything + // and removes it. The only caveat is GetInterfaceMap - see https://github.com/dotnet/runtimelab/issues/861 + [Kept (By = Tool.Trimmer)] public static void VirtualMethod () { } } @@ -54,6 +57,48 @@ public static void Test () } } + [Kept] + static class DamOnGenericKeepsMethod_UseImplType + { + [Kept] + interface IFoo + { + [Kept] + public static virtual void VirtualMethod () { } + } + + [Kept] + [KeptInterface (typeof (IFoo))] + class ImplIFoo : IFoo + { + [Kept] + public static void VirtualMethod () { } + } + + + [Kept] + static void MethodWithDamOnType< + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + T> () + { + } + + [Kept] + class UseStaticInterface where T : IFoo + { + [Kept] + public static void Do () { T.VirtualMethod (); } + } + + [Kept] + public static void Test () + { + MethodWithDamOnType (); + UseStaticInterface.Do (); + } + } + [Kept] static class DamOnMethodParameter { @@ -70,9 +115,13 @@ static virtual void VirtualMethod () { } [KeptInterface (typeof (IFoo))] class ImplIFoo : IFoo { - [Kept] + // NativeAOT correctly finds out that the method is not actually used by anything + // and removes it. The only caveat is GetInterfaceMap - see https://github.com/dotnet/runtimelab/issues/861 + [Kept (By = Tool.Trimmer)] public static void VirtualMethod () { } - [Kept] + // NativeAOT correctly finds out that the method is not actually used by anything + // and removes it. The only caveat is GetInterfaceMap - see https://github.com/dotnet/runtimelab/issues/861 + [Kept (By = Tool.Trimmer)] public static void AbstractMethod () { } }