Skip to content

Commit 847a8c2

Browse files
Fix explicit offset of ByRefLike fields. (#111584)
* Fix explicit offset of ByRefLike fields. A ByRefLike type was previously forced to be pointer aligned so ref fields were implicitly aligned. Using Explicit layout one can create valid aligned ByRefLike types with no object pointers. This change tkaes into account the base offset of the ByRefLike type and includes that for determining offsets of fields in the ByRefLike type. --------- Co-authored-by: Michal Strehovský <[email protected]>
1 parent 55e9ab5 commit 847a8c2

File tree

19 files changed

+307
-79
lines changed

19 files changed

+307
-79
lines changed

src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ public override bool ComputeContainsGCPointers(DefType type)
6161
return false;
6262
}
6363

64+
public override bool ComputeContainsByRefs(DefType type)
65+
{
66+
Debug.Assert(!_fallbackAlgorithm.ComputeContainsByRefs(type));
67+
return false;
68+
}
69+
6470
public override bool ComputeIsUnsafeValueType(DefType type)
6571
{
6672
Debug.Assert(!_fallbackAlgorithm.ComputeIsUnsafeValueType(type));

src/coreclr/tools/Common/Compiler/VectorFieldLayoutAlgorithm.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ public override bool ComputeContainsGCPointers(DefType type)
133133
return false;
134134
}
135135

136+
public override bool ComputeContainsByRefs(DefType type)
137+
{
138+
Debug.Assert(!_fallbackAlgorithm.ComputeContainsByRefs(type));
139+
return false;
140+
}
141+
136142
public override bool ComputeIsUnsafeValueType(DefType type)
137143
{
138144
Debug.Assert(!_fallbackAlgorithm.ComputeIsUnsafeValueType(type));

src/coreclr/tools/Common/JitInterface/SwiftPhysicalLowering.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ protected override LoweredType GetIntervalDataForType(int offset, TypeDesc field
101101
return LoweredType.Opaque;
102102
}
103103

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

106106
private List<FieldLayoutInterval> CreateConsolidatedIntervals()
107107
{

src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ private static class FieldLayoutFlags
7878
/// True if the type transitively has a Vector<T> in it or is Vector<T>
7979
/// </summary>
8080
public const int IsVectorTOrHasVectorTFields = 0x1000;
81+
82+
/// <summary>
83+
/// True if ContainsByRefs has been computed
84+
/// </summary>
85+
public const int ComputedContainsByRefs = 0x2000;
86+
87+
/// <summary>
88+
/// True if the type contains byrefs
89+
/// </summary>
90+
public const int ContainsByRefs = 0x4000;
8191
}
8292

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

126+
/// <summary>
127+
/// Does a type transitively have any fields which are byrefs
128+
/// </summary>
129+
public bool ContainsByRefs
130+
{
131+
get
132+
{
133+
if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedContainsByRefs))
134+
{
135+
ComputeTypeContainsByRefs();
136+
}
137+
return _fieldLayoutFlags.HasFlags(FieldLayoutFlags.ContainsByRefs);
138+
}
139+
}
140+
116141
/// <summary>
117142
/// Does a type transitively have any fields which are marked with System.Runtime.CompilerServices.UnsafeValueTypeAttribute
118143
/// </summary>
@@ -545,6 +570,20 @@ public void ComputeTypeContainsGCPointers()
545570
_fieldLayoutFlags.AddFlags(flagsToAdd);
546571
}
547572

573+
public void ComputeTypeContainsByRefs()
574+
{
575+
if (_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedContainsByRefs))
576+
return;
577+
578+
int flagsToAdd = FieldLayoutFlags.ComputedContainsByRefs;
579+
if (this.Context.GetLayoutAlgorithmForType(this).ComputeContainsByRefs(this))
580+
{
581+
flagsToAdd |= FieldLayoutFlags.ContainsByRefs;
582+
}
583+
584+
_fieldLayoutFlags.AddFlags(flagsToAdd);
585+
}
586+
548587
public void ComputeIsUnsafeValueType()
549588
{
550589
if (_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedIsUnsafeValueType))

src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ protected override FieldLayoutTag GetIntervalDataForType(int offset, TypeDesc fi
6060
else if (fieldType.IsValueType)
6161
{
6262
MetadataType mdType = (MetadataType)fieldType;
63-
if (!mdType.ContainsGCPointers && !mdType.IsByRefLike)
63+
if (!mdType.ContainsGCPointers && !mdType.ContainsByRefs)
6464
{
6565
// Plain value type, mark the entire range as NonORef
6666
return FieldLayoutTag.NonORef;
6767
}
68-
Debug.Fail("We should recurse on value types with GC pointers or ByRefLike types");
68+
Debug.Fail("We should recurse on value types with GC pointers or byrefs");
6969
return FieldLayoutTag.Empty;
7070
}
7171
else
@@ -74,20 +74,16 @@ protected override FieldLayoutTag GetIntervalDataForType(int offset, TypeDesc fi
7474
}
7575
}
7676

77-
protected override bool NeedsRecursiveLayout(int offset, TypeDesc fieldType)
77+
protected override bool NeedsRecursiveLayout(TypeDesc fieldType)
7878
{
79-
if (!fieldType.IsValueType || !((MetadataType)fieldType).ContainsGCPointers && !fieldType.IsByRefLike)
79+
if (!fieldType.IsValueType)
8080
{
8181
return false;
8282
}
8383

84-
if (offset % PointerSize != 0)
85-
{
86-
// Misaligned struct with GC pointers or ByRef
87-
ThrowFieldLayoutError(offset);
88-
}
89-
90-
return true;
84+
// Valuetypes with GC references or byrefs need to be checked for overlaps and alignment
85+
MetadataType mdType = (MetadataType)fieldType;
86+
return mdType.ContainsGCPointers || mdType.ContainsByRefs;
9187
}
9288

9389
private void ThrowFieldLayoutError(int offset)

src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutAlgorithm.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ public abstract class FieldLayoutAlgorithm
3131
/// </summary>
3232
public abstract bool ComputeContainsGCPointers(DefType type);
3333

34+
/// <summary>
35+
/// Compute whether the fields of the specified type contains a byref.
36+
/// </summary>
37+
public abstract bool ComputeContainsByRefs(DefType type);
38+
3439
/// <summary>
3540
/// Compute whether the specified type is a value type that transitively has UnsafeValueTypeAttribute
3641
/// </summary>

src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutIntervalCalculator.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public FieldLayoutIntervalCalculator(int pointerSize)
5555
PointerSize = pointerSize;
5656
}
5757

58-
protected abstract bool NeedsRecursiveLayout(int offset, TypeDesc fieldType);
58+
protected abstract bool NeedsRecursiveLayout(TypeDesc fieldType);
5959

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

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

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

127127
private void AddToFieldLayout(List<FieldLayoutInterval> fieldLayout, int offset, TypeDesc fieldType)
128128
{
129-
if (NeedsRecursiveLayout(offset, fieldType))
129+
if (NeedsRecursiveLayout(fieldType))
130130
{
131131
if (fieldType is MetadataType { IsInlineArray: true } mdType)
132132
{

0 commit comments

Comments
 (0)