Skip to content

Fix explicit offset of ByRefLike fields. #111584

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

Merged
merged 16 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
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
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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>
Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)]++;
}
}
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand All @@ -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++)
Expand Down Expand Up @@ -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)
{
Expand All @@ -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
{
Expand All @@ -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.
Expand Down
Loading
Loading