Skip to content

Commit

Permalink
Fix preinit of types placing the same value in two fields (#73605)
Browse files Browse the repository at this point in the history
We had a problem where types that put the same object instance in two different fields would see two different object instances at runtime due to two frozen objects being created for what should have been just one instance. (See the test.)

Frozen objects were deriving their identity from the field to which they were assigned to so the problem fell out from this awkward design.

The fix is actually a simplification - stop deriving object identity from field and use a "Allocation site ID" instead. The Allocation Site ID is a tuple of "Type whose cctor we were interpreting" + "instruction counter at the time of allocation". That way we can uniquely identify object instances and keep referring to objects allocated in different cctors.

I've also lifted the limitation that instance delegates can only point to objects that were assigned to some fields in a different cctor because it's no longer required to limit it.
  • Loading branch information
MichalStrehovsky committed Aug 10, 2022
1 parent 6da2046 commit bbcc0ab
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 62 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;

using Internal.Text;
Expand All @@ -14,19 +15,22 @@ namespace ILCompiler.DependencyAnalysis
/// </summary>
public class FrozenObjectNode : EmbeddedObjectNode, ISymbolDefinitionNode
{
private readonly FieldDesc _field;
private readonly MetadataType _owningType;
private readonly TypePreinit.ISerializableReference _data;
private readonly int _allocationSiteId;

public FrozenObjectNode(FieldDesc field, TypePreinit.ISerializableReference data)
public FrozenObjectNode(MetadataType owningType, int allocationSiteId, TypePreinit.ISerializableReference data)
{
_field = field;
_owningType = owningType;
_allocationSiteId = allocationSiteId;
_data = data;
}

public void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
{
sb.Append(nameMangler.CompilationUnitPrefix).Append("__FrozenObj_")
.Append(nameMangler.GetMangledFieldName(_field));
.Append(nameMangler.GetMangledTypeName(_owningType))
.Append(_allocationSiteId.ToStringInvariant());
}

public override bool StaticDependenciesAreComputed => true;
Expand All @@ -38,7 +42,7 @@ int ISymbolDefinitionNode.Offset
get
{
// The frozen object symbol points at the MethodTable portion of the object, skipping over the sync block
return OffsetFromBeginningOfArray + _field.Context.Target.PointerSize;
return OffsetFromBeginningOfArray + _owningType.Context.Target.PointerSize;
}
}

Expand Down Expand Up @@ -81,7 +85,12 @@ protected override void OnMarked(NodeFactory factory)

public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
{
return comparer.Compare(((FrozenObjectNode)other)._field, _field);
var otherFrozenObjectNode = (FrozenObjectNode)other;
int result = comparer.Compare(otherFrozenObjectNode._owningType, _owningType);
if (result != 0)
return result;

return _allocationSiteId.CompareTo(otherFrozenObjectNode._allocationSiteId);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
TypePreinit.ISerializableValue val = _preinitializationInfo.GetFieldValue(field);
int currentOffset = builder.CountBytes;
if (val != null)
val.WriteFieldData(ref builder, field, factory);
val.WriteFieldData(ref builder, factory);
else
builder.EmitZeroPointer();
Debug.Assert(builder.CountBytes - currentOffset == field.FieldType.GetElementSize().AsInt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ private void CreateNodeCaches()

_frozenObjectNodes = new NodeCache<SerializedFrozenObjectKey, FrozenObjectNode>(key =>
{
return new FrozenObjectNode(key.Owner, key.SerializableObject);
return new FrozenObjectNode(key.OwnerType, key.AllocationSiteId, key.SerializableObject);
});

_interfaceDispatchCells = new NodeCache<DispatchCellKey, InterfaceDispatchCellNode>(callSiteCell =>
Expand Down Expand Up @@ -1063,9 +1063,9 @@ public FrozenStringNode SerializedStringObject(string data)

private NodeCache<SerializedFrozenObjectKey, FrozenObjectNode> _frozenObjectNodes;

public FrozenObjectNode SerializedFrozenObject(FieldDesc owningField, TypePreinit.ISerializableReference data)
public FrozenObjectNode SerializedFrozenObject(MetadataType owningType, int allocationSiteId, TypePreinit.ISerializableReference data)
{
return _frozenObjectNodes.GetOrAdd(new SerializedFrozenObjectKey(owningField, data));
return _frozenObjectNodes.GetOrAdd(new SerializedFrozenObjectKey(owningType, allocationSiteId, data));
}

private NodeCache<MethodDesc, EmbeddedObjectNode> _eagerCctorIndirectionNodes;
Expand Down Expand Up @@ -1306,18 +1306,21 @@ public UninitializedWritableDataBlobKey(Utf8String name, int size, int alignment

protected struct SerializedFrozenObjectKey : IEquatable<SerializedFrozenObjectKey>
{
public readonly FieldDesc Owner;
public readonly MetadataType OwnerType;
public readonly int AllocationSiteId;
public readonly TypePreinit.ISerializableReference SerializableObject;

public SerializedFrozenObjectKey(FieldDesc owner, TypePreinit.ISerializableReference obj)
public SerializedFrozenObjectKey(MetadataType ownerType, int allocationSiteId, TypePreinit.ISerializableReference obj)
{
Owner = owner;
Debug.Assert(ownerType.HasStaticConstructor);
OwnerType = ownerType;
AllocationSiteId = allocationSiteId;
SerializableObject = obj;
}

public override bool Equals(object obj) => obj is SerializedFrozenObjectKey && Equals((SerializedFrozenObjectKey)obj);
public bool Equals(SerializedFrozenObjectKey other) => Owner == other.Owner;
public override int GetHashCode() => Owner.GetHashCode();
public bool Equals(SerializedFrozenObjectKey other) => OwnerType == other.OwnerType && AllocationSiteId == other.AllocationSiteId;
public override int GetHashCode() => HashCode.Combine(OwnerType.GetHashCode(), AllocationSiteId);
}

private struct MethodILKey : IEquatable<MethodILKey>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly)

TypePreinit.ISerializableValue val = preinitInfo.GetFieldValue(field);
int currentOffset = builder.CountBytes;
val.WriteFieldData(ref builder, field, factory);
val.WriteFieldData(ref builder, factory);
Debug.Assert(builder.CountBytes - currentOffset == field.FieldType.GetElementSize().AsInt);
}

Expand Down
Loading

0 comments on commit bbcc0ab

Please sign in to comment.