From 1bbf5a4914610d994e92263dea7cc04968740ec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 17 Jun 2022 17:55:05 +0900 Subject: [PATCH 1/2] Generate method bodies for delegate Invoke methods There is an invalid dataflow warning suppression in System.Linq.Expressions that assumes we'll always have an invocable method body for delegate Invoke method. We need one in IL. We don't in native code. Emulate what IL Linker does and generate a method body. This is a size regression. Invalid suppression: https://github.com/dotnet/runtime/blob/3b2883b097a773715ca84056885e0ca1488da36e/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs#L906-L912 Fixes #70880. --- .../DependencyAnalysis/TypeMetadataNode.cs | 6 +++++- .../nativeaot/SmokeTests/Reflection/Reflection.cs | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs index a85e669a17a532..6604c3a0aac008 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs @@ -48,9 +48,13 @@ public override IEnumerable GetStaticDependencies(NodeFacto { // A delegate type metadata is rather useless without the Invoke method. // If someone reflects on a delegate, chances are they're going to look at the signature. + // The libraries (e.g. System.Linq.Expressions) also have trimming warning suppressions + // in places where they assume IL-level trimming (where the method cannot be removed). + // So we ask for a full reflectable method with its method body instead of just the + // metadata. var invokeMethod = _type.GetMethod("Invoke", null); if (!mdManager.IsReflectionBlocked(invokeMethod)) - dependencies.Add(factory.MethodMetadata(invokeMethod), "Delegate invoke method metadata"); + dependencies.Add(factory.ReflectableMethod(invokeMethod), "Delegate invoke method"); } if (_type.IsEnum) diff --git a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs index 78d1a6c0560c62..67894c9e5fb52e 100644 --- a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs +++ b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs @@ -9,6 +9,7 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Linq.Expressions; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Reflection; @@ -31,6 +32,7 @@ private static int Main() TestVirtualDelegateTargets.Run(); TestRunClassConstructor.Run(); TestFieldMetadata.Run(); + TestLinqInvocation.Run(); #if !OPTIMIZED_MODE_WITHOUT_SCANNER TestContainment.Run(); TestInterfaceMethod.Run(); @@ -1859,6 +1861,19 @@ public static void Run() } } + class TestLinqInvocation + { + delegate void RunMeDelegate(); + + static void RunMe() { } + + public static void Run() + { + Expression> ex = (RunMeDelegate m) => m(); + ex.Compile()(RunMe); + } + } + class TestRunClassConstructor { static class TypeWithNoStaticFieldsButACCtor From 8d25b4c761597310c99b61b53fc8bada90bb64df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 17 Jun 2022 20:48:52 +0900 Subject: [PATCH 2/2] FB --- .../Compiler/DependencyAnalysis/TypeMetadataNode.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs index 6604c3a0aac008..b81cddee9188c6 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs @@ -46,11 +46,10 @@ public override IEnumerable GetStaticDependencies(NodeFacto var mdManager = (UsageBasedMetadataManager)factory.MetadataManager; if (_type.IsDelegate) { - // A delegate type metadata is rather useless without the Invoke method. - // If someone reflects on a delegate, chances are they're going to look at the signature. - // The libraries (e.g. System.Linq.Expressions) also have trimming warning suppressions + // We've decided as a policy that delegate Invoke methods will be generated in full. + // The libraries (e.g. System.Linq.Expressions) have trimming warning suppressions // in places where they assume IL-level trimming (where the method cannot be removed). - // So we ask for a full reflectable method with its method body instead of just the + // We ask for a full reflectable method with its method body instead of just the // metadata. var invokeMethod = _type.GetMethod("Invoke", null); if (!mdManager.IsReflectionBlocked(invokeMethod))