Skip to content

Commit

Permalink
Static cctor analysis for reflection (dotnet#84089)
Browse files Browse the repository at this point in the history 
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 dotnet#82447.
  • Loading branch information
@vitek-karas
vitek-karas authored Apr 4, 2023
1 parent 9b11e69 commit 7b645f7
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static IEnumerable<MethodDesc> GetConstructorsOnType(this TypeDesc type,

foreach (var method in type.GetMethods())
{
if (!method.IsConstructor)
if (!method.IsConstructor && !method.IsStaticConstructor)
continue;

if (filter != null && !filter(method))
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using ILCompiler.Dataflow;
using ILCompiler.DependencyAnalysis;
using ILCompiler.DependencyAnalysisFramework;
using ILCompiler.Logging;
using ILCompiler.Metadata;
using ILLink.Shared;

Expand Down Expand Up @@ -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)
Expand Down
39 changes: 29 additions & 10 deletions ...ols/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnStaticConstructor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ()
Expand All @@ -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 ();
}

Expand All @@ -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()")]
Expand Down Expand Up @@ -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--")]
Expand All @@ -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 ();
Expand All @@ -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;
Expand Down

0 comments on commit 7b645f7

Please sign in to comment.