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

Fix explicit offset of ByRefLike fields. #111584

Merged
merged 16 commits into from
Jan 31, 2025
Merged
Changes from 11 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
@@ -61,6 +61,12 @@ public override bool ComputeContainsGCPointers(DefType type)
return false;
}

public override bool ComputeContainsByRefs(DefType type)
{
Debug.Assert(!_fallbackAlgorithm.ComputeContainsByRefs(type));
return false;
}

public override bool ComputeIsUnsafeValueType(DefType type)
{
Debug.Assert(!_fallbackAlgorithm.ComputeIsUnsafeValueType(type));
Original file line number Diff line number Diff line change
@@ -133,6 +133,12 @@ public override bool ComputeContainsGCPointers(DefType type)
return false;
}

public override bool ComputeContainsByRefs(DefType type)
{
Debug.Assert(!_fallbackAlgorithm.ComputeContainsByRefs(type));
return false;
}

public override bool ComputeIsUnsafeValueType(DefType type)
{
Debug.Assert(!_fallbackAlgorithm.ComputeIsUnsafeValueType(type));
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ protected override LoweredType GetIntervalDataForType(int offset, TypeDesc field
return LoweredType.Opaque;
}

protected override bool NeedsRecursiveLayout(int offset, TypeDesc fieldType) => fieldType.IsValueType && !fieldType.IsPrimitiveNumeric;
protected override bool NeedsRecursiveLayout(TypeDesc fieldType) => fieldType.IsValueType && !fieldType.IsPrimitiveNumeric;

private List<FieldLayoutInterval> CreateConsolidatedIntervals()
{
Original file line number Diff line number Diff line change
@@ -78,6 +78,16 @@ private static class FieldLayoutFlags
/// True if the type transitively has a Vector<T> in it or is Vector<T>
/// </summary>
public const int IsVectorTOrHasVectorTFields = 0x1000;

/// <summary>
/// True if ContainsByRefs has been computed
/// </summary>
public const int ComputedContainsByRefs = 0x2000;

/// <summary>
/// True if the type contains byrefs
/// </summary>
public const int ContainsByRefs = 0x4000;
}

private sealed class StaticBlockInfo
@@ -113,6 +123,21 @@ public bool ContainsGCPointers
}
}

/// <summary>
/// Does a type transitively have any fields which are byrefs
/// </summary>
public bool ContainsByRefs
{
get
{
if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedContainsByRefs))
{
ComputeTypeContainsByRefs();
}
return _fieldLayoutFlags.HasFlags(FieldLayoutFlags.ContainsByRefs);
}
}

/// <summary>
/// Does a type transitively have any fields which are marked with System.Runtime.CompilerServices.UnsafeValueTypeAttribute
/// </summary>
@@ -545,6 +570,20 @@ public void ComputeTypeContainsGCPointers()
_fieldLayoutFlags.AddFlags(flagsToAdd);
}

public void ComputeTypeContainsByRefs()
{
if (_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedContainsByRefs))
return;

int flagsToAdd = FieldLayoutFlags.ComputedContainsByRefs;
if (this.Context.GetLayoutAlgorithmForType(this).ComputeContainsByRefs(this))
{
flagsToAdd |= FieldLayoutFlags.ContainsByRefs;
}

_fieldLayoutFlags.AddFlags(flagsToAdd);
}

public void ComputeIsUnsafeValueType()
{
if (_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedIsUnsafeValueType))
Original file line number Diff line number Diff line change
@@ -74,20 +74,15 @@ protected override FieldLayoutTag GetIntervalDataForType(int offset, TypeDesc fi
}
}

protected override bool NeedsRecursiveLayout(int offset, TypeDesc fieldType)
protected override bool NeedsRecursiveLayout(TypeDesc fieldType)
{
if (!fieldType.IsValueType || !((MetadataType)fieldType).ContainsGCPointers && !fieldType.IsByRefLike)
if (!fieldType.IsValueType)
{
return false;
}

if (offset % PointerSize != 0)
{
// Misaligned struct with GC pointers or ByRef
ThrowFieldLayoutError(offset);
}

return true;
// Valuetypes with GC references and byref-like types need to be checked for overlaps and alignment
return ((MetadataType)fieldType).ContainsGCPointers || fieldType.IsByRefLike;
}

private void ThrowFieldLayoutError(int offset)
Original file line number Diff line number Diff line change
@@ -31,6 +31,11 @@ public abstract class FieldLayoutAlgorithm
/// </summary>
public abstract bool ComputeContainsGCPointers(DefType type);

/// <summary>
/// Compute whether the fields of the specified type contains a byref.
/// </summary>
public abstract bool ComputeContainsByRefs(DefType type);

/// <summary>
/// Compute whether the specified type is a value type that transitively has UnsafeValueTypeAttribute
/// </summary>
Original file line number Diff line number Diff line change
@@ -55,7 +55,7 @@ public FieldLayoutIntervalCalculator(int pointerSize)
PointerSize = pointerSize;
}

protected abstract bool NeedsRecursiveLayout(int offset, TypeDesc fieldType);
protected abstract bool NeedsRecursiveLayout(TypeDesc fieldType);

protected abstract TIntervalTag GetIntervalDataForType(int offset, TypeDesc fieldType);

@@ -84,7 +84,7 @@ private int GetFieldSize(TypeDesc fieldType)

public void AddToFieldLayout(int offset, TypeDesc fieldType, bool addTrailingEmptyInterval)
{
if (NeedsRecursiveLayout(offset, fieldType))
if (NeedsRecursiveLayout(fieldType))
{
if (fieldType is MetadataType { IsInlineArray: true } mdType)
{
@@ -126,7 +126,7 @@ public void AddToFieldLayout(int offset, TypeDesc fieldType, bool addTrailingEmp

private void AddToFieldLayout(List<FieldLayoutInterval> fieldLayout, int offset, TypeDesc fieldType)
{
if (NeedsRecursiveLayout(offset, fieldType))
if (NeedsRecursiveLayout(fieldType))
{
if (fieldType is MetadataType { IsInlineArray: true } mdType)
{
Original file line number Diff line number Diff line change
@@ -207,7 +207,7 @@ public override ComputedStaticFieldLayout ComputeStaticFieldLayout(DefType defTy
}

ref StaticsBlock block = ref GetStaticsBlockForField(ref result, field);
SizeAndAlignment sizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout: false, context.Target.DefaultPackingSize, out bool _, out bool _, out bool _, out bool _);
SizeAndAlignment sizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout: false, context.Target.DefaultPackingSize, out ComputedFieldData _);

block.Size = LayoutInt.AlignUp(block.Size, sizeAndAlignment.Alignment, context.Target);
result.Offsets[index] = new FieldAndOffset(field, block.Size);
@@ -269,6 +269,27 @@ public override bool ComputeContainsGCPointers(DefType type)
return someFieldContainsPointers;
}

public override bool ComputeContainsByRefs(DefType type)
{
if (!type.IsByRefLike)
return false;

foreach (var field in type.GetFields())
{
if (field.IsStatic)
continue;

TypeDesc fieldType = field.FieldType;
if (fieldType.IsByRef
|| ((fieldType is DefType df) && df.ContainsByRefs))
{
return true;
}
}

return false;
}

/// <summary>
/// Called during static field layout to setup initial contents of statics blocks
/// </summary>
@@ -291,7 +312,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false, requiresAlignedBase: false) - offsetBias;
LayoutInt instanceSize = cumulativeInstanceFieldPos + offsetBias;

var layoutMetadata = type.GetClassLayout();
ClassLayoutMetadata layoutMetadata = type.GetClassLayout();
int packingSize = ComputePackingSize(type, layoutMetadata);
LayoutInt largestAlignmentRequired = LayoutInt.One;

@@ -308,17 +329,17 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
hasVectorTField = type.BaseType.IsVectorTOrHasVectorTFields;
}

foreach (var fieldAndOffset in layoutMetadata.Offsets)
foreach (FieldAndOffset fieldAndOffset in layoutMetadata.Offsets)
{
TypeDesc fieldType = fieldAndOffset.Field.FieldType;
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable, out bool fieldHasAutoLayout, out bool fieldHasInt128Field, out bool fieldHasVectorTField);
if (!fieldLayoutAbiStable)
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType.UnderlyingType, hasLayout: true, packingSize, out ComputedFieldData fieldData);
if (!fieldData.LayoutAbiStable)
layoutAbiStable = false;
if (fieldHasAutoLayout)
if (fieldData.HasAutoLayout)
hasAutoLayoutField = true;
if (fieldHasInt128Field)
if (fieldData.HasInt128Field)
hasInt128Field = true;
if (fieldHasVectorTField)
if (fieldData.HasVectorTField)
hasVectorTField = true;

largestAlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequired);
@@ -329,14 +350,13 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
LayoutInt computedOffset = fieldAndOffset.Offset + cumulativeInstanceFieldPos + offsetBias;

// GC pointers MUST be aligned.
// We treat byref-like structs as GC pointers too.
bool needsToBeAligned =
!computedOffset.IsIndeterminate
&&
(
fieldType.IsGCPointer
|| fieldType.IsByRefLike
|| (fieldType.IsValueType && ((DefType)fieldType).ContainsGCPointers)
|| (fieldType.IsByRefLike && ((DefType)fieldType).ContainsByRefs)
);
if (needsToBeAligned)
{
@@ -422,14 +442,14 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
if (field.IsStatic)
continue;

var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable, out bool fieldHasAutoLayout, out bool fieldHasInt128Field, out bool fieldHasVectorTField);
if (!fieldLayoutAbiStable)
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType.UnderlyingType, hasLayout: true, packingSize, out ComputedFieldData fieldData);
if (!fieldData.LayoutAbiStable)
layoutAbiStable = false;
if (fieldHasAutoLayout)
if (fieldData.HasAutoLayout)
hasAutoLayoutField = true;
if (fieldHasInt128Field)
if (fieldData.HasInt128Field)
hasInt128Field = true;
if (fieldHasVectorTField)
if (fieldData.HasVectorTField)
hasVectorTField = true;

largestAlignmentRequirement = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequirement);
@@ -559,7 +579,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
{
Debug.Assert(fieldType.IsPrimitive || fieldType.IsPointer || fieldType.IsFunctionPointer || fieldType.IsEnum || fieldType.IsByRef);

var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out bool _, out bool _, out bool _, out bool _);
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out ComputedFieldData _);
instanceNonGCPointerFieldsCount[CalculateLog2(fieldSizeAndAlignment.Size.AsInt)]++;
}
}
@@ -596,8 +616,8 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,

TypeDesc fieldType = field.FieldType;

var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable, out bool _, out bool _, out bool _);
if (!fieldLayoutAbiStable)
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out ComputedFieldData fieldData);
if (!fieldData.LayoutAbiStable)
layoutAbiStable = false;

if (IsByValueClass(fieldType))
@@ -682,7 +702,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
int j;

// Check if the position is aligned such that we could place a larger type
int offsetModulo = cumulativeInstanceFieldPos.AsInt % (1 << (i+1));
int offsetModulo = cumulativeInstanceFieldPos.AsInt % (1 << (i + 1));
if (offsetModulo == 0)
{
continue;
@@ -766,8 +786,8 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
for (int i = 0; i < instanceValueClassFieldsArr.Length; i++)
{
// Align the cumulative field offset to the indeterminate value
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(instanceValueClassFieldsArr[i].FieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable, out bool _, out bool _, out bool _);
if (!fieldLayoutAbiStable)
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(instanceValueClassFieldsArr[i].FieldType, hasLayout, packingSize, out ComputedFieldData fieldData);
if (!fieldData.LayoutAbiStable)
layoutAbiStable = false;

cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, context.Target);
@@ -837,7 +857,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,

private static void PlaceInstanceField(FieldDesc field, bool hasLayout, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias)
{
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, hasLayout, packingSize, out bool _, out bool _, out bool _, out bool _);
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, hasLayout, packingSize, out ComputedFieldData _);

instanceFieldPos = AlignUpInstanceFieldOffset(instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target);
offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos + offsetBias);
@@ -850,9 +870,9 @@ private static void PlaceInstanceField(FieldDesc field, bool hasLayout, int pack
// This will calculate the next multiple of 4 that is greater than or equal to the instance size
private static LayoutInt GetAlignedNumInstanceFieldBytes(LayoutInt instanceSize)
{
uint inputSize = (uint) instanceSize.AsInt;
uint inputSize = (uint)instanceSize.AsInt;
uint result = (uint)(((inputSize + 3) & (~3)));
return new LayoutInt((int) result);
return new LayoutInt((int)result);
}

private static int CalculateLog2(int size)
@@ -861,7 +881,7 @@ private static int CalculateLog2(int size)
Debug.Assert(size > 0);

// Size must be a power of 2
Debug.Assert( 0 == (size & (size - 1)));
Debug.Assert(0 == (size & (size - 1)));

int log2size;
for (log2size = 0; size > 1; log2size++)
@@ -897,13 +917,25 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8
return cumulativeInstanceFieldPos;
}

private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, bool hasLayout, int packingSize, out bool layoutAbiStable, out bool fieldTypeHasAutoLayout, out bool fieldTypeHasInt128Field, out bool fieldTypeHasVectorTField)
private struct ComputedFieldData
{
public bool LayoutAbiStable;
public bool HasAutoLayout;
public bool HasInt128Field;
public bool HasVectorTField;
}

private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, bool hasLayout, int packingSize, out ComputedFieldData fieldData)
{
SizeAndAlignment result;
layoutAbiStable = true;
fieldTypeHasAutoLayout = true;
fieldTypeHasInt128Field = false;
fieldTypeHasVectorTField = false;
fieldData = new()
{
// Initialize to expected default value
LayoutAbiStable = true,
HasAutoLayout = true,
HasInt128Field = false,
HasVectorTField = false
};

if (fieldType.IsDefType)
{
@@ -912,10 +944,10 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType,
DefType defType = (DefType)fieldType;
result.Size = defType.InstanceFieldSize;
result.Alignment = defType.InstanceFieldAlignment;
layoutAbiStable = defType.LayoutAbiStable;
fieldTypeHasAutoLayout = defType.IsAutoLayoutOrHasAutoLayoutFields;
fieldTypeHasInt128Field = defType.IsInt128OrHasInt128Fields;
fieldTypeHasVectorTField = defType.IsVectorTOrHasVectorTFields;
fieldData.LayoutAbiStable = defType.LayoutAbiStable;
fieldData.HasAutoLayout = defType.IsAutoLayoutOrHasAutoLayoutFields;
fieldData.HasInt128Field = defType.IsInt128OrHasInt128Fields;
fieldData.HasVectorTField = defType.IsVectorTOrHasVectorTFields;
}
else
{
@@ -936,7 +968,7 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType,
Debug.Assert(fieldType.IsPointer || fieldType.IsFunctionPointer || fieldType.IsByRef);
result.Size = fieldType.Context.Target.LayoutPointerSize;
result.Alignment = fieldType.Context.Target.LayoutPointerSize;
fieldTypeHasAutoLayout = fieldType.IsByRef;
fieldData.HasAutoLayout = fieldType.IsByRef;
}

// For non-auto layouts, we need to respect tighter packing requests for alignment.
Loading