From 7b645f78ff7c48bf1d2636965ec27caba3b1d46b Mon Sep 17 00:00:00 2001 From: Vitek Karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 4 Apr 2023 02:44:28 -0700 Subject: [PATCH] Static cctor analysis for reflection (#84089) This brings the NativeAOT behavior around static cctor analysis on part with illink. Main change is to add checks for RUC/RAF/RDC on static cctor and produces a warning if there is one (as these are not allowed on static cctor). The rest of the changes are just minor fixes and adjusting tests to the new behavior. Contributes to https://github.com/dotnet/runtime/issues/82447. --- .../DynamicallyAccessedMembersBinder.cs | 2 +- .../Compiler/Dataflow/ReflectionMarker.cs | 3 ++ .../Compiler/UsageBasedMetadataManager.cs | 13 +++++++ .../RequiresOnStaticConstructor.cs | 39 ++++++++++++++----- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs b/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs index 59c3cc6848111..6780cb5619289 100644 --- a/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs @@ -133,7 +133,7 @@ public static IEnumerable GetConstructorsOnType(this TypeDesc type, foreach (var method in type.GetMethods()) { - if (!method.IsConstructor) + if (!method.IsConstructor && !method.IsStaticConstructor) continue; if (filter != null && !filter(method)) 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 c61bd273a0f6f..4ecabf6aad6d6 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs @@ -198,6 +198,9 @@ internal void MarkStaticConstructor(in MessageOrigin origin, TypeDesc type, stri if (!_enabled) return; + if (type.HasStaticConstructor) + CheckAndWarnOnReflectionAccess(origin, type.GetStaticConstructor()); + if (!type.IsGenericDefinition && !type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true) && Factory.PreinitializationManager.HasLazyStaticConstructor(type)) { // Mark the GC static base - it contains a pointer to the class constructor, but also info diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index bfb48d1b27600..883d077a9bcc4 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -11,6 +11,7 @@ using ILCompiler.Dataflow; using ILCompiler.DependencyAnalysis; using ILCompiler.DependencyAnalysisFramework; +using ILCompiler.Logging; using ILCompiler.Metadata; using ILLink.Shared; @@ -560,6 +561,18 @@ protected override void GetDependenciesDueToMethodCodePresenceInternal(ref Depen { AddDataflowDependency(ref dependencies, factory, methodIL, "Method has annotated parameters"); } + + if (method.IsStaticConstructor) + { + if (DiagnosticUtilities.TryGetRequiresAttribute(method, DiagnosticUtilities.RequiresUnreferencedCodeAttribute, out _)) + Logger.LogWarning(new MessageOrigin(method), DiagnosticId.RequiresUnreferencedCodeOnStaticConstructor, method.GetDisplayName()); + + if (DiagnosticUtilities.TryGetRequiresAttribute(method, DiagnosticUtilities.RequiresDynamicCodeAttribute, out _)) + Logger.LogWarning(new MessageOrigin(method), DiagnosticId.RequiresDynamicCodeOnStaticConstructor, method.GetDisplayName()); + + if (DiagnosticUtilities.TryGetRequiresAttribute(method, DiagnosticUtilities.RequiresAssemblyFilesAttribute, out _)) + Logger.LogWarning(new MessageOrigin(method), DiagnosticId.RequiresAssemblyFilesOnStaticConstructor, method.GetDisplayName()); + } } if (method.GetTypicalMethodDefinition() is Internal.TypeSystem.Ecma.EcmaMethod ecmaMethod) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnStaticConstructor.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnStaticConstructor.cs index 5532bd60b91cf..c8f059bdc0596 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnStaticConstructor.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnStaticConstructor.cs @@ -14,7 +14,6 @@ namespace Mono.Linker.Tests.Cases.RequiresCapability { - [IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)] [SkipKeptItemsValidation] [ExpectedNoWarnings] class RequiresOnStaticConstructor @@ -28,14 +27,15 @@ public static void Main () TestTypeIsBeforeFieldInit (); TestStaticCtorOnTypeWithRequires (); TestRunClassConstructorOnTypeWithRequires (); + TestRunClassConstructorOnConstructorWithRequires (); typeof (StaticCtor).RequiresNonPublicConstructors (); } class StaticCtor { [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 ("IL2116", "StaticCtor..cctor()")] [RequiresUnreferencedCode ("Message for --TestStaticCtor--")] static StaticCtor () @@ -52,8 +52,8 @@ static void TestStaticCctorRequires () [RequiresUnreferencedCode ("Message for --StaticCtorOnTypeWithRequires--")] class StaticCtorOnTypeWithRequires { - [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 StaticCtorOnTypeWithRequires () => MethodWithRequires (); } @@ -71,6 +71,23 @@ static void TestRunClassConstructorOnTypeWithRequires () RuntimeHelpers.RunClassConstructor (typeHandle); } + class StaticCtorForRunClassConstructorWithRequires + { + [ExpectedWarning ("IL2116")] + [ExpectedWarning ("IL3004", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3056", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [RequiresUnreferencedCode ("Message for --StaticCtorOnTypeWithRequires--")] + [RequiresAssemblyFiles ("Message for --StaticCtorOnTypeWithRequires--")] + [RequiresDynamicCode ("Message for --StaticCtorOnTypeWithRequires--")] + static StaticCtorForRunClassConstructorWithRequires () { } + } + + static void TestRunClassConstructorOnConstructorWithRequires () + { + // No warnings generated here - we rely on IL2116/IL3004/IL3056 in this case and avoid duplicate warnings + RuntimeHelpers.RunClassConstructor (typeof (StaticCtorForRunClassConstructorWithRequires).TypeHandle); + } + class StaticCtorTriggeredByFieldAccess { [ExpectedWarning ("IL2116", "StaticCtorTriggeredByFieldAccess..cctor()")] @@ -105,8 +122,8 @@ static void TestStaticCtorMarkingIsTriggeredByFieldAccessOnExplicitLayout () class StaticCtorTriggeredByMethodCall { [ExpectedWarning ("IL2116", "StaticCtorTriggeredByMethodCall..cctor()")] - [ExpectedWarning ("IL3004", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3056", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3004", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3056", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [RequiresUnreferencedCode ("Message for --StaticCtorTriggeredByMethodCall.Cctor--")] [RequiresAssemblyFiles ("Message for --StaticCtorTriggeredByMethodCall.Cctor--")] [RequiresDynamicCode ("Message for --StaticCtorTriggeredByMethodCall.Cctor--")] @@ -124,8 +141,8 @@ public void TriggerStaticCtorMarking () [ExpectedWarning ("IL2026", "--StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--")] - [ExpectedWarning ("IL3002", "--StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--", ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL3002", "--StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] static void TestStaticCtorTriggeredByMethodCall () { new StaticCtorTriggeredByMethodCall ().TriggerStaticCtorMarking (); @@ -149,7 +166,9 @@ class TypeIsBeforeFieldInit [LogContains ("IL2026: Mono.Linker.Tests.Cases.RequiresCapability.RequiresOnStaticConstructor.TypeIsBeforeFieldInit..cctor():" + " Using member 'Mono.Linker.Tests.Cases.RequiresCapability.RequiresOnStaticConstructor.TypeIsBeforeFieldInit.AnnotatedMethod()'" + " which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code." + - " Message from --TypeIsBeforeFieldInit.AnnotatedMethod--.", ProducedBy = Tool.Trimmer)] + " Message from --TypeIsBeforeFieldInit.AnnotatedMethod--.", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [LogContains ("IL3002: Mono.Linker.Tests.Cases.RequiresCapability.RequiresOnStaticConstructor.TypeIsBeforeFieldInit..cctor():", ProducedBy = Tool.NativeAot)] + [LogContains ("IL3050: Mono.Linker.Tests.Cases.RequiresCapability.RequiresOnStaticConstructor.TypeIsBeforeFieldInit..cctor():", ProducedBy = Tool.NativeAot)] static void TestTypeIsBeforeFieldInit () { var x = TypeIsBeforeFieldInit.field + 42;