Skip to content

Commit

Permalink
Update tests to help uncover CrossGen2 scenario validation (#65477)
Browse files Browse the repository at this point in the history 
* Update CrossGen2 to support ByRef fields in all scenarios

* Update tests to help uncover CrossGen2 scenario validation

* Add support in Mono for the ByReference`1 being considered a ref field
  during Explicit offset validation.
  • Loading branch information
@AaronRobinsonMSFT
AaronRobinsonMSFT authored Feb 17, 2022
1 parent ea5376a commit 053a472
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 24 deletions.
39 changes: 19 additions & 20 deletions src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ private enum FieldLayoutTag : byte
Empty,
NonORef,
ORef,
ByRef,
}

private struct FieldLayoutInterval : IComparable<FieldLayoutInterval>
Expand Down Expand Up @@ -92,15 +93,9 @@ private void AddToFieldLayout(int offset, TypeDesc fieldType)
}
else if (fieldType.IsValueType)
{
if (fieldType.IsByRefLike && offset % _pointerSize != 0)
{
// Misaligned ByRefLike
ThrowFieldLayoutError(offset);
}

MetadataType mdType = (MetadataType)fieldType;
int fieldSize = mdType.InstanceByteCountUnaligned.AsInt;
if (!mdType.ContainsGCPointers)
if (!mdType.ContainsGCPointers && !mdType.IsByRefLike)
{
// Plain value type, mark the entire range as NonORef
SetFieldLayout(offset, fieldSize, FieldLayoutTag.NonORef);
Expand All @@ -109,27 +104,27 @@ private void AddToFieldLayout(int offset, TypeDesc fieldType)
{
if (offset % _pointerSize != 0)
{
// Misaligned struct with GC pointers
// Misaligned struct with GC pointers or ByRef
ThrowFieldLayoutError(offset);
}

List<FieldLayoutInterval> fieldORefMap = new List<FieldLayoutInterval>();
MarkORefLocations(mdType, fieldORefMap, offset: 0);
List<FieldLayoutInterval> fieldRefMap = new();
MarkByRefAndORefLocations(mdType, fieldRefMap, offset: 0);

// Merge in fieldORefMap from structure specifying not attributed intervals as NonORef
// Merge in fieldRefMap from structure specifying not attributed intervals as NonORef
int lastGCRegionReportedEnd = 0;

foreach (var gcRegion in fieldORefMap)
foreach (var gcRegion in fieldRefMap)
{
SetFieldLayout(offset + lastGCRegionReportedEnd, gcRegion.Start - lastGCRegionReportedEnd, FieldLayoutTag.NonORef);
Debug.Assert(gcRegion.Tag == FieldLayoutTag.ORef);
Debug.Assert(gcRegion.Tag == FieldLayoutTag.ORef || gcRegion.Tag == FieldLayoutTag.ByRef);
SetFieldLayout(offset + gcRegion.Start, gcRegion.Size, gcRegion.Tag);
lastGCRegionReportedEnd = gcRegion.EndSentinel;
}

if (fieldORefMap.Count > 0)
if (fieldRefMap.Count > 0)
{
int trailingRegionStart = fieldORefMap[fieldORefMap.Count - 1].EndSentinel;
int trailingRegionStart = fieldRefMap[fieldRefMap.Count - 1].EndSentinel;
int trailingRegionSize = fieldSize - trailingRegionStart;
SetFieldLayout(offset + trailingRegionStart, trailingRegionSize, FieldLayoutTag.NonORef);
}
Expand All @@ -142,15 +137,15 @@ private void AddToFieldLayout(int offset, TypeDesc fieldType)
// Misaligned pointer field
ThrowFieldLayoutError(offset);
}
SetFieldLayout(offset, _pointerSize, FieldLayoutTag.NonORef);
SetFieldLayout(offset, _pointerSize, FieldLayoutTag.ByRef);
}
else
{
Debug.Assert(false, fieldType.ToString());
}
}

private void MarkORefLocations(MetadataType type, List<FieldLayoutInterval> orefMap, int offset)
private void MarkByRefAndORefLocations(MetadataType type, List<FieldLayoutInterval> refMap, int offset)
{
// Recurse into struct fields
foreach (FieldDesc field in type.GetFields())
Expand All @@ -160,14 +155,18 @@ private void MarkORefLocations(MetadataType type, List<FieldLayoutInterval> oref
int fieldOffset = offset + field.Offset.AsInt;
if (field.FieldType.IsGCPointer)
{
SetFieldLayout(orefMap, offset, _pointerSize, FieldLayoutTag.ORef);
SetFieldLayout(refMap, offset, _pointerSize, FieldLayoutTag.ORef);
}
else if (field.FieldType.IsByRef || field.FieldType.IsByReferenceOfT)
{
SetFieldLayout(refMap, offset, _pointerSize, FieldLayoutTag.ByRef);
}
else if (field.FieldType.IsValueType)
{
MetadataType mdFieldType = (MetadataType)field.FieldType;
if (mdFieldType.ContainsGCPointers)
if (mdFieldType.ContainsGCPointers || mdFieldType.IsByRefLike)
{
MarkORefLocations(mdFieldType, orefMap, fieldOffset);
MarkByRefAndORefLocations(mdFieldType, refMap, fieldOffset);
}
}
}
Expand Down
27 changes: 25 additions & 2 deletions src/mono/mono/metadata/class-init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1843,12 +1843,35 @@ class_has_ref_fields (MonoClass *klass)
return klass->has_ref_fields;
}

static gboolean
class_is_byreference (MonoClass* klass)
{
const char* klass_name_space = m_class_get_name_space (klass);
const char* klass_name = m_class_get_name (klass);
MonoImage* klass_image = m_class_get_image (klass);
gboolean in_corlib = klass_image == mono_defaults.corlib;

if (in_corlib &&
!strcmp (klass_name_space, "System") &&
!strcmp (klass_name, "ByReference`1")) {
return TRUE;
}

return FALSE;
}

static gboolean
type_has_ref_fields (MonoType *ftype)
{
if (m_type_is_byref (ftype) || (MONO_TYPE_ISSTRUCT (ftype) && class_has_ref_fields (mono_class_from_mono_type_internal (ftype))))
return TRUE;

/* Check for the ByReference`1 type */
if (MONO_TYPE_ISSTRUCT (ftype)) {
MonoClass* klass = mono_class_from_mono_type_internal (ftype);
return class_is_byreference (klass);
}

return FALSE;
}

Expand Down Expand Up @@ -1942,7 +1965,7 @@ validate_struct_fields_overlaps (guint8 *layout_check, int layout_size, MonoClas
if (mono_type_is_struct (ftype)) {
// recursively check the layout of the embedded struct
MonoClass *embedded_class = mono_class_from_mono_type_internal (ftype);
mono_class_setup_fields (embedded_class);
mono_class_setup_fields (embedded_class);

const int embedded_fields_count = mono_class_get_field_count (embedded_class);
int *embedded_offsets = g_new0 (int, embedded_fields_count);
Expand All @@ -1962,7 +1985,7 @@ validate_struct_fields_overlaps (guint8 *layout_check, int layout_size, MonoClas
} else {
int align = 0;
int size = mono_type_size (field->type, &align);
guint8 type = type_has_references (klass, ftype) ? 1 : m_type_is_byref (ftype) ? 2 : 3;
guint8 type = type_has_references (klass, ftype) ? 1 : (m_type_is_byref (ftype) || class_is_byreference (klass)) ? 2 : 3;

// Mark the bytes used by this fields type based on if it contains references or not.
// Make sure there are no overlaps between object and non-object fields.
Expand Down
1 change: 1 addition & 0 deletions src/tests/Loader/classloader/RefFields/InvalidCSharp.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
01 00 00 00
)
.field public int32& Field
.field public int32 Size
}

.class public explicit ansi sealed beforefieldinit InvalidCSharp.IntPtrOverlapWithInnerFieldType
Expand Down
35 changes: 33 additions & 2 deletions src/tests/Loader/classloader/RefFields/Validate.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,52 @@
using System;
using System.IO;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using InvalidCSharp;

using Xunit;

class Validate
{
[StructLayout(LayoutKind.Explicit)]
private ref struct Explicit
{
[FieldOffset(0)] public Span<byte> Bytes;
[FieldOffset(0)] public Guid Guid;
}

[Fact]
public static void Validate_Invalid_RefField_Fails()
{
Console.WriteLine($"{nameof(Validate_Invalid_RefField_Fails)}...");
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidStructWithRefField); });
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidRefFieldAlignment); });
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidObjectRefRefFieldOverlap); });
Assert.Throws<TypeLoadException>(() => { var t = typeof(IntPtrRefFieldOverlap); });
Assert.Throws<TypeLoadException>(() => { var t = typeof(IntPtrOverlapWithInnerFieldType); });
Assert.Throws<TypeLoadException>(() =>
{
var t = new IntPtrRefFieldOverlap()
{
Field = IntPtr.Zero
};
return t.Field.ToString();
});
Assert.Throws<TypeLoadException>(() =>
{
var t = new IntPtrOverlapWithInnerFieldType()
{
Field = IntPtr.Zero
};
ref var i = ref t.Invalid;
return i.Size;
});
Assert.Throws<TypeLoadException>(() =>
{
var t = new Explicit()
{
Guid = Guid.NewGuid()
};
return t.Bytes.Length;
});
}

[Fact]
Expand Down

0 comments on commit 053a472

Please sign in to comment.