From 053a472be0143bda0891c715d7b2ce7398538127 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 17 Feb 2022 13:19:18 -0800 Subject: [PATCH] Update tests to help uncover CrossGen2 scenario validation (#65477) * 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. --- .../Common/ExplicitLayoutValidator.cs | 39 +++++++++---------- src/mono/mono/metadata/class-init.c | 27 ++++++++++++- .../classloader/RefFields/InvalidCSharp.il | 1 + .../Loader/classloader/RefFields/Validate.cs | 35 ++++++++++++++++- 4 files changed, 78 insertions(+), 24 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs b/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs index 8394ff897db..2decce4fa74 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs @@ -14,6 +14,7 @@ private enum FieldLayoutTag : byte Empty, NonORef, ORef, + ByRef, } private struct FieldLayoutInterval : IComparable @@ -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); @@ -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 fieldORefMap = new List(); - MarkORefLocations(mdType, fieldORefMap, offset: 0); + List 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); } @@ -142,7 +137,7 @@ private void AddToFieldLayout(int offset, TypeDesc fieldType) // Misaligned pointer field ThrowFieldLayoutError(offset); } - SetFieldLayout(offset, _pointerSize, FieldLayoutTag.NonORef); + SetFieldLayout(offset, _pointerSize, FieldLayoutTag.ByRef); } else { @@ -150,7 +145,7 @@ private void AddToFieldLayout(int offset, TypeDesc fieldType) } } - private void MarkORefLocations(MetadataType type, List orefMap, int offset) + private void MarkByRefAndORefLocations(MetadataType type, List refMap, int offset) { // Recurse into struct fields foreach (FieldDesc field in type.GetFields()) @@ -160,14 +155,18 @@ private void MarkORefLocations(MetadataType type, List 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); } } } diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index f0e1fdb2c6b..65e348e333a 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -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; } @@ -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); @@ -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. diff --git a/src/tests/Loader/classloader/RefFields/InvalidCSharp.il b/src/tests/Loader/classloader/RefFields/InvalidCSharp.il index f45e7d3a704..5f014841c89 100644 --- a/src/tests/Loader/classloader/RefFields/InvalidCSharp.il +++ b/src/tests/Loader/classloader/RefFields/InvalidCSharp.il @@ -61,6 +61,7 @@ 01 00 00 00 ) .field public int32& Field + .field public int32 Size } .class public explicit ansi sealed beforefieldinit InvalidCSharp.IntPtrOverlapWithInnerFieldType diff --git a/src/tests/Loader/classloader/RefFields/Validate.cs b/src/tests/Loader/classloader/RefFields/Validate.cs index 22c72e212e1..68b505dc76c 100644 --- a/src/tests/Loader/classloader/RefFields/Validate.cs +++ b/src/tests/Loader/classloader/RefFields/Validate.cs @@ -4,12 +4,20 @@ 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 Bytes; + [FieldOffset(0)] public Guid Guid; + } + [Fact] public static void Validate_Invalid_RefField_Fails() { @@ -17,8 +25,31 @@ public static void Validate_Invalid_RefField_Fails() Assert.Throws(() => { var t = typeof(InvalidStructWithRefField); }); Assert.Throws(() => { var t = typeof(InvalidRefFieldAlignment); }); Assert.Throws(() => { var t = typeof(InvalidObjectRefRefFieldOverlap); }); - Assert.Throws(() => { var t = typeof(IntPtrRefFieldOverlap); }); - Assert.Throws(() => { var t = typeof(IntPtrOverlapWithInnerFieldType); }); + Assert.Throws(() => + { + var t = new IntPtrRefFieldOverlap() + { + Field = IntPtr.Zero + }; + return t.Field.ToString(); + }); + Assert.Throws(() => + { + var t = new IntPtrOverlapWithInnerFieldType() + { + Field = IntPtr.Zero + }; + ref var i = ref t.Invalid; + return i.Size; + }); + Assert.Throws(() => + { + var t = new Explicit() + { + Guid = Guid.NewGuid() + }; + return t.Bytes.Length; + }); } [Fact] -- GitLab