From 6e4f0fca13822d4b895f2475d22bdb183a2203fc Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Thu, 11 May 2023 06:31:14 -0700 Subject: [PATCH 01/10] First attempt at the new behavior in illink and methods only --- .../src/linker/Linker.Steps/MarkStep.cs | 30 ++++++--- ...pilerGeneratedCodeAccessedViaReflection.cs | 14 ---- ...flectionAccessFromCompilerGeneratedCode.cs | 12 ---- .../RequiresAttributeMismatch.cs | 28 ++++---- .../RequiresInCompilerGeneratedCode.cs | 64 ------------------- .../RequiresCapability/RequiresOnClass.cs | 14 +--- .../RequiresOnVirtualsAndInterfaces.cs | 3 +- .../RequiresCapability/RequiresViaDataflow.cs | 3 +- 8 files changed, 38 insertions(+), 130 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 98b456626e093..28d92afd3d43b 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -3088,32 +3088,42 @@ void ProcessAnalysisAnnotationsForMethod (MethodDefinition method, DependencyKin if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider, out _)) return; - bool skipWarningsForOverride; bool isReflectionAccessCoveredByRUC; + bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (method); + bool forceRUCCheck = false; RequiresUnreferencedCodeAttribute? requiresUnreferencedCode; - if (dependencyKind == DependencyKind.AttributeProperty) { + switch (dependencyKind) { + case DependencyKind.AttributeProperty: // Property assignment in an attribute instance. // This case is more like a direct method call than reflection, and should // be logically similar to what is done in ReflectionMethodBodyScanner for method calls. - skipWarningsForOverride = false; isReflectionAccessCoveredByRUC = Annotations.DoesMethodRequireUnreferencedCode (method, out requiresUnreferencedCode); - } else { - // All override methods should have the same annotations as their base methods - // (else we will produce warning IL2046 or IL2092 or some other warning). - // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. - skipWarningsForOverride = dependencyKind == DependencyKind.DynamicallyAccessedMember && method.IsVirtual && Annotations.GetBaseMethods (method) != null; + break; + + case DependencyKind.Ldftn: + case DependencyKind.Ldvirtftn: + case DependencyKind.Ldtoken: + // Compiler generated code accessed via a token is considered a "hard" reference + // even though we also have to treat it as reflection access. + // So we need to enforce RUC check/warn in this case. + forceRUCCheck = true; + isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (method, out requiresUnreferencedCode); + break; + + default: // If the method being accessed has warnings suppressed due to Requires attributes, // we need to issue a warning for the reflection access. This is true even for instance // methods, which can be reflection-invoked without ever calling a constructor of the // accessed type. isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (method, out requiresUnreferencedCode); + break; } - if (isReflectionAccessCoveredByRUC && !skipWarningsForOverride) + if (isReflectionAccessCoveredByRUC && (!isCompilerGenerated || forceRUCCheck)) ReportRequiresUnreferencedCode (method.GetDisplayName (), requiresUnreferencedCode!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method); - if (isReflectionAccessCoveredByDAM && !skipWarningsForOverride) { + if (isReflectionAccessCoveredByDAM) { // ReflectionMethodBodyScanner handles more cases for data flow annotations // so don't warn for those. switch (dependencyKind) { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index 283f08e89446a..654eef9300af6 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -144,11 +144,7 @@ public IEnumerable InstanceIteratorCallsMethodWithRequires () [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators) + "()", "--RUCTypeWithIterators--")] // Expect to see warnings about RUC on type, for all static state machine members. [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators.StaticIteratorCallsMethodWithRequires) + "()", "--RUCTypeWithIterators--")] - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithIterators.StaticIteratorCallsMethodWithRequires) + ">", - ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators.InstanceIteratorCallsMethodWithRequires) + "()", ProducedBy = Tool.Trimmer | Tool.Analyzer)] - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithIterators.InstanceIteratorCallsMethodWithRequires) + ">", "(Int32)", - ProducedBy = Tool.Trimmer)] // state machine ctor [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithCorrectDataflow) + ">", "", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithProblematicDataflow) + ">", "", @@ -401,10 +397,6 @@ public void MethodWithLambdas () ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2026", nameof (RUCTypeWithLambdas) + "()", "--RUCTypeWithLambdas--")] [ExpectedWarning ("IL2026", nameof (RUCTypeWithLambdas.MethodWithLambdas) + "()", "--RUCTypeWithLambdas--", ProducedBy = Tool.Trimmer | Tool.Analyzer)] - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithLambdas.MethodWithLambdas) + ">", "--RUCTypeWithLambdas--", - ProducedBy = Tool.Trimmer)] - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithLambdas.MethodWithLambdas) + ">", "--RUCTypeWithLambdas--", - ProducedBy = Tool.Trimmer)] public static void Test (Lambdas test = null) { typeof (Lambdas).RequiresAll (); @@ -543,12 +535,6 @@ void LocalFunctionWithCapturedState () ProducedBy = Tool.Trimmer)] // Expect RUC warnings for static, compiler-generated code warnings for instance. [ExpectedWarning ("IL2026", nameof (RUCTypeWithLocalFunctions) + "()", "--RUCTypeWithLocalFunctions--")] - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithLocalFunctions.MethodWithLocalFunctions) + ">", "LocalFunctionWithCapturedState", - ProducedBy = Tool.Trimmer)] // displayclass ctor - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithLocalFunctions.MethodWithLocalFunctions) + ">", "StaticLocalFunction", - ProducedBy = Tool.Trimmer)] - [ExpectedWarning ("IL2026", "<" + nameof (RUCTypeWithLocalFunctions.MethodWithLocalFunctions) + ">", "LocalFunction", - ProducedBy = Tool.Trimmer | Tool.NativeAot)] [ExpectedWarning ("IL2026", nameof (RUCTypeWithLocalFunctions.MethodWithLocalFunctions) + "()", ProducedBy = Tool.Trimmer | Tool.Analyzer)] public static void Test (LocalFunctions test = null) { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs index f58b051b4a3e9..2e8ae0d476c61 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs @@ -26,9 +26,6 @@ class ReflectionAccessFromStateMachine [ExpectedWarning ("IL2026", "--TypeWithMethodWithRequires.MethodWithRequires--", CompilerGeneratedCode = true)] [ExpectedWarning ("IL3002", "--TypeWithMethodWithRequires.MethodWithRequires--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TypeWithMethodWithRequires.MethodWithRequires--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--MethodWithLocalFunctionWithRUC.LocalFunction--", CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--MethodWithLocalFunctionWithRUC.LocalFunction--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--MethodWithLocalFunctionWithRUC.LocalFunction--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2118", nameof (TypeWithMethodWithRequires.MethodWithLocalFunctionCallsRUC), "LocalFunction", CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2111", nameof (TypeWithMethodWithRequires.MethodWithAnnotations), CompilerGeneratedCode = true)] @@ -50,9 +47,6 @@ static IEnumerable TestIteratorWithRUC () [ExpectedWarning ("IL2026", "--TypeWithMethodWithRequires.MethodWithRequires--", CompilerGeneratedCode = true)] [ExpectedWarning ("IL3002", "--TypeWithMethodWithRequires.MethodWithRequires--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TypeWithMethodWithRequires.MethodWithRequires--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--MethodWithLocalFunctionWithRUC.LocalFunction--", CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--MethodWithLocalFunctionWithRUC.LocalFunction--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--MethodWithLocalFunctionWithRUC.LocalFunction--", CompilerGeneratedCode = true, ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2118", nameof (TypeWithMethodWithRequires.MethodWithLocalFunctionCallsRUC), "LocalFunction", CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2111", nameof (TypeWithMethodWithRequires.MethodWithAnnotations), CompilerGeneratedCode = true)] @@ -93,9 +87,6 @@ static void TestLocalFunction () [ExpectedWarning ("IL2026", "--TypeWithMethodWithRequires.MethodWithRequires--")] [ExpectedWarning ("IL3002", "--TypeWithMethodWithRequires.MethodWithRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TypeWithMethodWithRequires.MethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--MethodWithLocalFunctionWithRUC.LocalFunction--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--MethodWithLocalFunctionWithRUC.LocalFunction--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--MethodWithLocalFunctionWithRUC.LocalFunction--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2118", nameof (TypeWithMethodWithRequires.MethodWithLocalFunctionCallsRUC), "LocalFunction", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2111", nameof (TypeWithMethodWithRequires.MethodWithAnnotations))] @@ -152,9 +143,6 @@ static void TestLambda () [ExpectedWarning ("IL2026", "--TypeWithMethodWithRequires.MethodWithRequires--")] [ExpectedWarning ("IL3002", "--TypeWithMethodWithRequires.MethodWithRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TypeWithMethodWithRequires.MethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--MethodWithLocalFunctionWithRUC.LocalFunction--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--MethodWithLocalFunctionWithRUC.LocalFunction--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--MethodWithLocalFunctionWithRUC.LocalFunction--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2118", nameof (TypeWithMethodWithRequires.MethodWithLocalFunctionCallsRUC), "LocalFunction", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2111", nameof (TypeWithMethodWithRequires.MethodWithAnnotations))] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs index e5b28b192b763..62b8e029f0064 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs @@ -22,7 +22,7 @@ namespace Mono.Linker.Tests.Cases.RequiresCapability class RequiresAttributeMismatch { // General comment about IL3003 - // Analyzer looks at properties, events and method separately. So mistmatch on property level will be reported + // Analyzer looks at properties, events and methods separately. So mismatch on property level will be reported // only on the property and not on the accessors. And vice versa. // NativeAOT doesn't really see properties and events, only methods. So it is much easier to handle everything // on the method level. While it's a discrepancy in behavior, it's small and should have no adverse effects @@ -31,8 +31,6 @@ class RequiresAttributeMismatch // then both accessors will report IL3003 (the attribute on the property is treated as if it was specified // on all accessors, always). // This discrepancy is tracked by https://github.com/dotnet/runtime/issues/83235. - - // Base/Derived and Implementation/Interface differs between ILLink and analyzer https://github.com/dotnet/linker/issues/2533 [ExpectedWarning ("IL2026", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] @@ -42,11 +40,11 @@ class RequiresAttributeMismatch [ExpectedWarning ("IL2026", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "DerivedClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.set", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.get", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.set", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor.set", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL2026", "DerivedClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] + [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.set")] + [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.get")] + [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.set")] + [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor.set")] [ExpectedWarning ("IL2026", "BaseClassWithRequires.VirtualMethod()")] [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] @@ -56,7 +54,7 @@ class RequiresAttributeMismatch [ExpectedWarning ("IL2026", "BaseClassWithRequires.VirtualMethod()")] [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "DerivedClassWithRequires.VirtualMethod()", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL2026", "DerivedClassWithRequires.VirtualMethod()")] [ExpectedWarning ("IL2026", "IBaseWithRequires.PropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "IBaseWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "IBaseWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] @@ -68,23 +66,23 @@ class RequiresAttributeMismatch [ExpectedWarning ("IL2026", "IBaseWithRequires.Method()")] [ExpectedWarning ("IL3002", "IBaseWithRequires.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "IBaseWithRequires.Method()", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "ImplementationClassWithRequires.Method()", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationClassWithRequires.Method()")] [ExpectedWarning ("IL3002", "ImplementationClassWithRequires.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "ImplementationClassWithRequires.Method()", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "ImplementationClassWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationClassWithRequires.PropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "ImplementationClassWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "ImplementationClassWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3002", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3002", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor.set", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3002", "ImplementationClassWithRequires.PropertyAnnotationInProperty.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3002", "ImplementationClassWithRequires.PropertyAnnotationInProperty.set", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3002", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "ImplementationClassWithRequiresInSource.Method()", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationClassWithRequiresInSource.Method()")] [ExpectedWarning ("IL3002", "ImplementationClassWithRequiresInSource.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "ImplementationClassWithRequiresInSource.Method()", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "ImplementationClassWithRequiresInSource.PropertyAnnotationInAccesor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationClassWithRequiresInSource.PropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "ImplementationClassWithRequiresInSource.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "ImplementationClassWithRequiresInSource.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3002", "ImplementationClassWithRequiresInSource.PropertyAnnotationInProperty.get", ProducedBy = Tool.NativeAot)] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs index 06179031c754b..6fdb10f72bfb9 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs @@ -1880,11 +1880,6 @@ static async void TestAsyncOnlyReferencedViaReflectionWhichShouldWarn () [ExpectedWarning ("IL2026", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--")] [ExpectedWarning ("IL3002", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - // Analyzer doesn't emit additional warnings about reflection access to the compiler-generated - // state machine members. - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] // The MoveNext, Current.get and Reset methods produces warning in NativeAOT // https://github.com/dotnet/runtime/issues/82447 @@ -1915,15 +1910,6 @@ static async void TestAsyncOnlyReferencedViaReflectionWhichShouldWarn () [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] -#if !RELEASE - [ExpectedWarning ("IL2026", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - // In debug mode, the async state machine is a class with a constructor, so a warning is emitted for the constructor. - // The MoveNext method is virtual, so doesn't warn either way. -#else - // In release mode, the async state machine is a struct which doesn't have a constructor, so no warning is emitted. -#endif // The MoveNext method produces warning in NativeAOT // https://github.com/dotnet/runtime/issues/82447 [ExpectedWarning ("IL2026", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] @@ -2045,24 +2031,6 @@ void LocalFunction () [ExpectedWarning ("IL2026", "--TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection--")] [ExpectedWarning ("IL3002", "--TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] - // Trimming tools correctly emit warnings about reflection access to local functions with Requires - // or which inherit Requires from the containing method. The analyzer doesn't bind to local functions - // so does not warn here. - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithClosureWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithClosureWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithClosureWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionInMethodWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithClosureInMethodWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2118", nameof (LocalFunctionsReferencedViaReflection), nameof (TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection), ProducedBy = Tool.Trimmer)] static void TestAll () @@ -2080,22 +2048,6 @@ static void TestAll () [ExpectedWarning ("IL2026", "--TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection--")] [ExpectedWarning ("IL3002", "--TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] - // NonPublicMethods warns for local functions not emitted into display classes. - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithRequiresOnlyAccessedViaReflection--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithClosureWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithClosureWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithClosureWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionInMethodWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLocalFunctionWithClosureInMethodWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLocalFunctionWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLocalFunctionWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2118", nameof (LocalFunctionsReferencedViaReflection), "<" + nameof (TestLocalFunctionInMethodWithRequiresOnlyAccessedViaReflection) + ">", ProducedBy = Tool.Trimmer)] static void TestNonPublicMethods () @@ -2173,21 +2125,6 @@ static void TestLambdaWithClosureInMethodWithRequires (int p = 0) [ExpectedWarning ("IL2026", "--TestLambdaWithClosureInMethodWithRequires--")] [ExpectedWarning ("IL3002", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - // Trimming tools correctly emit warnings about reflection access to lambdas with Requires - // or which inherit Requires from the containing method. The analyzer doesn't bind to lambdas - // so does not warn here. - [ExpectedWarning ("IL2026", "--TestLambdaWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLambdaWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLambdaWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLambdaWithClosureWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLambdaWithClosureWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLambdaWithClosureWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLambdaInMethodWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLambdaInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLambdaInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] static void TestAll () { typeof (LambdasReferencedViaReflection).RequiresAll (); @@ -2200,7 +2137,6 @@ static void TestAll () [ExpectedWarning ("IL2026", "--TestLambdaWithClosureInMethodWithRequires--")] [ExpectedWarning ("IL3002", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestLambdaWithClosureInMethodWithRequires--", ProducedBy = Tool.NativeAot)] - // NonPublicMethods doesn't warn for lambdas emitted into display class types. static void TestNonPublicMethods () { typeof (LambdasReferencedViaReflection).RequiresNonPublicMethods (); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs index 2ae2c7fb2002b..e0a64ced8be8d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs @@ -535,24 +535,16 @@ class ReflectionAccessOnMethod [ExpectedWarning ("IL3050", "BaseWithoutRequiresOnType.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "BaseWithoutRequiresOnType.Method()")] [ExpectedWarning ("IL3050", "BaseWithoutRequiresOnType.Method()", ProducedBy = Tool.NativeAot)] - // https://github.com/dotnet/linker/issues/2533 - [ExpectedWarning ("IL2026", "DerivedWithRequiresOnType.Method()", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL2026", "DerivedWithRequiresOnType.Method()")] [ExpectedWarning ("IL2026", "InterfaceWithoutRequires.Method(Int32)")] [ExpectedWarning ("IL3050", "InterfaceWithoutRequires.Method(Int32)", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "InterfaceWithoutRequires.Method()")] [ExpectedWarning ("IL3050", "InterfaceWithoutRequires.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "ImplementationWithRequiresOnType.Method()")] [ExpectedWarning ("IL3050", "ImplementationWithRequiresOnType.Method()", ProducedBy = Tool.NativeAot)] - // https://github.com/dotnet/linker/issues/2533 - // NativeAOT has a correct override resolution and in this case the method is not an override - so it should warn - [ExpectedWarning ("IL2026", "ImplementationWithRequiresOnType.Method(Int32)", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "ImplementationWithRequiresOnType.Method(Int32)")] [ExpectedWarning ("IL3050", "ImplementationWithRequiresOnType.Method(Int32)", ProducedBy = Tool.NativeAot)] - // ILLink incorrectly skips warnings for derived method, under the assumption that - // it will be covered by the base method. But in this case the base method - // is unannotated (and the mismatch produces no warning because the derived - // type has RUC). - // https://github.com/dotnet/linker/issues/2533 - [ExpectedWarning ("IL2026", "DerivedWithRequiresOnTypeOverBaseWithNoRequires.Method()", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL2026", "DerivedWithRequiresOnTypeOverBaseWithNoRequires.Method()")] static void TestDAMAccess () { // Warns because BaseWithoutRequiresOnType.Method has Requires on the method diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs index 5470aa1171787..b3c4696a65348 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs @@ -317,8 +317,7 @@ public virtual void RUCMethod () { } // Reflection triggered warnings are not produced by analyzer for RDC/RAS [ExpectedWarning ("IL3002", "Message for --NewSlotVirtual.Base.RUCMethod--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "Message for --NewSlotVirtual.Base.RUCMethod--", ProducedBy = Tool.NativeAot)] - // https://github.com/dotnet/linker/issues/2815 - [ExpectedWarning ("IL2026", "Message for --NewSlotVirtual.Derived.RUCMethod--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "Message for --NewSlotVirtual.Derived.RUCMethod--")] // Reflection triggered warnings are not produced by analyzer for RDC/RAS [ExpectedWarning ("IL3002", "Message for --NewSlotVirtual.Derived.RUCMethod--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "Message for --NewSlotVirtual.Derived.RUCMethod--", ProducedBy = Tool.NativeAot)] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs index 15dd829a93a19..a1941969b9c23 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs @@ -15,9 +15,8 @@ namespace Mono.Linker.Tests.Cases.RequiresCapability [ExpectedNoWarnings] class RequiresViaDataflow { - // Base/Derived and Implementation/Interface differs between ILLink and analyzer https://github.com/dotnet/linker/issues/2533 [ExpectedWarning ("IL2026", "--DynamicallyAccessedTypeWithRequires.MethodWithRequires--")] - [ExpectedWarning ("IL2026", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL2026", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--")] [ExpectedWarning ("IL2026", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--")] [ExpectedWarning ("IL3002", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] From f8d4bb1351f3d10de27008e5e2295e6ab0c00e4a Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 12 May 2023 06:32:16 -0700 Subject: [PATCH 02/10] Port to NativeAOT and fix tests --- .../Compiler/Dataflow/ReflectionMarker.cs | 32 +++++++-------- .../RequiresAttributeMismatch.cs | 11 ++++++ .../RequiresInCompilerGeneratedCode.cs | 39 ------------------- .../RequiresCapability/RequiresOnClass.cs | 2 + .../RequiresCapability/RequiresViaDataflow.cs | 2 + 5 files changed, 30 insertions(+), 56 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs index d32757d162d80..a5f210e5367fb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs @@ -239,30 +239,24 @@ private void ReportWarningsForReflectionAccess(in MessageOrigin origin, TypeSyst // This is because reflection access is actually problematic on all members which are in a "requires" scope // so for example even instance methods. See for example https://github.com/dotnet/linker/issues/3140 - it's possible // to call a method on a "null" instance via reflection. - if (_logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresUnreferencedCodeAttribute, out CustomAttributeValue? requiresAttribute)) - { - if (!ShouldSkipWarningsForOverride(entity, accessKind)) + if (_logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresUnreferencedCodeAttribute, out CustomAttributeValue? requiresAttribute) && + ShouldProduceRequiresWarningForReflectionAccess(entity, accessKind)) ReportRequires(origin, entity, DiagnosticUtilities.RequiresUnreferencedCodeAttribute, requiresAttribute.Value); - } - if (_logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresAssemblyFilesAttribute, out requiresAttribute)) - { - if (!ShouldSkipWarningsForOverride(entity, accessKind)) + if (_logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresAssemblyFilesAttribute, out requiresAttribute) && + ShouldProduceRequiresWarningForReflectionAccess(entity, accessKind)) ReportRequires(origin, entity, DiagnosticUtilities.RequiresAssemblyFilesAttribute, requiresAttribute.Value); - } - if (_logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresDynamicCodeAttribute, out requiresAttribute)) - { - if (!ShouldSkipWarningsForOverride(entity, accessKind)) + if (_logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresDynamicCodeAttribute, out requiresAttribute) && + ShouldProduceRequiresWarningForReflectionAccess(entity, accessKind)) ReportRequires(origin, entity, DiagnosticUtilities.RequiresDynamicCodeAttribute, requiresAttribute.Value); - } // Below is about accessing DAM annotated members, so only RUC is applicable as a suppression scope if (_logger.ShouldSuppressAnalysisWarningsForRequires(origin.MemberDefinition, DiagnosticUtilities.RequiresUnreferencedCodeAttribute)) return; bool isReflectionAccessCoveredByDAM = Annotations.ShouldWarnWhenAccessedForReflection(entity); - if (isReflectionAccessCoveredByDAM && !ShouldSkipWarningsForOverride(entity, accessKind)) + if (isReflectionAccessCoveredByDAM) { if (entity is MethodDesc) _logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, entity.GetDisplayName()); @@ -277,12 +271,16 @@ private void ReportWarningsForReflectionAccess(in MessageOrigin origin, TypeSyst // (else we will produce warning IL2046 or IL2092 or some other warning). // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. // PERF: Avoid precomputing this as this method is relatively expensive. Only call it once we're about to produce a warning. - static bool ShouldSkipWarningsForOverride(TypeSystemEntity entity, AccessKind accessKind) + static bool ShouldProduceRequiresWarningForReflectionAccess(TypeSystemEntity entity, AccessKind accessKind) { - if (accessKind != AccessKind.DynamicallyAccessedMembersMark || entity is not MethodDesc method || !method.IsVirtual) - return false; + bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember(entity); - return MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(method) != method; + // Compiler generated code accessed via a token is considered a "hard" reference + // even though we also have to treat it as reflection access. + // So we need to enforce RUC check/warn in this case. + bool forceRequiresWarning = accessKind == AccessKind.TokenAccess; + + return !isCompilerGenerated || forceRequiresWarning; } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs index 62b8e029f0064..6e2fcca2875db 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs @@ -41,10 +41,19 @@ class RequiresAttributeMismatch [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] + [ExpectedWarning ("IL3002", "DerivedClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "DerivedClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3002", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty.get", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3002", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty.set", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3002", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.set")] + [ExpectedWarning ("IL3002", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.set", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.get")] + [ExpectedWarning ("IL3002", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.set")] + [ExpectedWarning ("IL3002", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.set", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor.set")] + [ExpectedWarning ("IL3002", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor.set", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "BaseClassWithRequires.VirtualMethod()")] [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] @@ -55,6 +64,8 @@ class RequiresAttributeMismatch [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedClassWithRequires.VirtualMethod()")] + [ExpectedWarning ("IL3002", "DerivedClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "DerivedClassWithRequires.VirtualMethod()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "IBaseWithRequires.PropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "IBaseWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "IBaseWithRequires.PropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs index 6fdb10f72bfb9..06a7b319cd8c1 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs @@ -1880,45 +1880,6 @@ static async void TestAsyncOnlyReferencedViaReflectionWhichShouldWarn () [ExpectedWarning ("IL2026", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--")] [ExpectedWarning ("IL3002", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - - // The MoveNext, Current.get and Reset methods produces warning in NativeAOT - // https://github.com/dotnet/runtime/issues/82447 - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestIteratorOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - // The MoveNext method produces warning in NativeAOT - // https://github.com/dotnet/runtime/issues/82447 - [ExpectedWarning ("IL2026", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--TestAsyncOnlyReferencedViaReflectionWhichShouldSuppress--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2118", nameof (StateMachinesOnlyReferencedViaReflection), "<" + nameof (TestAsyncOnlyReferencedViaReflectionWhichShouldWarn) + ">", "MoveNext()", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2118", nameof (StateMachinesOnlyReferencedViaReflection), "<" + nameof (TestIteratorOnlyReferencedViaReflectionWhichShouldWarn) + ">", "MoveNext()", diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs index e0a64ced8be8d..b2c3398e12937 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs @@ -536,6 +536,7 @@ class ReflectionAccessOnMethod [ExpectedWarning ("IL2026", "BaseWithoutRequiresOnType.Method()")] [ExpectedWarning ("IL3050", "BaseWithoutRequiresOnType.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedWithRequiresOnType.Method()")] + [ExpectedWarning ("IL3050", "DerivedWithRequiresOnType.Method()", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "InterfaceWithoutRequires.Method(Int32)")] [ExpectedWarning ("IL3050", "InterfaceWithoutRequires.Method(Int32)", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "InterfaceWithoutRequires.Method()")] @@ -545,6 +546,7 @@ class ReflectionAccessOnMethod [ExpectedWarning ("IL2026", "ImplementationWithRequiresOnType.Method(Int32)")] [ExpectedWarning ("IL3050", "ImplementationWithRequiresOnType.Method(Int32)", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "DerivedWithRequiresOnTypeOverBaseWithNoRequires.Method()")] + [ExpectedWarning ("IL3050", "DerivedWithRequiresOnTypeOverBaseWithNoRequires.Method()", ProducedBy = Tool.NativeAot)] static void TestDAMAccess () { // Warns because BaseWithoutRequiresOnType.Method has Requires on the method diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs index a1941969b9c23..ccea30b08a638 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs @@ -17,6 +17,8 @@ class RequiresViaDataflow { [ExpectedWarning ("IL2026", "--DynamicallyAccessedTypeWithRequires.MethodWithRequires--")] [ExpectedWarning ("IL2026", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--")] + [ExpectedWarning ("IL3002", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL2026", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--")] [ExpectedWarning ("IL3002", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] From 825e982de02677d07000243571942a70ce402f97 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 12 May 2023 13:32:11 -0700 Subject: [PATCH 03/10] Refactorings --- .../src/linker/Linker.Steps/MarkStep.cs | 138 +++++++++--------- .../RequiresCapability/RequiresViaDataflow.cs | 107 ++++++++------ 2 files changed, 135 insertions(+), 110 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 28d92afd3d43b..5be744d4929d9 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1628,6 +1628,74 @@ protected void MarkField (FieldReference reference, DependencyInfo reason, in Me MarkField (field, reason, origin); } + void ReportWarningsForReflectionAccess (in MessageOrigin origin, MethodDefinition method, DependencyKind dependencyKind) + { + if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider, out _)) + return; + + bool isReflectionAccessCoveredByRUC; + bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (method); + bool forceRUCCheck = false; + RequiresUnreferencedCodeAttribute? requiresUnreferencedCode; + switch (dependencyKind) { + case DependencyKind.AttributeProperty: + // Property assignment in an attribute instance. + // This case is more like a direct method call than reflection, and should + // be logically similar to what is done in ReflectionMethodBodyScanner for method calls. + isReflectionAccessCoveredByRUC = Annotations.DoesMethodRequireUnreferencedCode (method, out requiresUnreferencedCode); + break; + + case DependencyKind.Ldftn: + case DependencyKind.Ldvirtftn: + case DependencyKind.Ldtoken: + // Compiler generated code accessed via a token is considered a "hard" reference + // even though we also have to treat it as reflection access. + // So we need to enforce RUC check/warn in this case. + forceRUCCheck = true; + isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (method, out requiresUnreferencedCode); + break; + + default: + // If the method being accessed has warnings suppressed due to Requires attributes, + // we need to issue a warning for the reflection access. This is true even for instance + // methods, which can be reflection-invoked without ever calling a constructor of the + // accessed type. + isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (method, out requiresUnreferencedCode); + break; + } + + if (isReflectionAccessCoveredByRUC && (!isCompilerGenerated || forceRUCCheck)) + ReportRequiresUnreferencedCode (method.GetDisplayName (), requiresUnreferencedCode!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); + + bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method); + if (isReflectionAccessCoveredByDAM) { + // ReflectionMethodBodyScanner handles more cases for data flow annotations + // so don't warn for those. + switch (dependencyKind) { + case DependencyKind.AttributeConstructor: + case DependencyKind.AttributeProperty: + break; + default: + Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, method.GetDisplayName ()); + break; + } + } + + // Warn on reflection access to compiler-generated methods, if the method isn't already unsafe to access via reflection + // due to annotations. For the annotation-based warnings, we skip virtual overrides since those will produce warnings on + // the base, but for unannotated compiler-generated methods this is not the case, so we must produce these warnings even + // for virtual overrides. This ensures that we include the unannotated MoveNext state machine method. Lambdas and local + // functions should never be virtual overrides in the first place. + bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM; + switch (dependencyKind) { + case DependencyKind.AccessedViaReflection: + case DependencyKind.DynamicallyAccessedMember: + if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (method, isCoveredByAnnotations)) + Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, method.GetDisplayName ()); + break; + } + } + void ReportWarningsForTypeHierarchyReflectionAccess (IMemberDefinition member, MessageOrigin origin) { Debug.Assert (member is MethodDefinition or FieldDefinition); @@ -3071,84 +3139,20 @@ void ProcessAnalysisAnnotationsForMethod (MethodDefinition method, DependencyKin // Don't warn for methods kept due to non-understood DebuggerDisplayAttribute // until https://github.com/dotnet/linker/issues/1873 is fixed. case DependencyKind.KeptForSpecialAttribute: - return; + break; case DependencyKind.DynamicallyAccessedMemberOnType: // DynamicallyAccessedMembers on type gets special treatment so that the warning origin // is the type or the annotated member. ReportWarningsForTypeHierarchyReflectionAccess (method, origin); - return; + break; default: // All other cases have the potential of us missing a warning if we don't report it // It is possible that in some cases we may report the same warning twice, but that's better than not reporting it. + ReportWarningsForReflectionAccess (origin, method, dependencyKind); break; }; - - if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider, out _)) - return; - - bool isReflectionAccessCoveredByRUC; - bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (method); - bool forceRUCCheck = false; - RequiresUnreferencedCodeAttribute? requiresUnreferencedCode; - switch (dependencyKind) { - case DependencyKind.AttributeProperty: - // Property assignment in an attribute instance. - // This case is more like a direct method call than reflection, and should - // be logically similar to what is done in ReflectionMethodBodyScanner for method calls. - isReflectionAccessCoveredByRUC = Annotations.DoesMethodRequireUnreferencedCode (method, out requiresUnreferencedCode); - break; - - case DependencyKind.Ldftn: - case DependencyKind.Ldvirtftn: - case DependencyKind.Ldtoken: - // Compiler generated code accessed via a token is considered a "hard" reference - // even though we also have to treat it as reflection access. - // So we need to enforce RUC check/warn in this case. - forceRUCCheck = true; - isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (method, out requiresUnreferencedCode); - break; - - default: - // If the method being accessed has warnings suppressed due to Requires attributes, - // we need to issue a warning for the reflection access. This is true even for instance - // methods, which can be reflection-invoked without ever calling a constructor of the - // accessed type. - isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (method, out requiresUnreferencedCode); - break; - } - - if (isReflectionAccessCoveredByRUC && (!isCompilerGenerated || forceRUCCheck)) - ReportRequiresUnreferencedCode (method.GetDisplayName (), requiresUnreferencedCode!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); - - bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method); - if (isReflectionAccessCoveredByDAM) { - // ReflectionMethodBodyScanner handles more cases for data flow annotations - // so don't warn for those. - switch (dependencyKind) { - case DependencyKind.AttributeConstructor: - case DependencyKind.AttributeProperty: - break; - default: - Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, method.GetDisplayName ()); - break; - } - } - - // Warn on reflection access to compiler-generated methods, if the method isn't already unsafe to access via reflection - // due to annotations. For the annotation-based warnings, we skip virtual overrides since those will produce warnings on - // the base, but for unannotated compiler-generated methods this is not the case, so we must produce these warnings even - // for virtual overrides. This ensures that we include the unannotated MoveNext state machine method. Lambdas and local - // functions should never be virtual overrides in the first place. - bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM; - switch (dependencyKind) { - case DependencyKind.AccessedViaReflection: - case DependencyKind.DynamicallyAccessedMember: - if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (method, isCoveredByAnnotations)) - Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, method.GetDisplayName ()); - break; - } } internal static void ReportRequiresUnreferencedCode (string displayName, RequiresUnreferencedCodeAttribute requiresUnreferencedCode, in DiagnosticContext diagnosticContext) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs index ccea30b08a638..ff31846f08c4a 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs @@ -15,67 +15,88 @@ namespace Mono.Linker.Tests.Cases.RequiresCapability [ExpectedNoWarnings] class RequiresViaDataflow { - [ExpectedWarning ("IL2026", "--DynamicallyAccessedTypeWithRequires.MethodWithRequires--")] - [ExpectedWarning ("IL2026", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--")] - [ExpectedWarning ("IL3002", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--")] - [ExpectedWarning ("IL3002", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] public static void Main () { - TestDynamicallyAccessedMembersWithRequires (typeof (DynamicallyAccessedTypeWithRequires)); - TestDynamicallyAccessedMembersWithRequires (typeof (TypeWhichOverridesMethod)); - TestRequiresInDynamicDependency (); + AnnotatedParameter.Test (); + DynamicDependency.Test (); } - class BaseType + class AnnotatedParameter { - [RequiresUnreferencedCode ("Message for --BaseType.VirtualMethodRequires--")] - [RequiresAssemblyFiles ("Message for --BaseType.VirtualMethodRequires--")] - [RequiresDynamicCode ("Message for --BaseType.VirtualMethodRequires--")] - public virtual void VirtualMethodRequires () + static void MethodWithAnnotatedParameter ( + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) { } - } - class TypeWhichOverridesMethod : BaseType - { - [RequiresUnreferencedCode ("Message for --TypeWhichOverridesMethod.VirtualMethodRequires--")] - [RequiresAssemblyFiles ("Message for --TypeWhichOverridesMethod.VirtualMethodRequires--")] - [RequiresDynamicCode ("Message for --TypeWhichOverridesMethod.VirtualMethodRequires--")] - public override void VirtualMethodRequires () + public class DynamicallyAccessedTypeWithRequires { + [RequiresUnreferencedCode ("Message for --DynamicallyAccessedTypeWithRequires.MethodWithRequires--")] + public void MethodWithRequires () + { + } } - } - public class DynamicallyAccessedTypeWithRequires - { - [RequiresUnreferencedCode ("Message for --DynamicallyAccessedTypeWithRequires.MethodWithRequires--")] - public void MethodWithRequires () + [ExpectedWarning ("IL2026", "--DynamicallyAccessedTypeWithRequires.MethodWithRequires--")] + static void TestNonVirtualMethod () { + MethodWithAnnotatedParameter (typeof (DynamicallyAccessedTypeWithRequires)); } - } - static void TestDynamicallyAccessedMembersWithRequires ( - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) - { - } + class BaseType + { + [RequiresUnreferencedCode ("Message for --BaseType.VirtualMethodRequires--")] + [RequiresAssemblyFiles ("Message for --BaseType.VirtualMethodRequires--")] + [RequiresDynamicCode ("Message for --BaseType.VirtualMethodRequires--")] + public virtual void VirtualMethodRequires () + { + } + } - [RequiresUnreferencedCode ("Message for --RequiresInDynamicDependency--")] - [RequiresAssemblyFiles ("Message for --RequiresInDynamicDependency--")] - [RequiresDynamicCode ("Message for --RequiresInDynamicDependency--")] - static void RequiresInDynamicDependency () - { + class TypeWhichOverridesMethod : BaseType + { + [RequiresUnreferencedCode ("Message for --TypeWhichOverridesMethod.VirtualMethodRequires--")] + [RequiresAssemblyFiles ("Message for --TypeWhichOverridesMethod.VirtualMethodRequires--")] + [RequiresDynamicCode ("Message for --TypeWhichOverridesMethod.VirtualMethodRequires--")] + public override void VirtualMethodRequires () + { + } + } + + [ExpectedWarning ("IL2026", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--")] + [ExpectedWarning ("IL3002", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "TypeWhichOverridesMethod.VirtualMethodRequires()", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2026", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--")] + [ExpectedWarning ("IL3002", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "BaseType.VirtualMethodRequires()", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + static void TestOverriddenVirtualMethod () + { + MethodWithAnnotatedParameter (typeof (TypeWhichOverridesMethod)); + } + + public static void Test () + { + TestNonVirtualMethod (); + TestOverriddenVirtualMethod (); + } } - // https://github.com/dotnet/runtime/issues/83080 - Analyzer doesn't recognize DynamicDependency in any way - [ExpectedWarning ("IL2026", "--RequiresInDynamicDependency--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--RequiresInDynamicDependency--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--RequiresInDynamicDependency--", ProducedBy = Tool.NativeAot)] - [DynamicDependency ("RequiresInDynamicDependency")] - static void TestRequiresInDynamicDependency () + class DynamicDependency { + [RequiresUnreferencedCode ("Message for --RequiresInDynamicDependency--")] + [RequiresAssemblyFiles ("Message for --RequiresInDynamicDependency--")] + [RequiresDynamicCode ("Message for --RequiresInDynamicDependency--")] + static void RequiresInDynamicDependency () + { + } + + // https://github.com/dotnet/runtime/issues/83080 - Analyzer doesn't recognize DynamicDependency in any way + [ExpectedWarning ("IL2026", "--RequiresInDynamicDependency--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL3002", "--RequiresInDynamicDependency--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--RequiresInDynamicDependency--", ProducedBy = Tool.NativeAot)] + [DynamicDependency ("RequiresInDynamicDependency")] + public static void Test () + { + } } } } From ba13eed95279b66d83169aa2c4bc122c0c30ae59 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Sun, 14 May 2023 14:52:37 -0700 Subject: [PATCH 04/10] Fixes two bugs and test changes --- .../src/linker/Linker.Steps/MarkStep.cs | 11 ++-- ...pilerGeneratedCodeAccessedViaReflection.cs | 20 ++----- .../TypeHierarchyReflectionWarnings.cs | 33 +++++++----- .../RequiresAccessedThrough.cs | 54 ------------------- .../RequiresOnVirtualsAndInterfaces.cs | 14 ++--- .../RequiresCapability/RequiresViaDataflow.cs | 54 +++++++++++++++++++ 6 files changed, 84 insertions(+), 102 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 5be744d4929d9..112e94a248ca3 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1721,14 +1721,9 @@ static bool IsDeclaredWithinType (IMemberDefinition member, TypeDefinition type) if (reportOnMember) origin = new MessageOrigin (member); - - // All override methods should have the same annotations as their base methods - // (else we will produce warning IL2046 or IL2092 or some other warning). - // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. - bool skipWarningsForOverride = member is MethodDefinition m && m.IsVirtual && Annotations.GetBaseMethods (m) != null; - bool isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (member, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute); - if (isReflectionAccessCoveredByRUC && !skipWarningsForOverride) { + bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (member); + if (isReflectionAccessCoveredByRUC && !isCompilerGenerated) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithRequiresUnreferencedCode : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithRequiresUnreferencedCode; Context.LogWarning (origin, id, type.GetDisplayName (), ((MemberReference) member).GetDisplayName (), // The cast is valid since it has to be a method or field @@ -1737,7 +1732,7 @@ static bool IsDeclaredWithinType (IMemberDefinition member, TypeDefinition type) } bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (member); - if (isReflectionAccessCoveredByDAM && !skipWarningsForOverride) { + if (isReflectionAccessCoveredByDAM) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers; Context.LogWarning (origin, id, type.GetDisplayName (), ((MemberReference) member).GetDisplayName ()); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index 654eef9300af6..945289bed6307 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -106,20 +106,16 @@ public static IEnumerable IteratorWithProblematicDataflow () [RequiresUnreferencedCode ("--RUCTypeWithIterators--")] class RUCTypeWithIterators { - [ExpectedWarning ("IL2112", "<" + nameof (StaticIteratorCallsMethodWithRequires) + ">", "(Int32)", "--RUCTypeWithIterators--", CompilerGeneratedCode = true, - ProducedBy = Tool.Trimmer)] // state machine ctor - [ExpectedWarning ("IL2112", nameof (StaticIteratorCallsMethodWithRequires) + "()", "--RUCTypeWithIterators--", + [ExpectedWarning ("IL2112", nameof (StaticIteratorCallsMethodWithRequires) + "()", ProducedBy = Tool.Trimmer)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] public static IEnumerable StaticIteratorCallsMethodWithRequires () { yield return 0; MethodWithRequires (); } - [ExpectedWarning ("IL2112", "<" + nameof (InstanceIteratorCallsMethodWithRequires) + ">", "(Int32)", "--RUCTypeWithIterators--", CompilerGeneratedCode = true, - ProducedBy = Tool.Trimmer)] // state machine ctor [ExpectedWarning ("IL2112", nameof (InstanceIteratorCallsMethodWithRequires) + "()", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] @@ -361,16 +357,12 @@ class RUCTypeWithLambdas public void MethodWithLambdas () { var lambda = - [ExpectedWarning ("IL2112", "<" + nameof (MethodWithLambdas) + ">", "--RUCTypeWithLambdas--", - ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] () => MethodWithRequires (); int i = 0; var lambdaWithCapturedState = - [ExpectedWarning ("IL2112", "<" + nameof (MethodWithLambdas) + ">", "--RUCTypeWithLambdas--", - ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] () => { @@ -494,21 +486,15 @@ class RUCTypeWithLocalFunctions ProducedBy = Tool.Trimmer)] public void MethodWithLocalFunctions () { - [ExpectedWarning ("IL2112", "<" + nameof (MethodWithLocalFunctions) + ">", - ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] void LocalFunction () => MethodWithRequires (); - [ExpectedWarning ("IL2112", "<" + nameof (MethodWithLocalFunctions) + ">", - ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] static void StaticLocalFunction () => MethodWithRequires (); int i = 0; - [ExpectedWarning ("IL2112", "<" + nameof (MethodWithLocalFunctions) + ">", - ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] void LocalFunctionWithCapturedState () diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs index 087a11bc038aa..1ded813743e0d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs @@ -277,8 +277,7 @@ class AnnotatedInterfaces : RequiredInterface { [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - // This should produce a warning: https://github.com/dotnet/linker/issues/2161 - [ExpectedWarning("IL2112", "--RUC on AnnotatedInterfaces.UnusedMethod--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2112", "--RUC on AnnotatedInterfaces.UnusedMethod--")] [RequiresUnreferencedCode ("--RUC on AnnotatedInterfaces.UnusedMethod--")] public void RUCMethod () { } } @@ -426,7 +425,7 @@ public void RUCMethod () { } [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - // shouldn't warn because we warn on the base method instead + [ExpectedWarning ("IL2112", "--AnnotatedDerivedFromBase.RUCVirtualMethod--")] [RequiresUnreferencedCode ("--AnnotatedDerivedFromBase.RUCVirtualMethod--")] public override void RUCVirtualMethod () { } @@ -438,9 +437,12 @@ public override void RUCVirtualMethod () { } [Kept] [KeptBackingField] [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - // shouldn't warn because we warn on the base getter instead [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicMethods)] - public override string DAMVirtualProperty { [Kept] get; } + public override string DAMVirtualProperty { + [Kept] + [ExpectedWarning ("IL2114", nameof (AnnotatedDerivedFromBase), nameof (DAMVirtualProperty))] + get; + } } @@ -463,7 +465,7 @@ public void RUCMethod () { } [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - // shouldn't warn because we warn on the base method instead + [ExpectedWarning ("IL2112", "--DerivedFromAnnotatedDerivedFromBase.RUCVirtualMethod--")] [RequiresUnreferencedCode ("--DerivedFromAnnotatedDerivedFromBase.RUCVirtualMethod--")] public override void RUCVirtualMethod () { } @@ -476,9 +478,12 @@ public override void RUCVirtualMethod () { } [Kept] [KeptBackingField] [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - // shouldn't warn because we warn on the base getter instead [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicMethods)] - public override string DAMVirtualProperty { [Kept] get; } + public override string DAMVirtualProperty { + [Kept] + [ExpectedWarning ("IL2114", nameof (DerivedFromAnnotatedDerivedFromBase), nameof (DAMVirtualProperty))] + get; + } } [KeptMember (".ctor()")] @@ -629,8 +634,7 @@ public class Derived : Base [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - // https://github.com/dotnet/linker/issues/2815 - [ExpectedWarning ("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] public virtual void RUCVirtualMethod () { } } @@ -669,7 +673,8 @@ public class Derived : Base [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - public override void RUCVirtualMethod () { } + [ExpectedWarning("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] + public override void RUCVirtualMethod () { } } [Kept] @@ -712,7 +717,8 @@ public class Derived : Base [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - public override void RUCVirtualMethod () { } + [ExpectedWarning("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] + public override void RUCVirtualMethod () { } } [Kept] @@ -767,7 +773,8 @@ public static void MethodWithRequires () { } [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [KeptAttributeAttribute (typeof (RequiresDynamicCodeAttribute))] [KeptAttributeAttribute (typeof (RequiresAssemblyFilesAttribute))] - [RequiresUnreferencedCode ("--Derived.VirtualMethodWithRequires--")] + [ExpectedWarning("IL2112", "--Derived.VirtualMethodWithRequires--")] + [RequiresUnreferencedCode ("--Derived.VirtualMethodWithRequires--")] [RequiresDynamicCode ("--Derived.VirtualMethodWithRequires--")] [RequiresAssemblyFiles ("--Derived.VirtualMethodWithRequires--")] public override void VirtualMethodWithRequires () { } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs index 15c0993c7a953..c345c9cf4e6b9 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs @@ -22,7 +22,6 @@ public static void Main () { TestRequiresOnlyThroughReflection (); AccessedThroughReflectionOnGenericType.Test (); - AccessedThroughGenericParameterAnnotation.Test (); AccessThroughSpecialAttribute.Test (); AccessThroughPInvoke.Test (); AccessThroughNewConstraint.Test (); @@ -74,59 +73,6 @@ public static void Test () } } - class AccessedThroughGenericParameterAnnotation - { - class TypeWithRequiresMethod - { - [RequiresUnreferencedCode("--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] - [RequiresDynamicCode ("--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] - [RequiresAssemblyFiles ("--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] - public static void MethodWhichRequires () { } - } - - class TypeWithPublicMethods<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> - { - public TypeWithPublicMethods () { } - } - - [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] - [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] - static void TestAccessOnGenericType () - { - new TypeWithPublicMethods (); - } - - static void MethodWithPublicMethods<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> () { } - - [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] - [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] - static void TestAccessOnGenericMethod () - { - MethodWithPublicMethods (); - } - - static void MethodWithPublicMethodsInference<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> (T instance) { } - - // https://github.com/dotnet/runtime/issues/86032 - // IL2026 should be produced by the analyzer as well, but it has a bug around inferred generic arguments - [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] - static void TestAccessOnGenericMethodWithInferenceOnMethod () - { - MethodWithPublicMethodsInference (new TypeWithRequiresMethod ()); - } - - public static void Test () - { - TestAccessOnGenericType (); - TestAccessOnGenericMethod (); - TestAccessOnGenericMethodWithInferenceOnMethod (); - } - } - class AccessThroughSpecialAttribute { // https://github.com/dotnet/linker/issues/1873 diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs index b3c4696a65348..fa821b58e9973 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnVirtualsAndInterfaces.cs @@ -76,9 +76,6 @@ static void TestCallOnOverrideViaBase () tmp.VirtualMethodRequires (); } - // https://github.com/dotnet/runtime/issues/86008 - // This is the "direct reflection" case, which actually behaves differently from indirect (DAM annotation) - // in this case even trimmer will warn on both methods. [ExpectedWarning ("IL2026", "--BaseType.VirtualMethodRequires--")] [ExpectedWarning ("IL3002", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] @@ -96,13 +93,12 @@ static void TestDirectReflectionAccess () typeof(T).GetMethod("VirtualMethodRequires").Invoke(instance, Array.Empty ()); } - // https://github.com/dotnet/runtime/issues/86008 [ExpectedWarning ("IL2026", "--BaseType.VirtualMethodRequires--")] [ExpectedWarning ("IL3002", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--BaseType.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] - [ExpectedWarning ("IL2026", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.Analyzer)] - //[ExpectedWarning ("IL3002", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] - //[ExpectedWarning ("IL3050", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2026", "--TypeWhichOverridesMethod.VirtualMethodRequires--")] + [ExpectedWarning ("IL3002", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--TypeWhichOverridesMethod.VirtualMethodRequires--", ProducedBy = Tool.NativeAot)] static void TestAnnotatedReflectionAccess() { CallMethodWithRequiresOnInstance(new TypeWhichOverridesMethod ()); @@ -209,9 +205,7 @@ static void TestDirectReflectionAccess () typeof (T).GetMethod ("MethodWithRequires").Invoke (new ImplementationClass (), Array.Empty ()); } - // https://github.com/dotnet/runtime/issues/86008 - // This is a bug in illink, the fact that there's no warning is an analysis hole - [ExpectedWarning ("IL2026", "--ImplementationClass.RequiresMethod--", ProducedBy = Tool.NativeAot | Tool.Analyzer)] + [ExpectedWarning ("IL2026", "--ImplementationClass.RequiresMethod--")] [ExpectedWarning ("IL3002", "--ImplementationClass.RequiresMethod--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--ImplementationClass.RequiresMethod--", ProducedBy = Tool.NativeAot)] static void TestAnnotatedReflectionAccess () diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs index ff31846f08c4a..a88250de5683d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs @@ -18,6 +18,7 @@ class RequiresViaDataflow public static void Main () { AnnotatedParameter.Test (); + AnnotatedGenericParameter.Test (); DynamicDependency.Test (); } @@ -80,6 +81,59 @@ public static void Test () } } + class AnnotatedGenericParameter + { + class TypeWithRequiresMethod + { + [RequiresUnreferencedCode ("--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] + [RequiresDynamicCode ("--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] + [RequiresAssemblyFiles ("--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] + public static void MethodWhichRequires () { } + } + + class TypeWithPublicMethods<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> + { + public TypeWithPublicMethods () { } + } + + [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] + [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] + static void TestAccessOnGenericType () + { + new TypeWithPublicMethods (); + } + + static void MethodWithPublicMethods<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> () { } + + [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] + [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] + static void TestAccessOnGenericMethod () + { + MethodWithPublicMethods (); + } + + static void MethodWithPublicMethodsInference<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> (T instance) { } + + // https://github.com/dotnet/runtime/issues/86032 + // IL2026 should be produced by the analyzer as well, but it has a bug around inferred generic arguments + [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] + static void TestAccessOnGenericMethodWithInferenceOnMethod () + { + MethodWithPublicMethodsInference (new TypeWithRequiresMethod ()); + } + + public static void Test () + { + TestAccessOnGenericType (); + TestAccessOnGenericMethod (); + TestAccessOnGenericMethodWithInferenceOnMethod (); + } + } + class DynamicDependency { [RequiresUnreferencedCode ("Message for --RequiresInDynamicDependency--")] From 3eb375b60361d2b3aac2116fb12f1a2c4a25cea5 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 19 May 2023 14:26:14 -0700 Subject: [PATCH 05/10] More tests --- ...pilerGeneratedCodeAccessedViaReflection.cs | 52 +++++++++++ .../TypeHierarchyReflectionWarnings.cs | 75 +++++++++++++++ .../RequiresAccessedThrough.cs | 92 ++++++++++++++++++- 3 files changed, 218 insertions(+), 1 deletion(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index 945289bed6307..a70cae799c9ba 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -29,6 +29,7 @@ public static void Main () LocalFunctions.Test (); SelfMarkingMethods.Test (); + DelegateAccess.Test (); } class BaseTypeWithIteratorStateMachines @@ -630,6 +631,55 @@ public static void Test () } } + class DelegateAccess + { + static void AnnotatedMethod ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) + { + } + + [ExpectedWarning ("IL2111")] + static void TestMethodThroughDelegate () + { + Action a = AnnotatedMethod; + } + + [ExpectedWarning ("IL2111")] + static void TestLambdaThroughDelegate () + { + Action a = ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) => { }; + a (null); + } + + [ExpectedWarning ("IL2111")] + static void TestLocalFunctionThroughDelegate () + { + Action a = LocalFunction; + a (null); + + void LocalFunction ([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type) + { + } + } + + static void TestGenericLocalFunctionThroughDelegate () + { + Action a = LocalFunction; + a (); + + void LocalFunction <[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> () + { + } + } + + public static void Test () + { + TestMethodThroughDelegate (); + TestLambdaThroughDelegate (); + TestLocalFunctionThroughDelegate (); + TestGenericLocalFunctionThroughDelegate (); + } + } + [RequiresUnreferencedCode ("--MethodWithRequires--")] [RequiresAssemblyFiles ("--MethodWithRequires--")] [RequiresDynamicCode ("--MethodWithRequires--")] @@ -657,5 +707,7 @@ static async Task MethodAsync () [DllImport ("Foo")] static extern void MethodTakingObject ([MarshalAs (UnmanagedType.IUnknown)] object obj); + + class TestType { } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs index 1ded813743e0d..55c815c7eb0ab 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs @@ -3,8 +3,11 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using System.Text; +using System.Threading.Tasks; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Helpers; using Mono.Linker.Tests.Cases.Expectations.Metadata; @@ -57,6 +60,8 @@ public static void Main () RUCOnVirtualOnAnnotatedBase.Test (); RUCOnVirtualOnAnnotatedBaseUsedByDerived.Test (); UseByDerived.Test (); + + CompilerGeneratedCode.Test (null); } [Kept] @@ -853,5 +858,75 @@ public static void Test () derivedInstance.GetType ().RequiresNonPublicFields (); } } + + // This validates that marking compiler generated code via DAM-on-Type doesn't + // produce warnings about the compiler generated methods, even if they're in a RUC scope. + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [KeptMember (".ctor()")] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] + class CompilerGeneratedCode + { + [Kept] + [ExpectedWarning ("IL2026", "LambdaWithRUC")] + static void LambdaWithRUC () + { + Action a = + [RequiresUnreferencedCode ("LambdaWithRUC")] + (Type type) => { type.GetMethods (); }; + } + + [Kept] + [ExpectedWarning ("IL2026", "LocalFunctionWithRUC")] + static void LocalFunctionWithRUC () + { + LocalFunctionWithRUCInner (null); + + [RequiresUnreferencedCode ("LocalFunctionWithRUC")] + void LocalFunctionWithRUCInner (Type type) + { + type.GetMethods (); + } + } + + [Kept] + [KeptAttributeAttribute (typeof(IteratorStateMachineAttribute))] + [KeptAttributeAttribute (typeof(RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode ("IteratorWithRUC")] + [ExpectedWarning ("IL2112", "IteratorWithRUC")] + static IEnumerable IteratorWithRUC () + { + yield return 1; + yield return 0; + } + + [Kept] + [KeptAttributeAttribute (typeof (AsyncStateMachineAttribute))] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [KeptAttributeAttribute (typeof(DebuggerStepThroughAttribute))] + [RequiresUnreferencedCode ("AsyncWithRUC")] + [ExpectedWarning ("IL2112", "AsyncWithRUC")] + static async Task AsyncWithRUC () + { + await Task.Delay (100); + } + + [Kept] + [KeptAttributeAttribute (typeof (AsyncIteratorStateMachineAttribute))] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode ("AsyncIteratorWithRUC")] + [ExpectedWarning ("IL2112", "AsyncIteratorWithRUC")] + static async IAsyncEnumerable AsyncIteratorWithRUC () + { + await Task.Delay (100); + yield return 1; + } + + [Kept] + public static void Test (CompilerGeneratedCode instance) + { + instance.GetType ().RequiresAll (); + } + } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs index c345c9cf4e6b9..848d2b02669a4 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAccessedThrough.cs @@ -30,6 +30,7 @@ public static void Main () AccessThroughNewConstraint.TestNewConstraintOnTypeParameterInAnnotatedMethod (); AccessThroughNewConstraint.TestNewConstraintOnTypeParameterInAnnotatedType (); AccessThroughLdToken.Test (); + AccessThroughDelegate.Test (); } class TestType { } @@ -230,10 +231,99 @@ static bool PropertyWithLdToken { [ExpectedWarning ("IL3002", "--PropertyWithLdToken.get--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--PropertyWithLdToken.get--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL3050", "--PropertyWithLdToken.get--", ProducedBy = Tool.NativeAot)] - public static void Test () + static void TestPropertyLdToken () { Expression> getter = () => PropertyWithLdToken; } + + [RequiresUnreferencedCode ("Message for --MethodWithLdToken--")] + [RequiresAssemblyFiles ("Message for --MethodWithLdToken--")] + [RequiresDynamicCode ("Message for --MethodWithLdToken--")] + static void MethodWithLdToken () + { + } + + [ExpectedWarning ("IL2026", "--MethodWithLdToken--")] + [ExpectedWarning ("IL3002", "--MethodWithLdToken--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithLdToken--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestMethodLdToken () + { + Expression e = () => MethodWithLdToken (); + } + + [RequiresUnreferencedCode ("--FieldWithLdToken--")] + [RequiresDynamicCode ("--FieldWithLdToken--")] + class FieldWithLdTokenType + { + public static int Field = 0; + } + + [ExpectedWarning ("IL2026", "--FieldWithLdToken--")] + [ExpectedWarning ("IL3050", "--FieldWithLdToken--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestFieldLdToken () + { + Expression> f = () => FieldWithLdTokenType.Field; + } + + public static void Test () + { + TestPropertyLdToken (); + TestMethodLdToken (); + TestFieldLdToken (); + } + } + + class AccessThroughDelegate + { + [RequiresUnreferencedCode ("Message for --MethodWithDelegate--")] + [RequiresAssemblyFiles ("Message for --MethodWithDelegate--")] + [RequiresDynamicCode ("Message for --MethodWithDelegate--")] + static void MethodWithDelegate () + { + } + + [ExpectedWarning ("IL2026", "--MethodWithDelegate--")] + [ExpectedWarning ("IL3002", "--MethodWithDelegate--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithDelegate--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestMethodWithDelegate () + { + Action a = MethodWithDelegate; + } + + [ExpectedWarning ("IL2026", "--LambdaThroughDelegate--")] + [ExpectedWarning ("IL3002", "--LambdaThroughDelegate--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--LambdaThroughDelegate--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void LambdaThroughDelegate () + { + Action a = + [RequiresUnreferencedCode ("--LambdaThroughDelegate--")] + [RequiresAssemblyFiles ("--LambdaThroughDelegate--")] + [RequiresDynamicCode ("--LambdaThroughDelegate--")] + () => { }; + + a (); + } + + [ExpectedWarning ("IL2026", "--LocalFunctionThroughDelegate--")] + [ExpectedWarning ("IL3002", "--LocalFunctionThroughDelegate--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--LocalFunctionThroughDelegate--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void LocalFunctionThroughDelegate () + { + Action a = Local; + + [RequiresUnreferencedCode ("--LocalFunctionThroughDelegate--")] + [RequiresAssemblyFiles ("--LocalFunctionThroughDelegate--")] + [RequiresDynamicCode ("--LocalFunctionThroughDelegate--")] + void Local () + { } + } + + public static void Test () + { + TestMethodWithDelegate (); + LambdaThroughDelegate (); + LocalFunctionThroughDelegate (); + } } } } From f5bad2e05f03ae5456012645789fa0cfb45dbb8b Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 19 May 2023 14:49:17 -0700 Subject: [PATCH 06/10] Fix an analyzer test --- .../DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index a70cae799c9ba..e4ffb469ce9b1 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -637,20 +637,20 @@ static void AnnotatedMethod ([DynamicallyAccessedMembers (DynamicallyAccessedMem { } - [ExpectedWarning ("IL2111")] + [ExpectedWarning ("IL2111", ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void TestMethodThroughDelegate () { Action a = AnnotatedMethod; } - [ExpectedWarning ("IL2111")] + [ExpectedWarning ("IL2111", ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void TestLambdaThroughDelegate () { Action a = ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) => { }; a (null); } - [ExpectedWarning ("IL2111")] + [ExpectedWarning ("IL2111", ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void TestLocalFunctionThroughDelegate () { Action a = LocalFunction; From 6ab425a684a4968b83567245131984fd47de2907 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Mon, 22 May 2023 04:45:34 -0700 Subject: [PATCH 07/10] Fix type hierarchy marking in AOT to match linker --- .../Compiler/Dataflow/ReflectionMarker.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs index a5f210e5367fb..eb9ffa0b3a365 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs @@ -317,7 +317,8 @@ static bool IsDeclaredWithinType(TypeSystemEntity member, TypeDesc type) // causing problems is pretty low. bool isReflectionAccessCoveredByRUC = _logger.ShouldSuppressAnalysisWarningsForRequires(entity, DiagnosticUtilities.RequiresUnreferencedCodeAttribute, out CustomAttributeValue? requiresUnreferencedCodeAttribute); - if (isReflectionAccessCoveredByRUC && !ShouldSkipWarningsForOverride(entity)) + bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember(entity); + if (isReflectionAccessCoveredByRUC && !isCompilerGenerated) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithRequiresUnreferencedCode : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithRequiresUnreferencedCode; _logger.LogWarning(origin, id, _typeHierarchyDataFlowOrigin.GetDisplayName(), @@ -327,7 +328,7 @@ static bool IsDeclaredWithinType(TypeSystemEntity member, TypeDesc type) } bool isReflectionAccessCoveredByDAM = Annotations.ShouldWarnWhenAccessedForReflection(entity); - if (isReflectionAccessCoveredByDAM && !ShouldSkipWarningsForOverride(entity)) + if (isReflectionAccessCoveredByDAM) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers; _logger.LogWarning(origin, id, _typeHierarchyDataFlowOrigin.GetDisplayName(), entity.GetDisplayName()); @@ -335,17 +336,6 @@ static bool IsDeclaredWithinType(TypeSystemEntity member, TypeDesc type) // We decided to not warn on reflection access to compiler-generated methods: // https://github.com/dotnet/runtime/issues/85042 - - // All override methods should have the same annotations as their base methods - // (else we will produce warning IL2046 or IL2092 or some other warning). - // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. - static bool ShouldSkipWarningsForOverride(TypeSystemEntity entity) - { - if (entity is not MethodDesc method || !method.IsVirtual) - return false; - - return MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(method) != method; - } } private void ReportRequires(in MessageOrigin origin, TypeSystemEntity entity, string requiresAttributeName, in CustomAttributeValue requiresAttribute) From 4283ff7c20fc3783e06aaa503f68d24912b9e7da Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 23 May 2023 04:43:21 -0700 Subject: [PATCH 08/10] Add some tests --- ...flectionAccessFromCompilerGeneratedCode.cs | 2 +- .../RequiresInCompilerGeneratedCode.cs | 21 ++++++++++++++++ .../RequiresCapability/RequiresOnClass.cs | 24 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs index 2e8ae0d476c61..6aa4bc0e78370 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs @@ -73,7 +73,7 @@ static async void TestAsyncWithRUC () [ExpectedWarning ("IL3050", "--TestAsyncWithRUC--", ProducedBy = Tool.NativeAot | Tool.Analyzer)] public static void Test () { - TestIterator ().GetEnumerator ().MoveNext (); // Must actually une the enumerator, otherwise NativeAOT will trim the implementation + TestIterator ().GetEnumerator ().MoveNext (); // Must actually use the enumerator, otherwise NativeAOT will trim the implementation TestIteratorWithRUC (); TestAsync (); TestAsyncWithRUC (); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs index 06a7b319cd8c1..37c9da005fb07 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs @@ -44,6 +44,8 @@ public static void Main () ComplexCases.AsyncBodyCallingMethodWithRequires.Test (); ComplexCases.GenericAsyncBodyCallingMethodWithRequires.Test (); ComplexCases.GenericAsyncEnumerableBodyCallingRequiresWithAnnotations.Test (); + + RUCOnDelegateCacheFields.Test (); } class WarnInIteratorBody @@ -2215,6 +2217,25 @@ class Disposable : IDisposable { public void Dispose () { } } static Task GetDisposableAsync () { return Task.FromResult (new Disposable ()); } } + class RUCOnDelegateCacheFields + { + [RequiresUnreferencedCode ("--RUCOnDelegateCacheFields.TargetType--")] + class TargetType + { + public static void Init () + { + var Action = () => { }; + } + } + + [ExpectedWarning ("IL2026", nameof (TargetType) + "()")] // .ctor + [ExpectedWarning ("IL2026", nameof (TargetType.Init) + "()")] // Init method + public static void Test () + { + typeof (TargetType).RequiresAll (); + } + } + static async Task MethodAsync () { return await Task.FromResult (0); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs index b2c3398e12937..24faa7879210f 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs @@ -808,6 +808,28 @@ static void TestDAMOnTypeAccessInRUCScope (DAMAnnotatedClassAccessedFromRUCScope instance.GetType ().GetMethod ("RUCMethod"); } + [RequiresUnreferencedCode ("--GenericTypeWithRequires--")] + [RequiresDynamicCode ("--GenericTypeWithRequires--")] + class GenericTypeWithRequires + { + public static int NonGenericField; + } + + // https://github.com/dotnet/runtime/issues/86633 - analyzer doesn't report this warning + [ExpectedWarning ("IL2026", "NonGenericField", "--GenericTypeWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "NonGenericField", "--GenericTypeWithRequires--", ProducedBy = Tool.NativeAot)] + static void TestDAMAccessOnOpenGeneric () + { + typeof (GenericTypeWithRequires<>).RequiresPublicFields (); + } + + [ExpectedWarning ("IL2026", "NonGenericField", "--GenericTypeWithRequires--")] + [ExpectedWarning ("IL3050", "NonGenericField", "--GenericTypeWithRequires--", ProducedBy = Tool.NativeAot)] + static void TestDAMAccessOnInstantiatedGeneric () + { + typeof (GenericTypeWithRequires).RequiresPublicFields (); + } + [ExpectedWarning ("IL2026", "--TestDAMOnTypeAccessInRUCScope--")] public static void Test () { @@ -816,6 +838,8 @@ public static void Test () TestDynamicDependencyAccess (); TestDAMOnTypeAccess (null); TestDAMOnTypeAccessInRUCScope (); + TestDAMAccessOnOpenGeneric (); + TestDAMAccessOnInstantiatedGeneric (); } } From 3d610d9d43499f6202fc4057950ac6266a229b35 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 26 May 2023 14:32:44 -0700 Subject: [PATCH 09/10] Enable a test --- ...pilerGeneratedCodeAccessedViaReflection.cs | 75 +++++++++---------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index e4ffb469ce9b1..9ed65fd9f210e 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -15,7 +15,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow { - [IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)] [SkipKeptItemsValidation] [ExpectedNoWarnings] class CompilerGeneratedCodeAccessedViaReflection @@ -56,9 +55,9 @@ public static IEnumerable IteratorWithoutDataflow () [ExpectedWarning ("IL2026", "--MethodWithRequires--", CompilerGeneratedCode = true)] [ExpectedWarning ("IL3002", "--MethodWithRequires--", - ProducedBy = Tool.Analyzer)] + ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] [ExpectedWarning ("IL3050", "--MethodWithRequires--", - ProducedBy = Tool.Analyzer)] + ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] [ExpectedWarning ("IL2119", "<" + nameof (IteratorCallsMethodWithRequires) + ">", "MoveNext", CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer)] public static IEnumerable IteratorCallsMethodWithRequires () @@ -103,14 +102,14 @@ public static IEnumerable IteratorWithProblematicDataflow () } [ExpectedWarning ("IL2112", nameof (RUCTypeWithIterators) + "()", "--RUCTypeWithIterators--", CompilerGeneratedCode = true, - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] // warning about .ctor [RequiresUnreferencedCode ("--RUCTypeWithIterators--")] class RUCTypeWithIterators { [ExpectedWarning ("IL2112", nameof (StaticIteratorCallsMethodWithRequires) + "()", - ProducedBy = Tool.Trimmer)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] public static IEnumerable StaticIteratorCallsMethodWithRequires () { yield return 0; @@ -118,9 +117,9 @@ public static IEnumerable StaticIteratorCallsMethodWithRequires () } [ExpectedWarning ("IL2112", nameof (InstanceIteratorCallsMethodWithRequires) + "()", - ProducedBy = Tool.Trimmer)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] public IEnumerable InstanceIteratorCallsMethodWithRequires () { yield return 0; @@ -141,7 +140,7 @@ public IEnumerable InstanceIteratorCallsMethodWithRequires () [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators) + "()", "--RUCTypeWithIterators--")] // Expect to see warnings about RUC on type, for all static state machine members. [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators.StaticIteratorCallsMethodWithRequires) + "()", "--RUCTypeWithIterators--")] - [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators.InstanceIteratorCallsMethodWithRequires) + "()", ProducedBy = Tool.Trimmer | Tool.Analyzer)] + [ExpectedWarning ("IL2026", nameof (RUCTypeWithIterators.InstanceIteratorCallsMethodWithRequires) + "()")] [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithCorrectDataflow) + ">", "", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (IteratorWithProblematicDataflow) + ">", "", @@ -169,8 +168,8 @@ public static async Task AsyncWithoutDataflow () } [ExpectedWarning ("IL2026", "--MethodWithRequires--", CompilerGeneratedCode = true)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] public static async Task AsyncCallsMethodWithRequires () { MethodWithRequires (); @@ -213,8 +212,8 @@ public static async IAsyncEnumerable AsyncIteratorWithoutDataflow () } [ExpectedWarning ("IL2026", "--MethodWithRequires--", CompilerGeneratedCode = true)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot, CompilerGeneratedCode = true)] public static async IAsyncEnumerable AsyncIteratorCallsMethodWithRequires () { yield return await MethodAsync (); @@ -265,8 +264,8 @@ static void LambdaCallsMethodWithRequires () { var lambda = [ExpectedWarning ("IL2026", "--MethodWithRequires--")] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL2119", "<" + nameof (LambdaCallsMethodWithRequires) + ">", ProducedBy = Tool.Trimmer)] () => MethodWithRequires (); @@ -286,12 +285,12 @@ static void LambdaWithCorrectDataflow () } [ExpectedWarning ("IL2111", "<" + nameof (LambdaWithCorrectParameter) + ">", - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void LambdaWithCorrectParameter () { var lambda = [ExpectedWarning ("IL2114", "<" + nameof (LambdaWithCorrectParameter) + ">", - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] ([DynamicallyAccessedMembersAttribute (DynamicallyAccessedMemberTypes.All)] Type t) => { t.RequiresAll (); }; @@ -350,22 +349,22 @@ static void LambdaCallsPInvokeTakingObject () } [ExpectedWarning ("IL2112", nameof (RUCTypeWithLambdas) + "()", "--RUCTypeWithLambdas--", CompilerGeneratedCode = true, - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] [RequiresUnreferencedCode ("--RUCTypeWithLambdas--")] class RUCTypeWithLambdas { - [ExpectedWarning ("IL2112", nameof (MethodWithLambdas), "--RUCTypeWithLambdas--", ProducedBy = Tool.Trimmer)] + [ExpectedWarning ("IL2112", nameof (MethodWithLambdas), "--RUCTypeWithLambdas--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] public void MethodWithLambdas () { var lambda = - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] () => MethodWithRequires (); int i = 0; var lambdaWithCapturedState = - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] () => { i++; MethodWithRequires (); @@ -383,13 +382,13 @@ public void MethodWithLambdas () [ExpectedWarning ("IL2118", "<" + nameof (LambdaWithCorrectDataflow) + ">", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2111", "<" + nameof (LambdaWithCorrectParameter) + ">", - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] [ExpectedWarning ("IL2118", "<" + nameof (LambdaWithProblematicDataflow) + ">", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (LambdaWithCapturedTypeToDAM) + ">", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2026", nameof (RUCTypeWithLambdas) + "()", "--RUCTypeWithLambdas--")] - [ExpectedWarning ("IL2026", nameof (RUCTypeWithLambdas.MethodWithLambdas) + "()", "--RUCTypeWithLambdas--", ProducedBy = Tool.Trimmer | Tool.Analyzer)] + [ExpectedWarning ("IL2026", nameof (RUCTypeWithLambdas.MethodWithLambdas) + "()", "--RUCTypeWithLambdas--")] public static void Test (Lambdas test = null) { typeof (Lambdas).RequiresAll (); @@ -410,8 +409,8 @@ static void LocalFunctionWithoutDataflow () static void LocalFunctionCallsMethodWithRequires () { [ExpectedWarning ("IL2026", "--MethodWithRequires--")] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL2119", "<" + nameof (LocalFunctionCallsMethodWithRequires) + ">", ProducedBy = Tool.Trimmer)] void LocalFunction () => MethodWithRequires (); @@ -479,25 +478,25 @@ static void LocalFunctionCallsPInvokeTakingObject () } [ExpectedWarning ("IL2112", nameof (RUCTypeWithLocalFunctions) + "()", CompilerGeneratedCode = true, - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] [RequiresUnreferencedCode ("--RUCTypeWithLocalFunctions--")] class RUCTypeWithLocalFunctions { [ExpectedWarning ("IL2112", nameof (MethodWithLocalFunctions), "--RUCTypeWithLocalFunctions--", - ProducedBy = Tool.Trimmer)] + ProducedBy = Tool.Trimmer | Tool.NativeAot)] public void MethodWithLocalFunctions () { - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] void LocalFunction () => MethodWithRequires (); - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] static void StaticLocalFunction () => MethodWithRequires (); int i = 0; - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] void LocalFunctionWithCapturedState () { i++; @@ -522,7 +521,7 @@ void LocalFunctionWithCapturedState () ProducedBy = Tool.Trimmer)] // Expect RUC warnings for static, compiler-generated code warnings for instance. [ExpectedWarning ("IL2026", nameof (RUCTypeWithLocalFunctions) + "()", "--RUCTypeWithLocalFunctions--")] - [ExpectedWarning ("IL2026", nameof (RUCTypeWithLocalFunctions.MethodWithLocalFunctions) + "()", ProducedBy = Tool.Trimmer | Tool.Analyzer)] + [ExpectedWarning ("IL2026", nameof (RUCTypeWithLocalFunctions.MethodWithLocalFunctions) + "()")] public static void Test (LocalFunctions test = null) { typeof (LocalFunctions).RequiresAll (); From 8bdbe179f502bc1c753ee8c2764119f47bd44708 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 2 Jun 2023 14:31:07 -0700 Subject: [PATCH 10/10] Don't produce warnings when accessing compiler generated code via DAM annotations (not on type). --- .../Compiler/Dataflow/ReflectionMarker.cs | 8 +- .../src/linker/Linker.Steps/MarkStep.cs | 4 +- ...pilerGeneratedCodeAccessedViaReflection.cs | 72 +++++++++++++- .../TypeHierarchyReflectionWarnings.cs | 98 ++++++++++++++++--- .../TestCasesRunner/AssemblyChecker.cs | 19 ++-- 5 files changed, 170 insertions(+), 31 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs index eb9ffa0b3a365..c2132ce7aa6d1 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs @@ -256,7 +256,7 @@ private void ReportWarningsForReflectionAccess(in MessageOrigin origin, TypeSyst return; bool isReflectionAccessCoveredByDAM = Annotations.ShouldWarnWhenAccessedForReflection(entity); - if (isReflectionAccessCoveredByDAM) + if (isReflectionAccessCoveredByDAM && ShouldProduceRequiresWarningForReflectionAccess(entity, accessKind)) { if (entity is MethodDesc) _logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, entity.GetDisplayName()); @@ -267,10 +267,6 @@ private void ReportWarningsForReflectionAccess(in MessageOrigin origin, TypeSyst // We decided to not warn on reflection access to compiler-generated methods: // https://github.com/dotnet/runtime/issues/85042 - // All override methods should have the same annotations as their base methods - // (else we will produce warning IL2046 or IL2092 or some other warning). - // When marking override methods via DynamicallyAccessedMembers, we should only issue a warning for the base method. - // PERF: Avoid precomputing this as this method is relatively expensive. Only call it once we're about to produce a warning. static bool ShouldProduceRequiresWarningForReflectionAccess(TypeSystemEntity entity, AccessKind accessKind) { bool isCompilerGenerated = CompilerGeneratedState.IsNestedFunctionOrStateMachineMember(entity); @@ -328,7 +324,7 @@ static bool IsDeclaredWithinType(TypeSystemEntity member, TypeDesc type) } bool isReflectionAccessCoveredByDAM = Annotations.ShouldWarnWhenAccessedForReflection(entity); - if (isReflectionAccessCoveredByDAM) + if (isReflectionAccessCoveredByDAM && !isCompilerGenerated) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers; _logger.LogWarning(origin, id, _typeHierarchyDataFlowOrigin.GetDisplayName(), entity.GetDisplayName()); diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 112e94a248ca3..b6376efeef6f5 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1668,7 +1668,7 @@ void ReportWarningsForReflectionAccess (in MessageOrigin origin, MethodDefinitio ReportRequiresUnreferencedCode (method.GetDisplayName (), requiresUnreferencedCode!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context)); bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (method); - if (isReflectionAccessCoveredByDAM) { + if (isReflectionAccessCoveredByDAM && (!isCompilerGenerated || forceRUCCheck)) { // ReflectionMethodBodyScanner handles more cases for data flow annotations // so don't warn for those. switch (dependencyKind) { @@ -1732,7 +1732,7 @@ static bool IsDeclaredWithinType (IMemberDefinition member, TypeDefinition type) } bool isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (member); - if (isReflectionAccessCoveredByDAM) { + if (isReflectionAccessCoveredByDAM && !isCompilerGenerated) { var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers; Context.LogWarning (origin, id, type.GetDisplayName (), ((MemberReference) member).GetDisplayName ()); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs index 9ed65fd9f210e..abe939c2322b2 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs @@ -29,6 +29,8 @@ public static void Main () SelfMarkingMethods.Test (); DelegateAccess.Test (); + + DAMReflectionAccessToCompilerGeneratedCode.Test (); } class BaseTypeWithIteratorStateMachines @@ -289,8 +291,6 @@ static void LambdaWithCorrectDataflow () static void LambdaWithCorrectParameter () { var lambda = - [ExpectedWarning ("IL2114", "<" + nameof (LambdaWithCorrectParameter) + ">", - ProducedBy = Tool.Trimmer | Tool.NativeAot)] ([DynamicallyAccessedMembersAttribute (DynamicallyAccessedMemberTypes.All)] Type t) => { t.RequiresAll (); }; @@ -381,8 +381,6 @@ public void MethodWithLambdas () ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (LambdaWithCorrectDataflow) + ">", ProducedBy = Tool.Trimmer)] - [ExpectedWarning ("IL2111", "<" + nameof (LambdaWithCorrectParameter) + ">", - ProducedBy = Tool.Trimmer | Tool.NativeAot)] [ExpectedWarning ("IL2118", "<" + nameof (LambdaWithProblematicDataflow) + ">", ProducedBy = Tool.Trimmer)] [ExpectedWarning ("IL2118", "<" + nameof (LambdaWithCapturedTypeToDAM) + ">", @@ -679,6 +677,72 @@ public static void Test () } } + class DAMReflectionAccessToCompilerGeneratedCode + { + // ldftn access - this MUST warn since the action can be called without the annotation + [ExpectedWarning ("IL2111", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + static void Lambda () + { + Action a = ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) => { + type.GetMethods (); + }; + + a (typeof (string)); + } + + static void LambdaOnGeneric<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T> () + { + Action a = () => { + typeof(T).GetMethods (); + }; + + a (); + } + + static void LocalFunction () + { + LocalFunctionInner (typeof (string)); + + static void LocalFunctionInner ([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type) + { + type.GetMethods (); + } + } + + static void LocalFunctionOnGeneric<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>() + { + LocalFunctionInner (); + + static void LocalFunctionInner () + { + typeof(T).GetMethods (); + } + } + + static IEnumerable IteratorOnGeneric<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>() + { + yield return 0; + } + + static async Task AsyncOnGeneric<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> () + { + await Task.Delay (100); + } + + static async IAsyncEnumerable AsyncIteratorOnGeneric<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> () + { + yield return 0; + await Task.Delay (100); + } + + [ExpectedWarning ("IL2118", "<" + nameof (LambdaOnGeneric) + ">", ProducedBy = Tool.Trimmer)] + [ExpectedWarning ("IL2118", "<" + nameof (LocalFunctionOnGeneric) + ">", ProducedBy = Tool.Trimmer)] + public static void Test () + { + typeof (DAMReflectionAccessToCompilerGeneratedCode).RequiresAll (); + } + } + [RequiresUnreferencedCode ("--MethodWithRequires--")] [RequiresAssemblyFiles ("--MethodWithRequires--")] [RequiresDynamicCode ("--MethodWithRequires--")] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs index 55c815c7eb0ab..6c37b7dd3fe12 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Reflection; using System.Runtime.CompilerServices; using System.Text; using System.Threading.Tasks; @@ -61,7 +62,8 @@ public static void Main () RUCOnVirtualOnAnnotatedBaseUsedByDerived.Test (); UseByDerived.Test (); - CompilerGeneratedCode.Test (null); + CompilerGeneratedCodeRUC.Test (null); + CompilerGeneratedCodeDAM.Test (null); } [Kept] @@ -678,8 +680,8 @@ public class Derived : Base [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - [ExpectedWarning("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - public override void RUCVirtualMethod () { } + [ExpectedWarning ("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] + public override void RUCVirtualMethod () { } } [Kept] @@ -722,8 +724,8 @@ public class Derived : Base [Kept] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - [ExpectedWarning("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] - public override void RUCVirtualMethod () { } + [ExpectedWarning ("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")] + public override void RUCVirtualMethod () { } } [Kept] @@ -778,8 +780,8 @@ public static void MethodWithRequires () { } [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [KeptAttributeAttribute (typeof (RequiresDynamicCodeAttribute))] [KeptAttributeAttribute (typeof (RequiresAssemblyFilesAttribute))] - [ExpectedWarning("IL2112", "--Derived.VirtualMethodWithRequires--")] - [RequiresUnreferencedCode ("--Derived.VirtualMethodWithRequires--")] + [ExpectedWarning ("IL2112", "--Derived.VirtualMethodWithRequires--")] + [RequiresUnreferencedCode ("--Derived.VirtualMethodWithRequires--")] [RequiresDynamicCode ("--Derived.VirtualMethodWithRequires--")] [RequiresAssemblyFiles ("--Derived.VirtualMethodWithRequires--")] public override void VirtualMethodWithRequires () { } @@ -865,7 +867,7 @@ public static void Test () [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [KeptMember (".ctor()")] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] - class CompilerGeneratedCode + class CompilerGeneratedCodeRUC { [Kept] [ExpectedWarning ("IL2026", "LambdaWithRUC")] @@ -874,6 +876,8 @@ static void LambdaWithRUC () Action a = [RequiresUnreferencedCode ("LambdaWithRUC")] (Type type) => { type.GetMethods (); }; + + a (typeof (string)); } [Kept] @@ -890,8 +894,8 @@ void LocalFunctionWithRUCInner (Type type) } [Kept] - [KeptAttributeAttribute (typeof(IteratorStateMachineAttribute))] - [KeptAttributeAttribute (typeof(RequiresUnreferencedCodeAttribute))] + [KeptAttributeAttribute (typeof (IteratorStateMachineAttribute))] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode ("IteratorWithRUC")] [ExpectedWarning ("IL2112", "IteratorWithRUC")] static IEnumerable IteratorWithRUC () @@ -903,7 +907,7 @@ static IEnumerable IteratorWithRUC () [Kept] [KeptAttributeAttribute (typeof (AsyncStateMachineAttribute))] [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - [KeptAttributeAttribute (typeof(DebuggerStepThroughAttribute))] + [KeptAttributeAttribute (typeof (DebuggerStepThroughAttribute))] [RequiresUnreferencedCode ("AsyncWithRUC")] [ExpectedWarning ("IL2112", "AsyncWithRUC")] static async Task AsyncWithRUC () @@ -923,7 +927,77 @@ static async IAsyncEnumerable AsyncIteratorWithRUC () } [Kept] - public static void Test (CompilerGeneratedCode instance) + public static void Test (CompilerGeneratedCodeRUC instance) + { + instance.GetType ().RequiresAll (); + } + } + + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [KeptMember (".ctor()")] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] + class CompilerGeneratedCodeDAM + { + [Kept] + [ExpectedWarning ("IL2111", nameof (LambdaWithDAM))] + static void LambdaWithDAM () + { + Action a = + ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) => { type.GetMethods (); }; + + a (typeof (string)); + } + + [Kept] + static void LocalFunctionWithDAM () + { + LocalFunctionWithDAMInner (typeof (string)); + + static void LocalFunctionWithDAMInner ([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type) + { + type.GetMethods (); + } + } + + [Kept] + [KeptAttributeAttribute (typeof (IteratorStateMachineAttribute))] + [ExpectedWarning ("IL2119", nameof (IteratorWithGenericDAM), CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer)] + static IEnumerable IteratorWithGenericDAM< + [KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T> () + { + foreach (MethodInfo m in typeof (T).GetMethods ()) + yield return m.IsPublic; + } + + [Kept] + [KeptAttributeAttribute (typeof (AsyncStateMachineAttribute))] + [KeptAttributeAttribute (typeof (DebuggerStepThroughAttribute))] + [ExpectedWarning ("IL2119", nameof (AsyncWithGenericDAM), CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer)] + static async Task AsyncWithGenericDAM< + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>() + { + await Task.Delay (100); + typeof (T).GetMethods (); + } + + [Kept] + [KeptAttributeAttribute (typeof (AsyncIteratorStateMachineAttribute))] + [ExpectedWarning("IL2119", nameof(AsyncIteratorWithGenericDAM), CompilerGeneratedCode = true, ProducedBy = Tool.Trimmer)] + static async IAsyncEnumerable AsyncIteratorWithGenericDAM< + [KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>() + { + foreach (MethodInfo m in typeof (T).GetMethods ()) { + await Task.Delay (100); + yield return m.IsPublic; + } + } + + [Kept] + public static void Test (CompilerGeneratedCodeDAM instance) { instance.GetType ().RequiresAll (); } diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index 363c089f67f69..9431505a6ec22 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -248,7 +248,7 @@ protected virtual void VerifyTypeDefinitionKept (TypeDefinition original, TypeDe VerifyInterfaces (original, linked); VerifyPseudoAttributes (original, linked); - VerifyGenericParameters (original, linked); + VerifyGenericParameters (original, linked, compilerGenerated: false); VerifyCustomAttributes (original, linked); VerifySecurityAttributes (original, linked); @@ -537,12 +537,12 @@ protected virtual void VerifyMethodKept (MethodDefinition src, MethodDefinition Assert.Fail ($"Method `{src.FullName}' should have been kept"); VerifyPseudoAttributes (src, linked); - VerifyGenericParameters (src, linked); + VerifyGenericParameters (src, linked, compilerGenerated); if (!compilerGenerated) { VerifyCustomAttributes (src, linked); VerifyCustomAttributes (src.MethodReturnType, linked.MethodReturnType); } - VerifyParameters (src, linked); + VerifyParameters (src, linked, compilerGenerated); VerifySecurityAttributes (src, linked); VerifyArrayInitializers (src, linked); VerifyMethodBody (src, linked); @@ -1056,7 +1056,7 @@ void VerifyDelegateBackingFields (TypeDefinition src, TypeDefinition linked) } } - void VerifyGenericParameters (IGenericParameterProvider src, IGenericParameterProvider linked) + void VerifyGenericParameters (IGenericParameterProvider src, IGenericParameterProvider linked, bool compilerGenerated) { Assert.AreEqual (src.HasGenericParameters, linked.HasGenericParameters); if (src.HasGenericParameters) { @@ -1064,7 +1064,10 @@ void VerifyGenericParameters (IGenericParameterProvider src, IGenericParameterPr // TODO: Verify constraints var srcp = src.GenericParameters[i]; var lnkp = linked.GenericParameters[i]; - VerifyCustomAttributes (srcp, lnkp); + + if (!compilerGenerated) { + VerifyCustomAttributes (srcp, lnkp); + } if (checkNames) { if (srcp.CustomAttributes.Any (attr => attr.AttributeType.Name == nameof (RemovedNameValueAttribute))) { @@ -1078,7 +1081,7 @@ void VerifyGenericParameters (IGenericParameterProvider src, IGenericParameterPr } } - void VerifyParameters (IMethodSignature src, IMethodSignature linked) + void VerifyParameters (IMethodSignature src, IMethodSignature linked, bool compilerGenerated) { Assert.AreEqual (src.HasParameters, linked.HasParameters); if (src.HasParameters) { @@ -1086,7 +1089,9 @@ void VerifyParameters (IMethodSignature src, IMethodSignature linked) var srcp = src.Parameters[i]; var lnkp = linked.Parameters[i]; - VerifyCustomAttributes (srcp, lnkp); + if (!compilerGenerated) { + VerifyCustomAttributes (srcp, lnkp); + } if (checkNames) { if (srcp.CustomAttributes.Any (attr => attr.AttributeType.Name == nameof (RemovedNameValueAttribute)))