Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure static constructor is reflection-callable when NonPublicConstructors were kept #103946

Merged
merged 1 commit into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand All @@ -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)
{
}

Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
37 changes: 37 additions & 0 deletions src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Loading