Skip to content

Commit a9f63a7

Browse files
Fix TypePreinit preinitialization correctness (#128973)
I asked GPT-5.5 to find bugs and it found bugs. One is more severe. Intern foreign preinit instances so nested static reference identity is preserved, conservatively mark serialized object instances mutable, and fix vtable-like slot clearing. Add NativeAOT smoke coverage for nested preinit identity and vtable slot clearing. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 91f6523 commit a9f63a7

2 files changed

Lines changed: 104 additions & 9 deletions

File tree

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

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public class TypePreinit
4242
private readonly Dictionary<FieldDesc, Value> _fieldValues = new Dictionary<FieldDesc, Value>();
4343
private readonly Dictionary<string, StringInstance> _internedStrings = new Dictionary<string, StringInstance>();
4444
private readonly Dictionary<TypeDesc, RuntimeTypeValue> _internedTypes = new Dictionary<TypeDesc, RuntimeTypeValue>();
45+
private readonly Dictionary<AllocationSite, ForeignTypeInstance> _foreignInstances = new Dictionary<AllocationSite, ForeignTypeInstance>();
4546
private readonly Dictionary<MetadataType, NestedPreinitResult> _nestedPreinitResults = new Dictionary<MetadataType, NestedPreinitResult>();
4647
private readonly Dictionary<EcmaField, byte[]> _rvaFieldDatas = new Dictionary<EcmaField, byte[]>();
4748

@@ -153,6 +154,16 @@ private byte[] GetFieldRvaData(EcmaField field)
153154
return result;
154155
}
155156

157+
private ForeignTypeInstance GetOrCreateForeignInstance(TypeDesc type, AllocationSite allocationSite, ReferenceTypeValue data)
158+
{
159+
if (!_foreignInstances.TryGetValue(allocationSite, out ForeignTypeInstance result))
160+
{
161+
_foreignInstances.Add(allocationSite, result = new ForeignTypeInstance(type, allocationSite, data));
162+
}
163+
164+
return result;
165+
}
166+
156167
private Status TryScanMethod(MethodDesc method, Value[] parameters, Stack<MethodDesc> recursionProtect, ref int instructionCounter, out Value returnValue)
157168
{
158169
MethodIL methodIL = _ilProvider.GetMethodIL(method);
@@ -2791,7 +2802,7 @@ public override bool TryInitialize(int size)
27912802
if (_index + numSlots > _parent._methods.Length)
27922803
return false;
27932804

2794-
for (int i = _index; i < numSlots; i++)
2805+
for (int i = _index; i < _index + numSlots; i++)
27952806
_parent._methods[i] = null;
27962807

27972808
return true;
@@ -3176,7 +3187,7 @@ public override bool TryCompareEquality(Value value, out bool result)
31763187
public abstract ReferenceTypeValue ToForeignInstance(int baseInstructionCounter, TypePreinit preinitContext);
31773188
}
31783189

3179-
private struct AllocationSite
3190+
private readonly struct AllocationSite : IEquatable<AllocationSite>
31803191
{
31813192
public MetadataType OwningType { get; }
31823193
public int InstructionCounter { get; }
@@ -3186,6 +3197,14 @@ public AllocationSite(MetadataType type, int instructionCounter)
31863197
OwningType = type;
31873198
InstructionCounter = instructionCounter;
31883199
}
3200+
3201+
public bool Equals(AllocationSite other) =>
3202+
OwningType == other.OwningType
3203+
&& InstructionCounter == other.InstructionCounter;
3204+
3205+
public override bool Equals(object obj) => obj is AllocationSite other && Equals(other);
3206+
3207+
public override int GetHashCode() => HashCode.Combine(OwningType, InstructionCounter);
31893208
}
31903209

31913210
/// <summary>
@@ -3201,11 +3220,13 @@ public AllocatedReferenceTypeValue(TypeDesc type, AllocationSite allocationSite)
32013220
AllocationSite = allocationSite;
32023221
}
32033222

3204-
public override ReferenceTypeValue ToForeignInstance(int baseInstructionCounter, TypePreinit preinitContext) =>
3205-
new ForeignTypeInstance(
3206-
Type,
3207-
new AllocationSite(AllocationSite.OwningType, AllocationSite.InstructionCounter - baseInstructionCounter),
3208-
this);
3223+
public override ReferenceTypeValue ToForeignInstance(int baseInstructionCounter, TypePreinit preinitContext)
3224+
{
3225+
AllocationSite foreignAllocationSite = new AllocationSite(
3226+
AllocationSite.OwningType,
3227+
AllocationSite.InstructionCounter - baseInstructionCounter);
3228+
return preinitContext.GetOrCreateForeignInstance(Type, foreignAllocationSite, this);
3229+
}
32093230

32103231
public override bool GetRawData(NodeFactory factory, out object data)
32113232
{
@@ -3432,7 +3453,10 @@ public override void WriteFieldData(ref ObjectDataBuilder builder, NodeFactory f
34323453
}
34333454
}
34343455

3435-
public override ReferenceTypeValue ToForeignInstance(int baseInstructionCounter, TypePreinit preinitContext) => this;
3456+
public override ReferenceTypeValue ToForeignInstance(int baseInstructionCounter, TypePreinit preinitContext)
3457+
{
3458+
return preinitContext.GetOrCreateForeignInstance(Type, AllocationSite, Data);
3459+
}
34363460
}
34373461

34383462
private sealed class StringInstance : ReferenceTypeValue, IHasInstanceFields
@@ -3572,7 +3596,26 @@ public void WriteContent(ref ObjectDataBuilder builder, ISymbolNode thisNode, No
35723596
builder.EmitBytes(_data, pointerSize, _data.Length - pointerSize);
35733597
}
35743598

3575-
public bool IsKnownImmutable => !Type.GetFields().GetEnumerator().MoveNext();
3599+
public bool IsKnownImmutable
3600+
{
3601+
get
3602+
{
3603+
// Are there any instance fields?
3604+
if (Type.IsValueType)
3605+
{
3606+
// For value types, look at the actual fields.
3607+
foreach (FieldDesc f in Type.GetFields())
3608+
if (!f.IsStatic)
3609+
return false;
3610+
3611+
return true;
3612+
}
3613+
3614+
// For reference types, check if the unaligned size == MethodTable pointer
3615+
// since we always have at least that field in the hierarchy.
3616+
return ((DefType)Type).InstanceByteCountUnaligned == Type.Context.Target.LayoutPointerSize;
3617+
}
3618+
}
35763619

35773620
public int ArrayLength => throw new NotSupportedException();
35783621
}

src/tests/nativeaot/SmokeTests/Preinitialization/Preinitialization.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ private static int Main()
4040
TestDelegateReflectionVisible.Run();
4141
TestInitFromOtherClass.Run();
4242
TestInitFromOtherClassDouble.Run();
43+
TestNestedPreinitIdentity.Run();
4344
TestDelegateToOtherClass.Run();
4445
TestLotsOfBackwardsBranches.Run();
4546
TestSwitch.Run();
@@ -727,6 +728,38 @@ public static void Run()
727728
}
728729
}
729730

731+
class TestNestedPreinitIdentity
732+
{
733+
class OtherClass
734+
{
735+
public static readonly object ObjectValue = new object();
736+
}
737+
738+
class YetAnotherClass
739+
{
740+
public static readonly object ObjectValue = OtherClass.ObjectValue;
741+
}
742+
743+
static bool s_areSame;
744+
static bool s_areSameIndirect;
745+
746+
static TestNestedPreinitIdentity()
747+
{
748+
object first = OtherClass.ObjectValue;
749+
object second = OtherClass.ObjectValue;
750+
object third = YetAnotherClass.ObjectValue;
751+
752+
s_areSame = first == second;
753+
s_areSameIndirect = first == third;
754+
}
755+
756+
public static void Run()
757+
{
758+
Assert.IsPreinitialized(typeof(TestNestedPreinitIdentity));
759+
Assert.True(s_areSame);
760+
Assert.True(s_areSameIndirect);
761+
}
762+
}
730763

731764
class TestDelegateToOtherClass
732765
{
@@ -1904,6 +1937,20 @@ static TinyVtableCImpl()
19041937
}
19051938
}
19061939

1940+
public unsafe class TinyVtableClearSlotImpl
1941+
{
1942+
[FixedAddressValueType]
1943+
public static readonly ITinyVtableB Vtbl;
1944+
1945+
static TinyVtableClearSlotImpl()
1946+
{
1947+
Vtbl.First = &First;
1948+
Vtbl.Second = &Second;
1949+
Vtbl.Third = &Third;
1950+
Vtbl.Second = default;
1951+
}
1952+
}
1953+
19071954
public unsafe struct ITinyVtableA
19081955
{
19091956
public delegate*<void> First;
@@ -1946,6 +1993,11 @@ public static unsafe void Run()
19461993
Assert.AreEqual((nuint)(delegate*<void>)&Second, (nuint)TinyVtableCImpl.Vtbl.Second);
19471994
Assert.AreEqual((nuint)(delegate*<void>)&Third, (nuint)TinyVtableCImpl.Vtbl.Third);
19481995
Assert.AreEqual((nuint)(delegate*<void>)&Fourth, (nuint)TinyVtableCImpl.Vtbl.Fourth);
1996+
1997+
Assert.IsPreinitialized(typeof(TinyVtableClearSlotImpl));
1998+
Assert.AreEqual((void*)(delegate*<void>)&First, TinyVtableClearSlotImpl.Vtbl.First);
1999+
Assert.AreEqual((void*)null, TinyVtableClearSlotImpl.Vtbl.Second);
2000+
Assert.AreEqual((void*)(delegate*<void>)&Third, TinyVtableClearSlotImpl.Vtbl.Third);
19492001
}
19502002
}
19512003

0 commit comments

Comments
 (0)