Ensure static constructor is reflection-callable when NonPublicConstr…

…uctors were kept (#103946)

This ensures that whenever we've seen the static constructor was reflection-accessed, we make sure it's runnable with RunClassConstructor.

This previously required some acrobatics because the static constructor itself could be reflection blocked and we might have still needed to RunClassConstructor the type with such constructor (notably the SR class preinitializes random ResourceManager related types to avoid recursion). Redo everything to simply use the reflectable static constructor method as a marker.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs

@@ -22,7 +22,6 @@ namespace ILCompiler
     public sealed class AnalysisBasedMetadataManager : MetadataManager, ICompilationRootProvider
     {
         private readonly List<ModuleDesc> _modulesWithMetadata;
-        private readonly List<MetadataType> _typesWithRootedCctorContext;
         private readonly List<TypeDesc> _forcedTypes;
 
         private readonly Dictionary<TypeDesc, MetadataCategory> _reflectableTypes = new Dictionary<TypeDesc, MetadataCategory>();
@@ -36,7 +35,7 @@ public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext)
                 new NoDynamicInvokeThunkGenerationPolicy(), Array.Empty<ModuleDesc>(), Array.Empty<TypeDesc>(),
                 Array.Empty<ReflectableEntity<TypeDesc>>(), Array.Empty<ReflectableEntity<MethodDesc>>(),
                 Array.Empty<ReflectableEntity<FieldDesc>>(), Array.Empty<ReflectableCustomAttribute>(),
-                Array.Empty<MetadataType>(), default)
+                default)
         {
         }
 
@@ -53,12 +52,10 @@ public AnalysisBasedMetadataManager(
             IEnumerable<ReflectableEntity<MethodDesc>> reflectableMethods,
             IEnumerable<ReflectableEntity<FieldDesc>> reflectableFields,
             IEnumerable<ReflectableCustomAttribute> reflectableAttributes,
-            IEnumerable<MetadataType> rootedCctorContexts,
             MetadataManagerOptions options)
             : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy, invokeThunkGenerationPolicy, options)
         {
             _modulesWithMetadata = new List<ModuleDesc>(modulesWithMetadata);
-            _typesWithRootedCctorContext = new List<MetadataType>(rootedCctorContexts);
             _forcedTypes = new List<TypeDesc>(forcedTypes);
 
             foreach (var refType in reflectableTypes)
@@ -205,11 +202,6 @@ void ICompilationRootProvider.AddCompilationRoots(IRootingServiceProvider rootPr
                     rootProvider.AddReflectionRoot(field, reason);
                 }
             }
-
-            foreach (var type in _typesWithRootedCctorContext)
-            {
-                rootProvider.RootNonGCStaticBaseForType(type, reason);
-            }
         }
 
         private struct Policy : IMetadataPolicy

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs

@@ -206,15 +206,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
-                // about whether the class constructor already executed and it's what is looked at at runtime.
-                _dependencies.Add(Factory.TypeNonGCStaticsSymbol((MetadataType)type), "RunClassConstructor reference");
-            }
+            MethodDesc cctor = type.GetStaticConstructor();
+            if (cctor != null)
+                MarkMethod(origin, cctor, reason);
         }
 
         internal void CheckAndWarnOnReflectionAccess(in MessageOrigin origin, TypeSystemEntity entity, AccessKind accessKind = AccessKind.Unspecified)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs

@@ -463,6 +463,13 @@ public void GetDependenciesDueToReflectability(ref DependencyList dependencies,
                     ReflectionInvokeSupportDependencyAlgorithm.GetDependenciesFromParamsArray(ref dependencies, factory, method);
                 }
 
+                if (!method.IsCanonicalMethod(CanonicalFormKind.Any) && method.IsStaticConstructor)
+                {
+                    // Information about the static constructor prefixes the non-GC static base
+                    dependencies ??= new DependencyList();
+                    dependencies.Add(factory.TypeNonGCStaticsSymbol((MetadataType)method.OwningType), "Static constructor is reflection-callable");
+                }
+
                 GenericMethodsTemplateMap.GetTemplateMethodDependencies(ref dependencies, factory, method);
                 GenericTypesTemplateMap.GetTemplateTypeDependencies(ref dependencies, factory, method.OwningType);
             }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

@@ -893,28 +893,10 @@ public MetadataManager ToAnalysisBasedMetadataManager()
                 }
             }
 
-            var rootedCctorContexts = new List<MetadataType>();
-            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, _typesWithForcedEEType, reflectableTypes.ToEnumerable(), reflectableMethods.ToEnumerable(),
-                reflectableFields.ToEnumerable(), _customAttributesWithMetadata, rootedCctorContexts, _options);
+                reflectableFields.ToEnumerable(), _customAttributesWithMetadata, _options);
         }
 
         private void AddDataflowDependency(ref DependencyList dependencies, NodeFactory factory, MethodIL methodIL, string reason)

src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs

@@ -2207,13 +2207,50 @@ static TypeWithNoStaticFieldsButACCtor()
             }
         }
 
+        class ClassWithCctor
+        {
+            static ClassWithCctor() => s_field = 42;
+        }
+
+        class ClassWithCctor<T>
+        {
+            static ClassWithCctor() => s_field = 11;
+        }
+
+        class DynamicClassWithCctor<T>
+        {
+            static DynamicClassWithCctor() => s_field = 1000;
+        }
+
+        class Atom { }
+
         private static bool s_cctorRan;
+        private static int s_field;
 
         public static void Run()
         {
             RuntimeHelpers.RunClassConstructor(typeof(TypeWithNoStaticFieldsButACCtor).TypeHandle);
             if (!s_cctorRan)
                 throw new Exception();
+
+            RunTheCctor(typeof(ClassWithCctor));
+            if (s_field != 42)
+                throw new Exception();
+
+            RunTheCctor(typeof(ClassWithCctor<Atom>));
+            if (s_field != 11)
+                throw new Exception();
+
+            RunTheCctor(typeof(DynamicClassWithCctor<>).MakeGenericType(GetAtom()));
+            if (s_field != 1000)
+                throw new Exception();
+
+            [MethodImpl(MethodImplOptions.NoInlining)]
+            static void RunTheCctor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type t)
+                => RuntimeHelpers.RunClassConstructor(t.TypeHandle);
+
+            [MethodImpl(MethodImplOptions.NoInlining)]
+            static Type GetAtom() => typeof(Atom);
         }
     }