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.

Sign up for GitHub

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

Don't allow preinitialization to skip dataflow #98318

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Don't allow preinitialization to skip dataflow #98318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public CompilationBuilder UseDwarf5(bool value)
protected PreinitializationManager GetPreinitializationManager()
{
if (_preinitializationManager == null)
return new PreinitializationManager(_context, _compilationGroup, GetILProvider(), new TypePreinit.DisabledPreinitializationPolicy(), new StaticReadOnlyFieldPolicy());
return new PreinitializationManager(_context, _compilationGroup, GetILProvider(), new TypePreinit.DisabledPreinitializationPolicy(), new StaticReadOnlyFieldPolicy(), null);
return _preinitializationManager;
}

Expand Down
13 changes: 9 additions & 4 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/PreinitializationManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;

using Internal.IL;
using Internal.TypeSystem;

using FlowAnnotations = ILLink.Shared.TrimAnalysis.FlowAnnotations;

namespace ILCompiler
{
/// <summary>
Expand All @@ -14,10 +17,10 @@ public class PreinitializationManager
{
private readonly bool _supportsLazyCctors;

public PreinitializationManager(TypeSystemContext context, CompilationModuleGroup compilationGroup, ILProvider ilprovider, TypePreinit.TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy)
public PreinitializationManager(TypeSystemContext context, CompilationModuleGroup compilationGroup, ILProvider ilprovider, TypePreinit.TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy, FlowAnnotations flowAnnotations)
{
_supportsLazyCctors = context.SystemModule.GetType("System.Runtime.CompilerServices", "ClassConstructorRunner", throwIfNotFound: false) != null;
_preinitHashTable = new PreinitializationInfoHashtable(compilationGroup, ilprovider, policy, readOnlyPolicy);
_preinitHashTable = new PreinitializationInfoHashtable(compilationGroup, ilprovider, policy, readOnlyPolicy, flowAnnotations);
}

/// <summary>
Expand Down Expand Up @@ -137,13 +140,15 @@ private sealed class PreinitializationInfoHashtable : LockFreeReaderHashtable<Me
private readonly ILProvider _ilProvider;
internal readonly TypePreinit.TypePreinitializationPolicy _policy;
private readonly ReadOnlyFieldPolicy _readOnlyPolicy;
private readonly FlowAnnotations _flowAnnotations;

public PreinitializationInfoHashtable(CompilationModuleGroup compilationGroup, ILProvider ilProvider, TypePreinit.TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy)
public PreinitializationInfoHashtable(CompilationModuleGroup compilationGroup, ILProvider ilProvider, TypePreinit.TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy, FlowAnnotations flowAnnotations)
{
_compilationGroup = compilationGroup;
_ilProvider = ilProvider;
_policy = policy;
_readOnlyPolicy = readOnlyPolicy;
_flowAnnotations = flowAnnotations;
}

protected override bool CompareKeyToValue(MetadataType key, TypePreinit.PreinitializationInfo value) => key == value.Type;
Expand All @@ -153,7 +158,7 @@ public PreinitializationInfoHashtable(CompilationModuleGroup compilationGroup, I

protected override TypePreinit.PreinitializationInfo CreateValueFromKey(MetadataType key)
{
var info = TypePreinit.ScanType(_compilationGroup, _ilProvider, _policy, _readOnlyPolicy, key);
var info = TypePreinit.ScanType(_compilationGroup, _ilProvider, _policy, _readOnlyPolicy, _flowAnnotations, key);

// We either successfully preinitialized or
// the type doesn't have a canonical form or
Expand Down
41 changes: 37 additions & 4 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Internal.TypeSystem;

using CombinedDependencyList = System.Collections.Generic.List<ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.CombinedDependencyListEntry>;
using FlowAnnotations = ILLink.Shared.TrimAnalysis.FlowAnnotations;

namespace ILCompiler
{
Expand All @@ -36,17 +37,19 @@ public class TypePreinit
private readonly ILProvider _ilProvider;
private readonly TypePreinitializationPolicy _policy;
private readonly ReadOnlyFieldPolicy _readOnlyPolicy;
private readonly FlowAnnotations _flowAnnotations;
private readonly Dictionary<FieldDesc, Value> _fieldValues = new Dictionary<FieldDesc, Value>();
private readonly Dictionary<string, StringInstance> _internedStrings = new Dictionary<string, StringInstance>();
private readonly Dictionary<TypeDesc, RuntimeTypeValue> _internedTypes = new Dictionary<TypeDesc, RuntimeTypeValue>();

private TypePreinit(MetadataType owningType, CompilationModuleGroup compilationGroup, ILProvider ilProvider, TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy)
private TypePreinit(MetadataType owningType, CompilationModuleGroup compilationGroup, ILProvider ilProvider, TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy, FlowAnnotations flowAnnotations)
{
_type = owningType;
_compilationGroup = compilationGroup;
_ilProvider = ilProvider;
_policy = policy;
_readOnlyPolicy = readOnlyPolicy;
_flowAnnotations = flowAnnotations;

// Zero initialize all fields we model.
foreach (var field in owningType.GetFields())
Expand All @@ -58,7 +61,7 @@ private TypePreinit(MetadataType owningType, CompilationModuleGroup compilationG
}
}

public static PreinitializationInfo ScanType(CompilationModuleGroup compilationGroup, ILProvider ilProvider, TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy, MetadataType type)
public static PreinitializationInfo ScanType(CompilationModuleGroup compilationGroup, ILProvider ilProvider, TypePreinitializationPolicy policy, ReadOnlyFieldPolicy readOnlyPolicy, FlowAnnotations flowAnnotations, MetadataType type)
{
Debug.Assert(type.HasStaticConstructor);
Debug.Assert(!type.IsGenericDefinition);
Expand All @@ -85,7 +88,7 @@ public static PreinitializationInfo ScanType(CompilationModuleGroup compilationG
Status status;
try
{
preinit = new TypePreinit(type, compilationGroup, ilProvider, policy, readOnlyPolicy);
preinit = new TypePreinit(type, compilationGroup, ilProvider, policy, readOnlyPolicy, flowAnnotations);
int instructions = 0;
status = preinit.TryScanMethod(type.GetStaticConstructor(), null, null, ref instructions, out _);
}
Expand Down Expand Up @@ -313,6 +316,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Unsupported static");
}

if (_flowAnnotations.RequiresDataflowAnalysisDueToSignature(field))
{
return Status.Fail(methodIL.OwningMethod, opcode, "Needs dataflow analysis");
}

if (_fieldValues[field] is IAssignableValue assignableField)
{
if (!assignableField.TryAssign(stack.PopIntoLocation(field.FieldType)))
Expand Down Expand Up @@ -351,7 +359,7 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
&& field.OwningType.HasStaticConstructor
&& _policy.CanPreinitialize(field.OwningType))
{
TypePreinit nestedPreinit = new TypePreinit((MetadataType)field.OwningType, _compilationGroup, _ilProvider, _policy, _readOnlyPolicy);
TypePreinit nestedPreinit = new TypePreinit((MetadataType)field.OwningType, _compilationGroup, _ilProvider, _policy, _readOnlyPolicy, _flowAnnotations);
recursionProtect ??= new Stack<MethodDesc>();
recursionProtect.Push(methodIL.OwningMethod);

Expand Down Expand Up @@ -410,6 +418,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Unsupported static");
}

if (_flowAnnotations.RequiresDataflowAnalysisDueToSignature(field))
{
return Status.Fail(methodIL.OwningMethod, opcode, "Needs dataflow analysis");
}

Value fieldValue = _fieldValues[field];
if (fieldValue == null || !fieldValue.TryCreateByRef(out Value byRefValue))
{
Expand Down Expand Up @@ -452,6 +465,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Static constructor");
}

if (_flowAnnotations.RequiresDataflowAnalysisDueToSignature(method))
{
return Status.Fail(methodIL.OwningMethod, opcode, "Needs dataflow analysis");
}

Value[] methodParams = new Value[numParams];
for (int i = numParams - 1; i >= 0; i--)
{
Expand Down Expand Up @@ -514,6 +532,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Finalizable class");
}

if (_flowAnnotations.RequiresDataflowAnalysisDueToSignature(ctor))
{
return Status.Fail(methodIL.OwningMethod, opcode, "Needs dataflow analysis");
}

Value[] ctorParameters = new Value[ctorSig.Length + 1];
for (int i = ctorSig.Length - 1; i >= 0; i--)
{
Expand Down Expand Up @@ -712,6 +735,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Byref field");
}

if (_flowAnnotations.RequiresDataflowAnalysisDueToSignature(field))
{
return Status.Fail(methodIL.OwningMethod, opcode, "Needs dataflow analysis");
}

if (instance.Value is not IHasInstanceFields settableInstance
|| !settableInstance.TrySetField(field, value))
{
Expand Down Expand Up @@ -753,6 +781,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode);
}

if (_flowAnnotations.RequiresDataflowAnalysisDueToSignature(field))
{
return Status.Fail(methodIL.OwningMethod, opcode, "Needs dataflow analysis");
}

StackEntry instance = stack.Pop();

var loadableInstance = instance.Value as IHasInstanceFields;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/aot/ILCompiler/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ public int Run()
TypePreinit.TypePreinitializationPolicy preinitPolicy = preinitStatics ?
new TypePreinit.TypeLoaderAwarePreinitializationPolicy() : new TypePreinit.DisabledPreinitializationPolicy();

var preinitManager = new PreinitializationManager(typeSystemContext, compilationGroup, ilProvider, preinitPolicy, new StaticReadOnlyFieldPolicy());
var preinitManager = new PreinitializationManager(typeSystemContext, compilationGroup, ilProvider, preinitPolicy, new StaticReadOnlyFieldPolicy(), flowAnnotations);
builder
.UseILProvider(ilProvider)
.UsePreinitializationManager(preinitManager);
Expand Down Expand Up @@ -526,7 +526,7 @@ void RunScanner()
{
var readOnlyFieldPolicy = scanResults.GetReadOnlyFieldPolicy();
preinitManager = new PreinitializationManager(typeSystemContext, compilationGroup, ilProvider, scanResults.GetPreinitializationPolicy(),
readOnlyFieldPolicy);
readOnlyFieldPolicy, flowAnnotations);
builder.UsePreinitializationManager(preinitManager)
.UseReadOnlyFieldPolicy(readOnlyFieldPolicy);
}
Expand Down
31 changes: 25 additions & 6 deletions src/tests/nativeaot/SmokeTests/Preinitialization/Preinitialization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ private static int Main()
TestIsValueType.Run();
TestIndirectLoads.Run();
TestInitBlock.Run();
TestDataflow.Run();
#else
Console.WriteLine("Preinitialization is disabled in multimodule builds for now. Skipping test.");
#endif
Expand Down Expand Up @@ -1310,15 +1311,17 @@ public static void Run()

class TestIndirectLoads
{
static unsafe U Read<T, U>(T val) where T : unmanaged where U : unmanaged
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roslyn generates this as new() constraint which is very unfortunate because it requires dataflow.

=> *(U*)&val;
static unsafe sbyte Read(byte val) => *(sbyte*)&val;
static unsafe short Read(ushort val) => *(short*)&val;
static unsafe int Read(uint val) => *(int*)&val;
static unsafe long Read(ulong val) => *(long*)&val;

class LdindTester
{
public static sbyte SByte = Read<byte, sbyte>(byte.MaxValue);
public static short Short = Read<ushort, short>(ushort.MaxValue);
public static int Int = Read<uint, int>(uint.MaxValue);
public static long Long = Read<ulong, long>(ulong.MaxValue);
public static sbyte SByte = Read(byte.MaxValue);
public static short Short = Read(ushort.MaxValue);
public static int Int = Read(uint.MaxValue);
public static long Long = Read(ulong.MaxValue);
}

public static void Run()
Expand Down Expand Up @@ -1367,6 +1370,22 @@ public static void Run()
}
}

class TestDataflow
{
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
public static Type TheType = typeof(MyType);

class MyType
{
public static void TheMethod() => Console.WriteLine("Hello");
}

public static void Run()
{
TheType.GetMethod("TheMethod").Invoke(null, []);
}
}

static class Assert
{
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
Expand Down
Loading