Skip to content

Commit c74241f

Browse files
jkoritzinskyCopilot
jkoritzinsky
and
Copilot
authored
Disallow loading non-Explicit types with explicit field offsets (#113500)
* Throw TLE if 'sequence' feature is used * Disallow loading non-Explicit types with explicit field offsets * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Make methods local methods * Try to create an instance instead * Throw exceptions in mono and nativeaot * Fix check during non-explicit type layout. * Introduce FieldDesc.MetadataOffset remove field offsets from ClassLayoutMetadata * PR feedback and fix build for System.Private.TypeLoader * Move FieldAndOffset definition * Don't check offsets for AutoLayout * Don't error out on non-explicit offest information for sequential fields either. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 0ff4bf2 commit c74241f

13 files changed

+99
-117
lines changed

‎src/coreclr/tools/Common/TypeSystem/Common/CastingHelper.TypeEquivalence.cs

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,15 @@ static bool CompareStructuresForEquivalence(MetadataType type1, MetadataType typ
202202
return false;
203203
}
204204

205+
bool explicitLayout = false;
206+
if (!enumMode)
207+
{
208+
if (!CompareTypeLayout(type1, type2, out explicitLayout))
209+
{
210+
return false;
211+
}
212+
}
213+
205214
// Compare field types for equivalence
206215
var fields1 = type1.GetFields().GetEnumerator();
207216
var fields2 = type2.GetFields().GetEnumerator();
@@ -249,20 +258,22 @@ static bool CompareStructuresForEquivalence(MetadataType type1, MetadataType typ
249258
{
250259
return false;
251260
}
252-
}
253261

254-
// At this point we know that the set of fields is the same, and have the same types
255-
if (!enumMode)
256-
{
257-
if (!CompareTypeLayout(type1, type2))
262+
// If we are in explicit layout mode, we need to compare the offsets
263+
if (explicitLayout)
258264
{
259-
return false;
265+
if (field1.MetadataOffset != field2.MetadataOffset)
266+
{
267+
return false;
268+
}
260269
}
261270
}
271+
262272
return true;
263273

264-
static bool CompareTypeLayout(MetadataType type1, MetadataType type2)
274+
static bool CompareTypeLayout(MetadataType type1, MetadataType type2, out bool explicitLayout)
265275
{
276+
explicitLayout = false;
266277
// Types must either be Sequential or Explicit layout
267278
if (type1.IsSequentialLayout != type2.IsSequentialLayout)
268279
{
@@ -279,7 +290,7 @@ static bool CompareTypeLayout(MetadataType type1, MetadataType type2)
279290
return false;
280291
}
281292

282-
bool explicitLayout = type1.IsExplicitLayout;
293+
explicitLayout = type1.IsExplicitLayout;
283294

284295
// they must have the same charset
285296
if (type1.PInvokeStringFormat != type2.PInvokeStringFormat)
@@ -293,21 +304,6 @@ static bool CompareTypeLayout(MetadataType type1, MetadataType type2)
293304
(layoutMetadata1.Size != layoutMetadata2.Size))
294305
return false;
295306

296-
if ((explicitLayout) && !(layoutMetadata1.Offsets == null && layoutMetadata2.Offsets == null))
297-
{
298-
if (layoutMetadata1.Offsets == null)
299-
return false;
300-
301-
if (layoutMetadata2.Offsets == null)
302-
return false;
303-
304-
for (int index = 0; index < layoutMetadata1.Offsets.Length; index++)
305-
{
306-
if (layoutMetadata1.Offsets[index].Offset != layoutMetadata2.Offsets[index].Offset)
307-
return false;
308-
}
309-
}
310-
311307
return true;
312308
}
313309

‎src/coreclr/tools/Common/TypeSystem/Common/FieldDesc.FieldLayout.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,13 @@ internal void InitializeOffset(LayoutInt offset)
4747
Debug.Assert(_offset == FieldAndOffset.InvalidOffset || _offset == offset);
4848
_offset = offset;
4949
}
50+
51+
public virtual LayoutInt MetadataOffset
52+
{
53+
get
54+
{
55+
return LayoutInt.Indeterminate;
56+
}
57+
}
5058
}
5159
}

‎src/coreclr/tools/Common/TypeSystem/Common/FieldForInstantiatedType.FieldLayout.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Debug = System.Diagnostics.Debug;
5+
6+
namespace Internal.TypeSystem
7+
{
8+
public sealed partial class FieldForInstantiatedType : FieldDesc
9+
{
10+
public override LayoutInt MetadataOffset
11+
{
12+
get
13+
{
14+
return _fieldDef.MetadataOffset;
15+
}
16+
}
17+
}
18+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,21 @@ public enum StaticLayoutKind
8080
StaticRegionSizesAndFields
8181
}
8282

83+
public struct FieldAndOffset
84+
{
85+
public static readonly LayoutInt InvalidOffset = new LayoutInt(int.MaxValue);
86+
87+
public readonly FieldDesc Field;
88+
89+
public readonly LayoutInt Offset;
90+
91+
public FieldAndOffset(FieldDesc field, LayoutInt offset)
92+
{
93+
Field = field;
94+
Offset = offset;
95+
}
96+
}
97+
8398
public struct ComputedInstanceFieldLayout
8499
{
85100
public LayoutInt FieldSize;

‎src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,12 @@ out instanceByteSizeAndAlignment
144144
}
145145

146146
var layoutMetadata = type.GetClassLayout();
147-
148147
// If packing is out of range or not a power of two, throw that the size is invalid
149148
int packing = layoutMetadata.PackingSize;
150149
if (packing < 0 || packing > 128 || ((packing & (packing - 1)) != 0))
151150
{
152151
ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadBadFormat, type);
153152
}
154-
155-
Debug.Assert(layoutMetadata.Offsets == null || layoutMetadata.Offsets.Length == numInstanceFields);
156153
}
157154

158155
// At this point all special cases are handled and all inputs validated
@@ -329,9 +326,12 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
329326
hasVectorTField = type.BaseType.IsVectorTOrHasVectorTFields;
330327
}
331328

332-
foreach (FieldAndOffset fieldAndOffset in layoutMetadata.Offsets)
329+
foreach (FieldDesc field in type.GetFields())
333330
{
334-
TypeDesc fieldType = fieldAndOffset.Field.FieldType;
331+
if (field.IsStatic)
332+
continue;
333+
334+
TypeDesc fieldType = field.FieldType;
335335
var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType.UnderlyingType, hasLayout: true, packingSize, out ComputedFieldData fieldData);
336336
if (!fieldData.LayoutAbiStable)
337337
layoutAbiStable = false;
@@ -344,10 +344,12 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
344344

345345
largestAlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequired);
346346

347-
if (fieldAndOffset.Offset == FieldAndOffset.InvalidOffset)
347+
LayoutInt metadataOffset = field.MetadataOffset;
348+
349+
if (metadataOffset == LayoutInt.Indeterminate)
348350
ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadBadFormat, type);
349351

350-
LayoutInt computedOffset = fieldAndOffset.Offset + cumulativeInstanceFieldPos + offsetBias;
352+
LayoutInt computedOffset = metadataOffset + cumulativeInstanceFieldPos + offsetBias;
351353

352354
// GC pointers MUST be aligned.
353355
bool needsToBeAligned =
@@ -363,11 +365,11 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
363365
int offsetModulo = computedOffset.AsInt % type.Context.Target.PointerSize;
364366
if (offsetModulo != 0)
365367
{
366-
ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadExplicitLayout, type, fieldAndOffset.Offset.ToStringInvariant());
368+
ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadExplicitLayout, type, metadataOffset.ToStringInvariant());
367369
}
368370
}
369371

370-
offsets[fieldOrdinal] = new FieldAndOffset(fieldAndOffset.Field, computedOffset);
372+
offsets[fieldOrdinal] = new FieldAndOffset(field, computedOffset);
371373

372374
LayoutInt fieldExtent = computedOffset + fieldSizeAndAlignment.Size;
373375
instanceSize = LayoutInt.Max(fieldExtent, instanceSize);

‎src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,5 @@ public struct ClassLayoutMetadata
110110
{
111111
public int PackingSize;
112112
public int Size;
113-
public FieldAndOffset[] Offsets;
114-
}
115-
116-
public struct FieldAndOffset
117-
{
118-
public static readonly LayoutInt InvalidOffset = new LayoutInt(int.MaxValue);
119-
120-
public readonly FieldDesc Field;
121-
122-
public readonly LayoutInt Offset;
123-
124-
public FieldAndOffset(FieldDesc field, LayoutInt offset)
125-
{
126-
Field = field;
127-
Offset = offset;
128-
}
129113
}
130114
}

‎src/coreclr/tools/Common/TypeSystem/Ecma/EcmaField.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,15 @@ public override MarshalAsDescriptor GetMarshalAsDescriptor()
297297

298298
return null;
299299
}
300+
301+
public override LayoutInt MetadataOffset
302+
{
303+
get
304+
{
305+
int offset = MetadataReader.GetFieldDefinition(_handle).GetOffset();
306+
return offset == -1 ? LayoutInt.Indeterminate : new LayoutInt(offset);
307+
}
308+
}
300309
}
301310

302311
public static class EcmaFieldExtensions

‎src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -567,46 +567,11 @@ public override ClassLayoutMetadata GetClassLayout()
567567
{
568568
TypeLayout layout = _typeDefinition.GetLayout();
569569

570-
ClassLayoutMetadata result;
571-
result.PackingSize = layout.PackingSize;
572-
result.Size = layout.Size;
573-
574-
// Skip reading field offsets if this is not explicit layout
575-
if (IsExplicitLayout)
570+
return new ClassLayoutMetadata
576571
{
577-
var fieldDefinitionHandles = _typeDefinition.GetFields();
578-
var numInstanceFields = 0;
579-
580-
foreach (var handle in fieldDefinitionHandles)
581-
{
582-
var fieldDefinition = MetadataReader.GetFieldDefinition(handle);
583-
if ((fieldDefinition.Attributes & FieldAttributes.Static) != 0)
584-
continue;
585-
586-
numInstanceFields++;
587-
}
588-
589-
result.Offsets = new FieldAndOffset[numInstanceFields];
590-
591-
int index = 0;
592-
foreach (var handle in fieldDefinitionHandles)
593-
{
594-
var fieldDefinition = MetadataReader.GetFieldDefinition(handle);
595-
if ((fieldDefinition.Attributes & FieldAttributes.Static) != 0)
596-
continue;
597-
598-
// Note: GetOffset() returns -1 when offset was not set in the metadata
599-
int specifiedOffset = fieldDefinition.GetOffset();
600-
result.Offsets[index] =
601-
new FieldAndOffset(_module.GetField(handle, this), specifiedOffset == -1 ? FieldAndOffset.InvalidOffset : new LayoutInt(specifiedOffset));
602-
603-
index++;
604-
}
605-
}
606-
else
607-
result.Offsets = null;
608-
609-
return result;
572+
PackingSize = layout.PackingSize,
573+
Size = layout.Size
574+
};
610575
}
611576

612577
public override bool IsExplicitLayout

‎src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -249,29 +249,6 @@ public override ClassLayoutMetadata GetClassLayout()
249249
result.PackingSize = layout.PackingSize;
250250
result.Size = layout.Size;
251251

252-
if (IsExplicitLayout)
253-
{
254-
result.Offsets = new FieldAndOffset[layout.Offsets.Length];
255-
256-
Debug.Assert(layout.Offsets.Length <= _fields.Length);
257-
258-
int layoutIndex = 0;
259-
for (int index = 0; index < _fields.Length; index++)
260-
{
261-
if (_fields[index].Name == layout.Offsets[layoutIndex].Field.Name)
262-
{
263-
result.Offsets[layoutIndex] = new FieldAndOffset(_fields[index], layout.Offsets[layoutIndex].Offset);
264-
layoutIndex++;
265-
}
266-
}
267-
268-
Debug.Assert(layoutIndex == layout.Offsets.Length);
269-
}
270-
else
271-
{
272-
result.Offsets = null;
273-
}
274-
275252
return result;
276253
}
277254

@@ -440,6 +417,14 @@ public override string Name
440417
}
441418
}
442419

420+
public override LayoutInt MetadataOffset
421+
{
422+
get
423+
{
424+
return _managedField.MetadataOffset;
425+
}
426+
}
427+
443428
public NativeStructField(TypeDesc nativeType, MetadataType owningType, FieldDesc managedField)
444429
{
445430
_fieldType = nativeType;

‎src/coreclr/tools/ILVerification/ILVerification.projitems

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@
9999
<Compile Include="$(ToolsCommonPath)TypeSystem\Common\FieldForInstantiatedType.cs">
100100
<Link>TypeSystem\Common\FieldForInstantiatedType.cs</Link>
101101
</Compile>
102+
<Compile Include="$(ToolsCommonPath)TypeSystem\Common\FieldForInstantiatedType.FieldLayout.cs">
103+
<Link>TypeSystem\Common\FieldForInstantiatedType.FieldLayout.cs</Link>
104+
</Compile>
102105
<Compile Include="$(ToolsCommonPath)TypeSystem\Common\FieldDesc.cs">
103106
<Link>TypeSystem\Common\FieldDesc.cs</Link>
104107
</Compile>

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.GeneratedAssembly.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ public override ClassLayoutMetadata GetClassLayout()
153153
{
154154
return new ClassLayoutMetadata
155155
{
156-
Offsets = null,
157156
PackingSize = 0,
158157
Size = 0,
159158
};

‎src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@
237237
<Compile Include="..\..\Common\TypeSystem\Common\FieldForInstantiatedType.cs">
238238
<Link>TypeSystem\Common\FieldForInstantiatedType.cs</Link>
239239
</Compile>
240+
<Compile Include="..\..\Common\TypeSystem\Common\FieldForInstantiatedType.FieldLayout.cs">
241+
<Link>TypeSystem\Common\FieldForInstantiatedType.FieldLayout.cs</Link>
242+
</Compile>
240243
<Compile Include="..\..\Common\TypeSystem\Common\FieldDesc.cs">
241244
<Link>TypeSystem\Common\FieldDesc.cs</Link>
242245
</Compile>

‎src/coreclr/vm/classlayoutinfo.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,7 @@ namespace
4444
// if we haven't found a matching token, it must be a static field with layout -- ignore it
4545
if (pfwalk->m_MD != fd) continue;
4646

47-
if (!fExplicitOffsets)
48-
{
49-
// ulOffset is the sequence
50-
pfwalk->m_sequence = ulOffset;
51-
}
52-
else
47+
if (fExplicitOffsets)
5348
{
5449
// ulOffset is the explicit offset
5550
pfwalk->m_placement.m_offset = ulOffset;

0 commit comments

Comments
 (0)