From 3f40e7415e402fa4e5edd4a28a5f7cdc276d224e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 17 Dec 2021 17:19:11 +0900 Subject: [PATCH 1/2] Handle RunClassConstructor with nonreflectable cctor This is now getting hit in #62927, so it's somewhat more urgent. (The feature switches from the SDK put us into the situation that triggers this bug around RunClassConstructor on an otherwise unused type.) Fixes dotnet/runtimelab#987. Remember what class constructor contexts we saw during scanning phase and if the owning type is also generated, assume `RunClassConstructor` could be used and ensure the cctor context is also generated in the compilation phase. This is somewhat less precise, but introducing a new node type for "a type used with `RunClassConstructor`" that dataflow analysis could report doesn't seem worth it. --- .../Compiler/AnalysisBasedMetadataManager.cs | 13 ++++++++++-- .../Compiler/UsageBasedMetadataManager.cs | 20 +++++++++++++++++- .../SmokeTests/Reflection/Reflection.cs | 21 +++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs index 6ba6cc54c5122f..5c3db3a8525148 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs @@ -22,6 +22,7 @@ namespace ILCompiler public sealed class AnalysisBasedMetadataManager : GeneratingMetadataManager, ICompilationRootProvider { private readonly List _modulesWithMetadata; + private readonly List _typesWithRootedCctorContext = new List(); private readonly Dictionary _reflectableTypes = new Dictionary(); private readonly Dictionary _reflectableMethods = new Dictionary(); @@ -33,7 +34,8 @@ public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext) new FullyBlockedManifestResourceBlockingPolicy(), null, new NoStackTraceEmissionPolicy(), new NoDynamicInvokeThunkGenerationPolicy(), Array.Empty(), Array.Empty>(), Array.Empty>(), - Array.Empty>(), Array.Empty()) + Array.Empty>(), Array.Empty(), + Array.Empty()) { } @@ -48,10 +50,12 @@ public AnalysisBasedMetadataManager( IEnumerable> reflectableTypes, IEnumerable> reflectableMethods, IEnumerable> reflectableFields, - IEnumerable reflectableAttributes) + IEnumerable reflectableAttributes, + IEnumerable rootedCctorContexts) : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy, invokeThunkGenerationPolicy) { _modulesWithMetadata = new List(modulesWithMetadata); + _typesWithRootedCctorContext = new List(rootedCctorContexts); foreach (var refType in reflectableTypes) { @@ -209,6 +213,11 @@ void ICompilationRootProvider.AddCompilationRoots(IRootingServiceProvider rootPr } } } + + foreach (var type in _typesWithRootedCctorContext) + { + rootProvider.RootNonGCStaticBaseForType(type, reason); + } } private struct Policy : IMetadataPolicy diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index 687104277c2f2b..bd649904129bf8 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -794,10 +794,28 @@ public MetadataManager ToAnalysisBasedMetadataManager() } } + var rootedCctorContexts = new List(); + foreach (NonGCStaticsNode cctorContext in GetCctorContextMapping()) + { + // If we generated a static constructor and the owning type, this might be something + // that gets fed to RuntimeHelpers.RunClassConstructor. RunClassConstructor + // also works on reflection blocked types and there is a possibility that we + // wouldn't have generated the cctor otherwise. + // + // This is a heuristic and we'll possibly root more cctor contexts than + // strictly necessary, but it's not worth introducing a new node type + // in the compiler just so we can propagate this knowledge from dataflow analysis + // (that detects RunClassConstructor usage) and this spot. + if (!TypeGeneratesEEType(cctorContext.Type)) + continue; + + rootedCctorContexts.Add(cctorContext.Type); + } + return new AnalysisBasedMetadataManager( _typeSystemContext, _blockingPolicy, _resourceBlockingPolicy, _metadataLogFile, _stackTraceEmissionPolicy, _dynamicInvokeThunkGenerationPolicy, _modulesWithMetadata, reflectableTypes.ToEnumerable(), reflectableMethods.ToEnumerable(), - reflectableFields.ToEnumerable(), _customAttributesWithMetadata); + reflectableFields.ToEnumerable(), _customAttributesWithMetadata, rootedCctorContexts); } private struct ReflectableEntityBuilder diff --git a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs index ee88635a8b65d1..e8cf15ce7cf855 100644 --- a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs +++ b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs @@ -27,6 +27,7 @@ private static int Main() // // Tests for dependency graph in the compiler // + TestRunClassConstructor.Run(); #if !OPTIMIZED_MODE_WITHOUT_SCANNER TestContainment.Run(); TestInterfaceMethod.Run(); @@ -1655,6 +1656,26 @@ public static void Run() } } + class TestRunClassConstructor + { + static class TypeWithNoStaticFieldsButACCtor + { + static TypeWithNoStaticFieldsButACCtor() + { + s_cctorRan = true; + } + } + + private static bool s_cctorRan; + + public static void Run() + { + RuntimeHelpers.RunClassConstructor(typeof(TypeWithNoStaticFieldsButACCtor).TypeHandle); + if (!s_cctorRan) + throw new Exception(); + } + } + #region Helpers private static Type GetTestType(string testName, string typeName) From cfc8a9751d87709c7dadd961b25e62fdd79d01e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 17 Dec 2021 17:20:33 +0900 Subject: [PATCH 2/2] Oops --- .../Compiler/AnalysisBasedMetadataManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs index 5c3db3a8525148..8ad71eb3db5b5d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs @@ -22,7 +22,7 @@ namespace ILCompiler public sealed class AnalysisBasedMetadataManager : GeneratingMetadataManager, ICompilationRootProvider { private readonly List _modulesWithMetadata; - private readonly List _typesWithRootedCctorContext = new List(); + private readonly List _typesWithRootedCctorContext; private readonly Dictionary _reflectableTypes = new Dictionary(); private readonly Dictionary _reflectableMethods = new Dictionary();