From 22688e4afd2411815fa8c0d11e12b89b9f8fa2e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 19 Dec 2023 10:47:09 +0100 Subject: [PATCH 1/3] Precise reflection-visible delegate target analysis Before this change, the compiler considered all methods that are target of a delegate reflection-visible. This is because one can just call `Delegate.Method` on a delegate instance to obtain a `MethodBase` of the target. This is rather annoying because most delegates are not actually used with reflection. In this PR I'm adding analysis of places that use `Delegate.Method` to reflect on certain delegate type. This builds on top of the existing dataflow analysis within the compiler - we can often see the exact delegate type that was reflected on. When we see that, we make all targets of that specific type reflection-visible. The advantage is that if nobody ever calls `Delegate.Method`, no delegates are made reflection visible. If this is used with a certain known delegate type, only the methods pointed to by that specific delegate type are made reflection visible. And if this is used with something typed at `Delegate` or `MulticastDelegate`, we fall back to the old behavior that just makes everything reflection visible. I was hoping this would help ASP.NET but ASP.NET actually uses something typed as `Delegate` to obtain the `MethodInfo` and there's more code somewhere in the `Task` infrastructure (under `EventSourceSupport`) that also does it. So it unfortunately can't help there. --- .../Dataflow/ReflectionMethodBodyScanner.cs | 40 ++++++++++ .../Compiler/DelegateCreationInfo.cs | 10 ++- .../DependencyAnalysis/FrozenObjectNode.cs | 6 -- .../DependencyAnalysis/NodeFactory.cs | 15 ++++ .../ReadyToRunGenericHelperNode.cs | 13 ++-- .../ReadyToRunHelperNode.cs | 12 ++- .../ReflectedDelegateNode.cs | 49 ++++++++++++ .../SerializedFrozenObjectNode.cs | 11 ++- .../Compiler/MetadataManager.cs | 2 +- .../Compiler/TypePreinit.cs | 17 +++-- .../Compiler/UsageBasedMetadataManager.cs | 48 +++++++++++- .../ILCompiler.Compiler.csproj | 1 + .../TrimmingBehaviors/DeadCodeElimination.cs | 74 +++++++++++++++++++ .../ILLink.Shared/TrimAnalysis/IntrinsicId.cs | 10 ++- .../ILLink.Shared/TrimAnalysis/Intrinsics.cs | 11 +++ 15 files changed, 291 insertions(+), 28 deletions(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedDelegateNode.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index 4ac268a3e6263..d3e41cd36eaf9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -514,6 +514,46 @@ public static bool HandleCall( } break; + // + // System.Delegate + // + // get_Method () + // + // System.Reflection.RuntimeReflectionExtensions + // + // GetMethodInfo (System.Delegate) + // + case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: + case IntrinsicId.Delegate_get_Method: + { + // Find the parameter: first is an instance method, second is an extension method. + MultiValue param = intrinsicId == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo + ? argumentValues[0] : instanceValue; + + // If this is Delegate.Method accessed from RuntimeReflectionExtensions.GetMethodInfo, ignore + // because we handle the callsites to that one here as well. + if (Intrinsics.GetIntrinsicIdForMethod(callingMethodDefinition) == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo) + break; + + foreach (var valueNode in param.AsEnumerable()) + { + TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; + if (staticType is null || !staticType.IsDelegate) + { + // The static type is unknown or something useless like Delegate or MulticastDelegate. + reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(null), "Delegate.Method access on unknown delegate type"); + } + else + { + if (staticType.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) + reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(staticType.GetTypeDefinition()), "Delegate.Method access (on inexact type)"); + else + reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(staticType.ConvertToCanonForm(CanonicalFormKind.Specific)), "Delegate.Method access"); + } + } + } + break; + // // System.Object // diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs index 1b6677e22ce44..981d7a183b14b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs @@ -170,10 +170,16 @@ public IMethodNode Thunk get; } - private DelegateCreationInfo(IMethodNode constructor, MethodDesc targetMethod, TypeDesc constrainedType, TargetKind targetKind, IMethodNode thunk = null) + public TypeDesc DelegateType + { + get; + } + + private DelegateCreationInfo(TypeDesc delegateType, IMethodNode constructor, MethodDesc targetMethod, TypeDesc constrainedType, TargetKind targetKind, IMethodNode thunk = null) { Debug.Assert(targetKind != TargetKind.VTableLookup || MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(targetMethod) == targetMethod); + DelegateType = delegateType; Constructor = constructor; _targetMethod = targetMethod; _constrainedType = constrainedType; @@ -230,6 +236,7 @@ public static DelegateCreationInfo Create(TypeDesc delegateType, MethodDesc targ invokeThunk = context.GetMethodForInstantiatedType(invokeThunk, instantiatedDelegateType); return new DelegateCreationInfo( + delegateType, factory.MethodEntrypoint(initMethod), targetMethod, constrainedType, @@ -289,6 +296,7 @@ public static DelegateCreationInfo Create(TypeDesc delegateType, MethodDesc targ Debug.Assert(constrainedType == null); return new DelegateCreationInfo( + delegateType, factory.MethodEntrypoint(systemDelegate.GetKnownMethod(initializeMethodName, null)), targetMethod, constrainedType, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FrozenObjectNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FrozenObjectNode.cs index 27a8dd31db8eb..2d1bcefd4fd56 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FrozenObjectNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FrozenObjectNode.cs @@ -60,15 +60,9 @@ public sealed override IEnumerable GetStaticDependencies(No } } - GetNonRelocationDependencies(ref dependencies, factory); - return dependencies; } - public virtual void GetNonRelocationDependencies(ref DependencyList dependencies, NodeFactory factory) - { - } - public abstract void EncodeContents(ref ObjectDataBuilder dataBuilder, NodeFactory factory, bool relocsOnly); } } 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 10126a6525375..d2e82bc46813e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -304,6 +304,11 @@ private void CreateNodeCaches() return new DelegateTargetVirtualMethodNode(method); }); + _reflectedDelegates = new NodeCache(type => + { + return new ReflectedDelegateNode(type); + }); + _reflectedMethods = new NodeCache(method => { return new ReflectedMethodNode(method); @@ -996,6 +1001,16 @@ public DelegateTargetVirtualMethodNode DelegateTargetVirtualMethod(MethodDesc me return _delegateTargetMethods.GetOrAdd(method); } + private ReflectedDelegateNode _unknownReflectedDelegate = new ReflectedDelegateNode(null); + private NodeCache _reflectedDelegates; + public ReflectedDelegateNode ReflectedDelegate(TypeDesc type) + { + if (type == null) + return _unknownReflectedDelegate; + + return _reflectedDelegates.GetOrAdd(type); + } + private NodeCache _reflectedMethods; public ReflectedMethodNode ReflectedMethod(MethodDesc method) { 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 0cad5e99a02a3..10e5d8d162e7b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs @@ -219,12 +219,6 @@ 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; } @@ -263,6 +257,13 @@ public override IEnumerable GetConditionalStaticDep } } + if (_id == ReadyToRunHelperId.DelegateCtor) + { + var delegateCreationInfo = (DelegateCreationInfo)_target; + MethodDesc targetMethod = delegateCreationInfo.PossiblyUnresolvedTargetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref conditionalDependencies, factory, delegateCreationInfo.DelegateType, targetMethod); + } + return conditionalDependencies; } 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 36cc3c5ec1eda..f3a4410dcbab6 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs @@ -160,14 +160,22 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact #endif } - factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencyList, factory, info.PossiblyUnresolvedTargetMethod); - return dependencyList; } return null; } + public override bool HasConditionalStaticDependencies => _id == ReadyToRunHelperId.DelegateCtor; + + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) + { + List dependencyList = new List(); + var info = (DelegateCreationInfo)_target; + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencyList, factory, info.DelegateType, info.PossiblyUnresolvedTargetMethod); + return dependencyList; + } + IEnumerable INodeWithDebugInfo.GetNativeSequencePoints() { if (_id == ReadyToRunHelperId.VirtualCall) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedDelegateNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedDelegateNode.cs new file mode 100644 index 0000000000000..925642cd94384 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedDelegateNode.cs @@ -0,0 +1,49 @@ +// 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 ILCompiler.DependencyAnalysisFramework; + +using Internal.TypeSystem; + +using Debug = System.Diagnostics.Debug; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Represents a delegate type that was reflected on using . + /// + public class ReflectedDelegateNode : DependencyNodeCore + { + private readonly TypeDesc _delegateType; + + public ReflectedDelegateNode(TypeDesc delegateType) + { + Debug.Assert(delegateType is null || delegateType.IsDelegate); + + // We accept 3 kinds of types: + // * Null: Delegate.get_Method was used on an unknown type. All delegate targets are reflection-visible. + // * A type definition: An unknown instantiation of the delegate was reflected on. Typically caused by dataflow analysis analyzing uninstantiated code. + // * Canonical form: Some canonical form was reflected on. + Debug.Assert(delegateType is null + || delegateType.IsGenericDefinition + || delegateType.ConvertToCanonForm(CanonicalFormKind.Specific) == delegateType); + + _delegateType = delegateType; + } + + protected override string GetName(NodeFactory factory) + { + return "Reflectable delegate type: " + _delegateType?.ToString() ?? "All delegates"; + } + + 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/SerializedFrozenObjectNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SerializedFrozenObjectNode.cs index c1d71f6572ece..725ea9db9e3e5 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SerializedFrozenObjectNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SerializedFrozenObjectNode.cs @@ -2,10 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using Internal.Text; using Internal.TypeSystem; +using CombinedDependencyList = System.Collections.Generic.List.CombinedDependencyListEntry>; + namespace ILCompiler.DependencyAnalysis { /// @@ -53,9 +56,13 @@ public override void EncodeContents(ref ObjectDataBuilder dataBuilder, NodeFacto protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler); - public override void GetNonRelocationDependencies(ref DependencyList dependencies, NodeFactory factory) + public override bool HasConditionalStaticDependencies => _data.HasConditionalDependencies; + + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) { - _data.GetNonRelocationDependencies(ref dependencies, factory); + CombinedDependencyList result = null; + _data.GetConditionalDependencies(ref result, factory); + return result; } public override int ClassCode => 1789429316; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index fdceb022bd3d7..5058524c8cecb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -548,7 +548,7 @@ public virtual void GetDependenciesDueToLdToken(ref DependencyList dependencies, /// /// 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) + public virtual void GetDependenciesDueToDelegateCreation(ref CombinedDependencyList dependencies, NodeFactory factory, TypeDesc delegateType, MethodDesc target) { // MetadataManagers can override this to provide additional dependencies caused by the construction // of a delegate to a method. diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs index 6f5fa941b6a1c..6306894147eed 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs @@ -11,7 +11,7 @@ using Internal.IL; using Internal.TypeSystem; -using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore.DependencyList; +using CombinedDependencyList = System.Collections.Generic.List.CombinedDependencyListEntry>; namespace ILCompiler { @@ -2144,7 +2144,8 @@ public interface ISerializableReference : ISerializableValue { TypeDesc Type { get; } void WriteContent(ref ObjectDataBuilder builder, ISymbolNode thisNode, NodeFactory factory); - void GetNonRelocationDependencies(ref DependencyList dependencies, NodeFactory factory); + bool HasConditionalDependencies { get; } + void GetConditionalDependencies(ref CombinedDependencyList dependencies, NodeFactory factory); bool IsKnownImmutable { get; } int ArrayLength { get; } } @@ -2694,7 +2695,9 @@ public override bool GetRawData(NodeFactory factory, out object data) return false; } - public virtual void GetNonRelocationDependencies(ref DependencyList dependencies, NodeFactory factory) + public virtual bool HasConditionalDependencies => false; + + public virtual void GetConditionalDependencies(ref CombinedDependencyList dependencies, NodeFactory factory) { } } @@ -2719,12 +2722,16 @@ private DelegateCreationInfo GetDelegateCreationInfo(NodeFactory factory) factory, followVirtualDispatch: false); - public override void GetNonRelocationDependencies(ref DependencyList dependencies, NodeFactory factory) + public override bool HasConditionalDependencies => true; + + public override void GetConditionalDependencies(ref CombinedDependencyList dependencies, NodeFactory factory) { + dependencies ??= new CombinedDependencyList(); + DelegateCreationInfo creationInfo = GetDelegateCreationInfo(factory); MethodDesc targetMethod = creationInfo.PossiblyUnresolvedTargetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); - factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencies, factory, targetMethod); + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencies, factory, creationInfo.DelegateType, targetMethod); } public void WriteContent(ref ObjectDataBuilder builder, ISymbolNode thisNode, NodeFactory factory) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index 7715ce99a7373..a20328516d2f5 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -552,15 +552,55 @@ public override void GetDependenciesDueToLdToken(ref DependencyList dependencies } } - public override void GetDependenciesDueToDelegateCreation(ref DependencyList dependencies, NodeFactory factory, MethodDesc target) + public override void GetDependenciesDueToDelegateCreation(ref CombinedDependencyList dependencies, NodeFactory factory, TypeDesc delegateType, MethodDesc target) { if (!IsReflectionBlocked(target)) { - dependencies ??= new DependencyList(); - dependencies.Add(factory.ReflectedMethod(target.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Target of a delegate"); + dependencies ??= new CombinedDependencyList(); + + ReflectedMethodNode reflectedMethod = factory.ReflectedMethod(target.GetCanonMethodTarget(CanonicalFormKind.Specific)); + + TypeDesc canonDelegateType = delegateType.ConvertToCanonForm(CanonicalFormKind.Specific); + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + reflectedMethod, + factory.ReflectedDelegate(canonDelegateType), + "Target of delegate could be reflection-visible")); + + if (canonDelegateType.HasInstantiation) + { + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + reflectedMethod, + factory.ReflectedDelegate(canonDelegateType.GetTypeDefinition()), + "Target of delegate could be reflection-visible")); + } + + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + reflectedMethod, + factory.ReflectedDelegate(null), + "Target of delegate could be reflection-visible")); if (target.IsVirtual) - dependencies.Add(factory.DelegateTargetVirtualMethod(target.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Target of a delegate"); + { + DelegateTargetVirtualMethodNode targetVirtualMethod = factory.DelegateTargetVirtualMethod(target.GetCanonMethodTarget(CanonicalFormKind.Specific)); + + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + targetVirtualMethod, + factory.ReflectedDelegate(canonDelegateType), + "Target of delegate could be reflection-visible")); + + if (canonDelegateType.HasInstantiation) + { + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + targetVirtualMethod, + factory.ReflectedDelegate(canonDelegateType.GetTypeDefinition()), + "Target of delegate could be reflection-visible")); + } + + dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( + targetVirtualMethod, + factory.ReflectedDelegate(null), + "Target of delegate could be reflection-visible")); + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 804427d276c34..234fc31b09b13 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -425,6 +425,7 @@ + diff --git a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs index 43ba114cf02a0..858ecbbbe4804 100644 --- a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs +++ b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs @@ -24,6 +24,7 @@ public static int Run() TestBranchesInGenericCodeRemoval.Run(); TestUnmodifiableStaticFieldOptimization.Run(); TestUnmodifiableInstanceFieldOptimization.Run(); + TestGetMethodOptimization.Run(); return 100; } @@ -539,6 +540,79 @@ public static void Run() } } + class TestGetMethodOptimization + { + delegate void ReflectedOnDelegate(); + delegate void NotReflectedOnDelegate(); + delegate void ReflectedOnGenericDelegate(); + delegate void NotReflectedOnGenericDelegate(); + delegate void AnotherReflectedOnDelegate(); + + static class Delegates + { + public static void Method1() { } + public static ReflectedOnDelegate GetReflectedOnDelegate() => Method1; + + public static void Method2() { } + public static NotReflectedOnDelegate GetNotReflectedOnDelegate() => Method2; + + public static void Method3() { } + public static ReflectedOnGenericDelegate GetReflectedOnGenericDelegate() => Method3; + + public static void Method4() { } + public static NotReflectedOnGenericDelegate GetNotReflectedOnGenericDelegate() => Method4; + + public static void Method5() { } + public static AnotherReflectedOnDelegate GetAnotherReflectedOnDelegate() => Method5; + } + + static MethodInfo GetReflectedOnGenericDelegate() => Delegates.GetReflectedOnGenericDelegate().Method; + + static NotReflectedOnGenericDelegate GetNotReflectedOnGenericDelegate() => Delegates.GetNotReflectedOnGenericDelegate(); + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", + Justification = "That's the point")] + public static void Run() + { + Type t = GetTypeSecretly(typeof(TestGetMethodOptimization), nameof(Delegates)); + + { + ReflectedOnDelegate del = Delegates.GetReflectedOnDelegate(); + MethodInfo mi = t.GetMethod(nameof(Delegates.Method1)); + if (del.Method != mi) + throw new Exception(); + } + + { + NotReflectedOnDelegate del = Delegates.GetNotReflectedOnDelegate(); + MethodInfo mi = t.GetMethod(nameof(Delegates.Method2)); + if (mi != null) + throw new Exception(); + } + + { + MethodInfo m = GetReflectedOnGenericDelegate(); + MethodInfo mi = t.GetMethod(nameof(Delegates.Method3)); + if (m != mi) + throw new Exception(); + } + + { + NotReflectedOnGenericDelegate del = GetNotReflectedOnGenericDelegate(); + MethodInfo mi = t.GetMethod(nameof(Delegates.Method4)); + if (mi != null) + throw new Exception(); + } + + { + AnotherReflectedOnDelegate del = Delegates.GetAnotherReflectedOnDelegate(); + MethodInfo mi = t.GetMethod(nameof(Delegates.Method5)); + if (del.GetMethodInfo() != mi) + throw new Exception(); + } + } + } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", Justification = "That's the point")] private static Type GetTypeSecretly(Type testType, string typeName) => testType.GetNestedType(typeName, BindingFlags.NonPublic | BindingFlags.Public); diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs index 4392397cabf16..a9d3d4e83dd46 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs @@ -309,6 +309,10 @@ internal enum IntrinsicId /// AssemblyName_get_EscapedCodeBase, /// + /// + /// + RuntimeReflectionExtensions_GetMethodInfo, + /// /// /// RuntimeReflectionExtensions_GetRuntimeEvent, @@ -335,6 +339,10 @@ internal enum IntrinsicId /// /// /// - Nullable_GetUnderlyingType + Nullable_GetUnderlyingType, + /// + /// + /// + Delegate_get_Method, } } diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs index 42c7d00d9d2cd..72378937ec80b 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs @@ -38,6 +38,11 @@ public static IntrinsicId GetIntrinsicIdForMethod (MethodProxy calledMethod) // static System.Type.MakeGenericType (Type [] typeArguments) "MakeGenericType" when calledMethod.IsDeclaredOnType ("System.Type") => IntrinsicId.Type_MakeGenericType, + // static System.Reflection.RuntimeReflectionExtensions.GetMethodInfo (this Delegate del) + "GetMethodInfo" when calledMethod.IsDeclaredOnType ("System.Reflection.RuntimeReflectionExtensions") + && calledMethod.HasParameterOfType ((ParameterIndex) 0, "System.Delegate") + => IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo, + // static System.Reflection.RuntimeReflectionExtensions.GetRuntimeEvent (this Type type, string name) "GetRuntimeEvent" when calledMethod.IsDeclaredOnType ("System.Reflection.RuntimeReflectionExtensions") && calledMethod.HasParameterOfType ((ParameterIndex) 0, "System.Type") @@ -396,6 +401,12 @@ public static IntrinsicId GetIntrinsicIdForMethod (MethodProxy calledMethod) && calledMethod.HasParameterOfType ((ParameterIndex) 0, "System.Type") => IntrinsicId.Nullable_GetUnderlyingType, + // static System.Delegate.Method getter + "get_Method" when calledMethod.IsDeclaredOnType ("System.Delegate") + && calledMethod.HasImplicitThis () + && calledMethod.HasMetadataParametersCount (0) + => IntrinsicId.Delegate_get_Method, + _ => IntrinsicId.None, }; } From 89b0024afe028cbb5d43a9abb95edbe9826fc5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 19 Dec 2023 11:47:25 +0100 Subject: [PATCH 2/3] Try fixing illinker --- .../src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 984962cf456ed..2e3fbc4b0cef5 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -274,6 +274,8 @@ public static bool HandleCall ( case IntrinsicId.Assembly_GetFiles: case IntrinsicId.AssemblyName_get_CodeBase: case IntrinsicId.AssemblyName_get_EscapedCodeBase: + case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: + case IntrinsicId.Delegate_get_Method: // These intrinsics are not interesting for trimmer (they are interesting for AOT and that's why they are recognized) break; From ec22ff22cdc4c25e1a34c1139e0ab917f2f5e139 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 21 Dec 2023 09:20:42 +0100 Subject: [PATCH 3/3] Move a test Had to move this out because it depends on async stuff and that one makes it impossible to optimize out `Delegate.Method` with a debug corelib. --- .../SmokeTests/Reflection/Reflection.cs | 57 +++++++++++++++++++ .../SmokeTests/TrimmingBehaviors/Dataflow.cs | 52 ----------------- 2 files changed, 57 insertions(+), 52 deletions(-) diff --git a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs index 3be41c5fa1689..c000d2687b57f 100644 --- a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs +++ b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs @@ -70,6 +70,7 @@ private static int Main() TestGenericLdtoken.Run(); TestAbstractGenericLdtoken.Run(); TestTypeHandlesVisibleFromIDynamicInterfaceCastable.Run(); + TestCompilerGeneratedCode.Run(); // // Mostly functionality tests @@ -2542,6 +2543,62 @@ public static void Run() } } + class TestCompilerGeneratedCode + { + class Canary1 { } + class Canary2 { } + class Canary3 { } + class Canary4 { } + + private static void ReflectionInLambda() + { + var func = () => { + Type helpersType = Type.GetType(nameof(ReflectionTest) + "+" + nameof(TestCompilerGeneratedCode) + "+" + nameof(Canary1)); + Assert.NotNull(helpersType); + }; + + func(); + } + + private static void ReflectionInLocalFunction() + { + func(); + + void func() + { + Type helpersType = Type.GetType(nameof(ReflectionTest) + "+" + nameof(TestCompilerGeneratedCode) + "+" + nameof(Canary2)); + Assert.NotNull(helpersType); + }; + } + + private static async void ReflectionInAsync() + { + await System.Threading.Tasks.Task.Delay(100); + Type helpersType = Type.GetType(nameof(ReflectionTest) + "+" + nameof(TestCompilerGeneratedCode) + "+" + nameof(Canary3)); + Assert.NotNull(helpersType); + } + + private static async void ReflectionInLambdaAsync() + { + await System.Threading.Tasks.Task.Delay(100); + + var func = () => { + Type helpersType = Type.GetType(nameof(ReflectionTest) + "+" + nameof(TestCompilerGeneratedCode) + "+" + nameof(Canary4)); + Assert.NotNull(helpersType); + }; + + func(); + } + + public static void Run() + { + ReflectionInLambda(); + ReflectionInLocalFunction(); + ReflectionInAsync(); + ReflectionInLambdaAsync(); + } + } + #region Helpers private static Type SecretGetType(string testName, string typeName) diff --git a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs index 8e7f289db270f..fe050ada673ed 100644 --- a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs +++ b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs @@ -26,7 +26,6 @@ public static int Run() TestDynamicDependencyWithGenerics.Run(); TestObjectGetTypeDataflow.Run(); TestMarshalIntrinsics.Run(); - TestCompilerGeneratedCode.Run(); return 100; } @@ -632,57 +631,6 @@ static void SanityTest() } } } - - class TestCompilerGeneratedCode - { - private static void ReflectionInLambda() - { - var func = () => { - Type helpersType = Type.GetType(nameof(Helpers)); - Assert.NotNull(helpersType); - }; - - func(); - } - - private static void ReflectionInLocalFunction() - { - func(); - - void func() - { - Type helpersType = Type.GetType(nameof(Helpers)); - Assert.NotNull(helpersType); - }; - } - - private static async void ReflectionInAsync() - { - await System.Threading.Tasks.Task.Delay(100); - Type helpersType = Type.GetType(nameof(Helpers)); - Assert.NotNull(helpersType); - } - - private static async void ReflectionInLambdaAsync() - { - await System.Threading.Tasks.Task.Delay(100); - - var func = () => { - Type helpersType = Type.GetType(nameof(Helpers)); - Assert.NotNull(helpersType); - }; - - func(); - } - - public static void Run() - { - ReflectionInLambda(); - ReflectionInLocalFunction(); - ReflectionInAsync(); - ReflectionInLambdaAsync(); - } - } } static class Assert