Skip to content

Commit

Permalink
Handle RunClassConstructor with nonreflectable cctor (#62947)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MichalStrehovsky committed Dec 18, 2021
1 parent 5e9381c commit 118349f
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace ILCompiler
public sealed class AnalysisBasedMetadataManager : GeneratingMetadataManager, ICompilationRootProvider
{
private readonly List<ModuleDesc> _modulesWithMetadata;
private readonly List<MetadataType> _typesWithRootedCctorContext;

private readonly Dictionary<TypeDesc, MetadataCategory> _reflectableTypes = new Dictionary<TypeDesc, MetadataCategory>();
private readonly Dictionary<MethodDesc, MetadataCategory> _reflectableMethods = new Dictionary<MethodDesc, MetadataCategory>();
Expand All @@ -33,7 +34,8 @@ public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext)
new FullyBlockedManifestResourceBlockingPolicy(), null, new NoStackTraceEmissionPolicy(),
new NoDynamicInvokeThunkGenerationPolicy(), Array.Empty<ModuleDesc>(),
Array.Empty<ReflectableEntity<TypeDesc>>(), Array.Empty<ReflectableEntity<MethodDesc>>(),
Array.Empty<ReflectableEntity<FieldDesc>>(), Array.Empty<ReflectableCustomAttribute>())
Array.Empty<ReflectableEntity<FieldDesc>>(), Array.Empty<ReflectableCustomAttribute>(),
Array.Empty<MetadataType>())
{
}

Expand All @@ -48,10 +50,12 @@ public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext)
IEnumerable<ReflectableEntity<TypeDesc>> reflectableTypes,
IEnumerable<ReflectableEntity<MethodDesc>> reflectableMethods,
IEnumerable<ReflectableEntity<FieldDesc>> reflectableFields,
IEnumerable<ReflectableCustomAttribute> reflectableAttributes)
IEnumerable<ReflectableCustomAttribute> reflectableAttributes,
IEnumerable<MetadataType> rootedCctorContexts)
: base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy, invokeThunkGenerationPolicy)
{
_modulesWithMetadata = new List<ModuleDesc>(modulesWithMetadata);
_typesWithRootedCctorContext = new List<MetadataType>(rootedCctorContexts);

foreach (var refType in reflectableTypes)
{
Expand Down Expand Up @@ -209,6 +213,11 @@ void ICompilationRootProvider.AddCompilationRoots(IRootingServiceProvider rootPr
}
}
}

foreach (var type in _typesWithRootedCctorContext)
{
rootProvider.RootNonGCStaticBaseForType(type, reason);
}
}

private struct Policy : IMetadataPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,10 +794,28 @@ 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, reflectableTypes.ToEnumerable(), reflectableMethods.ToEnumerable(),
reflectableFields.ToEnumerable(), _customAttributesWithMetadata);
reflectableFields.ToEnumerable(), _customAttributesWithMetadata, rootedCctorContexts);
}

private struct ReflectableEntityBuilder<T>
Expand Down
21 changes: 21 additions & 0 deletions src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 118349f

Please sign in to comment.