From: Simon Rozsival Date: Thu, 2 Dec 2021 13:08:32 +0000 (+0100) Subject: [Mono] Fix support for nested structs with explicit layout (#61467) X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~12009 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=64d127653b6100174d80ac36c89f660de224a6da;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [Mono] Fix support for nested structs with explicit layout (#61467) * Add test case for nested struct with explicit layout * Add license * Modify test * Temporarily enable the test in the CI pipeline * Allow running Mac Catalyst builds in App Sandbox * Allow enabling app sandbox for the other Mac Catalyst sample * Revert "Allow enabling app sandbox for the other Mac Catalyst sample" This reverts commit 8c3c9f92daa47d1de844dbe897155d119415fb74. * Revert "Allow running Mac Catalyst builds in App Sandbox" This reverts commit 1e404165d1859bc9e25dbcdb0b4dc74a839dc708. * Add more test cases * WIP: First implementation of the recursive check * Improve code * Fix test case * Add more test cases * Unify test cases * Bugfixes * Remove test which behaves differently depending on the target platform * Remove sequential layout test which behaves differently on Linux and on Windows * Reorganize test cases * Remove call to mono_class_setup_fields * Fix embedding detection * Fix layout validation for generic structs * Remove unintentional change * Revert temporary change to the runtime.yml pipeline * Code clean-up * Revert unrelated change * Use getters instead of directly accessing MonoClass fields --- diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 86eadc6..c53f4e5 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -1864,6 +1864,85 @@ mono_class_is_gparam_with_nonblittable_parent (MonoClass *klass) return parent_class != mono_defaults.object_class; } +/** + * Checks if there are any overlapping object and non-object fields. + * The alignment of object reference fields is checked elswhere and this function assumes + * that all references are aligned correctly. + * + * \param layout_check A buffer to check which bytes hold object references or values + * \param klass Checked struct + * \param field_offsets Offsets of the klass' fields relative to the start of layout_check + * \param field_count Count of klass fields + * \param invalid_field_offset When the layout is invalid it will be set to the offset of the field which is invalid + * + * \return True if the layout of the struct is valid, otherwise false. + */ +static gboolean +validate_struct_fields_overlaps (guint8 *layout_check, int layout_size, MonoClass *klass, const int *field_offsets, const int field_count, int *invalid_field_offset) +{ + MonoClassField *field; + MonoType *ftype; + int field_offset; + + for (int i = 0; i < field_count && !mono_class_has_failure (klass); i++) { + // using mono_class_get_fields_internal isn't appropriate here because it will + // try to call mono_class_setup_fields which is what we're doing already + field = &m_class_get_fields (klass) [i]; + field_offset = field_offsets [i]; + + if (!field) + continue; + if (mono_field_is_deleted (field)) + continue; + if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) + continue; + + ftype = mono_type_get_underlying_type (field->type); + ftype = mono_type_get_basic_type_from_generic (ftype); + + if (mono_type_is_struct (ftype)) { + // recursively check the layout of the embedded struct + MonoClass *embedded_class = mono_class_from_mono_type_internal (ftype); + g_assert (m_class_is_fields_inited (embedded_class)); + + const int embedded_fields_count = mono_class_get_field_count (embedded_class); + int *embedded_offsets = g_new0 (int, embedded_fields_count); + for (int j = 0; j < embedded_fields_count; ++j) { + embedded_offsets [j] = field_offset + m_class_get_fields (embedded_class) [j].offset - MONO_ABI_SIZEOF (MonoObject); + } + + gboolean is_valid = validate_struct_fields_overlaps (layout_check, layout_size, embedded_class, embedded_offsets, embedded_fields_count, invalid_field_offset); + g_free (embedded_offsets); + + if (!is_valid) { + // overwrite whatever was in the invalid_field_offset with the offset of the currently checked field + // we want to return the outer most invalid field + *invalid_field_offset = field_offset; + return FALSE; + } + } else { + int align = 0; + int size = mono_type_size (field->type, &align); + guint8 type = type_has_references (klass, ftype) ? 1 : 2; + + // 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. + for (int j = 0; j < size; j++) { + int checked_byte = field_offset + j; + g_assert(checked_byte < layout_size); + + if (layout_check [checked_byte] != 0 && layout_check [checked_byte] != type) { + *invalid_field_offset = field_offset; + return FALSE; + } + layout_check [checked_byte] = type; + } + } + } + + return TRUE; +} + /* * mono_class_layout_fields: * @class: a class @@ -2154,29 +2233,12 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_ } /* check for incorrectly aligned or overlapped by a non-object field */ - guint8 *layout_check; + guint8 *layout_check; if (has_references) { layout_check = g_new0 (guint8, real_size); - for (i = 0; i < top && !mono_class_has_failure (klass); i++) { - field = &klass->fields [i]; - if (!field) - continue; - if (mono_field_is_deleted (field)) - continue; - if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) - continue; - int align = 0; - int size = mono_type_size (field->type, &align); - MonoType *ftype = mono_type_get_underlying_type (field->type); - ftype = mono_type_get_basic_type_from_generic (ftype); - guint8 type = type_has_references (klass, ftype) ? 1 : 2; - for (int j = 0; j < size; j++) { - if (layout_check [field_offsets [i] + j] != 0 && layout_check [field_offsets [i] + j] != type) { - mono_class_set_type_load_failure (klass, "Could not load type '%s' because it contains an object field at offset %d that is incorrectly aligned or overlapped by a non-object field.", klass->name, field->offset); - break; - } - layout_check [field_offsets [i] + j] = type; - } + int invalid_field_offset; + if (!validate_struct_fields_overlaps (layout_check, real_size, klass, field_offsets, top, &invalid_field_offset)) { + mono_class_set_type_load_failure (klass, "Could not load type '%s' because it contains an object field at offset %d that is incorrectly aligned or overlapped by a non-object field.", klass->name, invalid_field_offset); } g_free (layout_check); } diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.cs b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.cs new file mode 100644 index 0000000..eb86c19 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.cs @@ -0,0 +1,74 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +[StructLayout(LayoutKind.Explicit, Size = 24)] +public struct ComplexStruct +{ + [FieldOffset(0)] + public object? Object; + + [FieldOffset(0)] + public InnerStruct Inner; + + [FieldOffset(8)] + public double Double; + + [FieldOffset(8)] + public ulong High; + + [FieldOffset(16)] + public ulong Low; +} + +[StructLayout(LayoutKind.Explicit, Size = 16)] +public struct InnerStruct +{ + [FieldOffset(0)] + public object? Object; + + [FieldOffset(8)] + public int High; + + [FieldOffset(12)] + public int Low; +} + +public class Test_NestedStructsWithExplicitLayout_Case01 { + private ComplexStruct currentCount = default; + + private void IncrementCount() + { + var x = new ComplexStruct(); + x.Inner.High = currentCount.Inner.High + 1; + currentCount = x; + } + + public static int Main () + { + try + { + var instance = new Test_NestedStructsWithExplicitLayout_Case01(); + instance.IncrementCount(); + var result = 99 + instance.currentCount.Inner.High; + + if (result == 100) + { + Console.WriteLine("PASS: union of Explict + Explicit works correctly"); + } + + return result; + } + catch (TypeLoadException e) + { + Console.WriteLine("FAIL: type was not loaded"); + return 101; + } + catch (Exception e) + { + return 102; + } + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.csproj b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.csproj new file mode 100644 index 0000000..1c3478d --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.csproj @@ -0,0 +1,9 @@ + + + Exe + enable + + + + + diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.cs b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.cs new file mode 100644 index 0000000..0f9926d --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.cs @@ -0,0 +1,89 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +[StructLayout(LayoutKind.Explicit, Size = 16)] +public struct FirstLevel +{ + [FieldOffset(0)] + public object? Object; + + [FieldOffset(0)] + public SecondLevel SecondLevel; + + [FieldOffset(8)] + public int High; + + [FieldOffset(12)] + public int Low; +} + +[StructLayout(LayoutKind.Explicit, Size = 16)] +public struct SecondLevel +{ + [FieldOffset(0)] + public object? Object; + + [FieldOffset(0)] + public ThirdLevel ThirdLevel; + + [FieldOffset(8)] + public int High; + + [FieldOffset(12)] + public int Low; +} + +[StructLayout(LayoutKind.Explicit, Size = 16)] +public struct ThirdLevel +{ + [FieldOffset(0)] + public object? Object; + + [FieldOffset(8)] + public int High; + + [FieldOffset(12)] + public int Low; +} + +public class Test_NestedStructsWithExplicitLayout_Case02 { + private int Run(int value) + { + var x = new FirstLevel(); + x.Low = value; + return x.SecondLevel.ThirdLevel.Low; + } + + public static int Main () + { + try + { + var expectedResult = 13; + + var testInstance = new Test_NestedStructsWithExplicitLayout_Case02(); + var result = testInstance.Run(expectedResult); + + if (result == expectedResult) + { + Console.WriteLine("PASS: types were loaded correctly"); + return 100; + } + + Console.WriteLine("FAIL: invalid value"); + return 103; + } + catch (TypeLoadException e) + { + Console.WriteLine("FAIL: type was not loaded correctly"); + return 101; + } + catch (Exception e) + { + Console.WriteLine("FAIL: unknown error"); + return 102; + } + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.csproj b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.csproj new file mode 100644 index 0000000..7ab33ec --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.csproj @@ -0,0 +1,9 @@ + + + Exe + enable + + + + + diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.cs b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.cs new file mode 100644 index 0000000..57fd85b --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +[StructLayout(LayoutKind.Explicit)] +public struct FirstLevel +{ + [FieldOffset(0)] + public object? ConflictingObjectField; + + [FieldOffset(0)] + public SecondLevel SecondLevel; +} + +[StructLayout(LayoutKind.Explicit)] +public struct SecondLevel +{ + [FieldOffset(0)] + public ThirdLevel ThirdLevel; + + [FieldOffset(8)] + public long Value; +} + +[StructLayout(LayoutKind.Explicit)] +public struct ThirdLevel +{ + [FieldOffset(0)] + public long ConflictingValueTypeField; +} + +public class Test_NestedStructsWithExplicitLayout_Case06 { + private void Run() + { + var x = new FirstLevel(); + x.ConflictingObjectField = new object(); + } + + public static int Main () + { + try + { + var test = new Test_NestedStructsWithExplicitLayout_Case06(); + test.Run(); + } + catch (TypeLoadException e) + { + Console.WriteLine("PASS: object and non-object field overlap was detected"); + return 100; + } + catch (Exception e) + { + Console.WriteLine("FAIL: unexpected exception type"); + return 102; + } + + Console.WriteLine("FAIL: object and non-object field overlap was not detected"); + return 101; + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.csproj b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.csproj new file mode 100644 index 0000000..7308df2 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.csproj @@ -0,0 +1,9 @@ + + + Exe + enable + + + + + diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case04.cs b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case04.cs new file mode 100644 index 0000000..7e2cfc4 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case04.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +[StructLayout(LayoutKind.Explicit)] +public struct FirstLevel +{ + [FieldOffset(0)] + public object? ConflictingObjectField; + + [FieldOffset(0)] + public SecondLevel SecondLevel; +} + +[StructLayout(LayoutKind.Explicit)] +public struct SecondLevel +{ + [FieldOffset(0)] + public ThirdLevel ThirdLevel; + + [FieldOffset(8)] + public long Value; +} + +[StructLayout(LayoutKind.Explicit)] +public struct ThirdLevel +{ + [FieldOffset(6)] + public short ConflictingValueTypeField; +} + +public class Test_NestedStructsWithExplicitLayout_Case07 { + private void Run() + { + var x = new FirstLevel(); + x.ConflictingObjectField = new object(); + } + + public static int Main () + { + try + { + var test = new Test_NestedStructsWithExplicitLayout_Case07(); + test.Run(); + } + catch (TypeLoadException e) + { + Console.WriteLine("PASS: object and non-object field overlap was detected"); + return 100; + } + catch (Exception e) + { + Console.WriteLine("FAIL: unexpected exception type"); + return 102; + } + + Console.WriteLine("FAIL: object and non-object field overlap was not detected"); + return 101; + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case04.csproj b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case04.csproj new file mode 100644 index 0000000..6b6ec60 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case04.csproj @@ -0,0 +1,9 @@ + + + Exe + enable + + + + + diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case05.cs b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case05.cs new file mode 100644 index 0000000..9a4d8de --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case05.cs @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +[StructLayout(LayoutKind.Explicit)] +public struct FirstLevel +{ + [FieldOffset(0)] + public object? ConflictingObjectField; + + [FieldOffset(0)] + public SecondLevel SecondLevel; +} + +[StructLayout(LayoutKind.Explicit)] +public struct SecondLevel +{ + [FieldOffset(0)] + public long ConflictingValueTypeField; +} + +public class Test_NestedStructsWithExplicitLayout_Case05 { + private void Run() + { + var x = new FirstLevel(); + x.ConflictingObjectField = new object(); + } + + public static int Main () + { + try + { + var test = new Test_NestedStructsWithExplicitLayout_Case05(); + test.Run(); + } + catch (TypeLoadException e) + { + Console.WriteLine("PASS: object and non-object field overlap was detected"); + return 100; + } + catch (Exception e) + { + Console.WriteLine("FAIL: unexpected exception type"); + return 102; + } + + Console.WriteLine("FAIL: object and non-object field overlap was not detected"); + return 101; + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/NestedStructs/case05.csproj b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case05.csproj new file mode 100644 index 0000000..e843149 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/NestedStructs/case05.csproj @@ -0,0 +1,9 @@ + + + Exe + enable + + + + + diff --git a/src/tests/Loader/classloader/explicitlayout/misc/array_segment.cs b/src/tests/Loader/classloader/explicitlayout/misc/array_segment.cs new file mode 100644 index 0000000..16efed5 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/misc/array_segment.cs @@ -0,0 +1,57 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +// Structs come from from System.Net.Sockets tests +internal struct FakeArraySegment +{ + public byte[] Array; + public int Offset; + public int Count; + + public ArraySegment ToActual() + { + ArraySegmentWrapper wrapper = default(ArraySegmentWrapper); + wrapper.Fake = this; + return wrapper.Actual; + } +} + +[StructLayout(LayoutKind.Explicit)] +internal struct ArraySegmentWrapper +{ + [FieldOffset(0)] public ArraySegment Actual; + [FieldOffset(0)] public FakeArraySegment Fake; +} + +public class Test_ExplicitLayoutWithArraySegment +{ + private void Run() + { + var fakeArraySegment = new FakeArraySegment() { Array = new byte[10], Offset = 0, Count = 10 }; + ArraySegment internalBuffer = fakeArraySegment.ToActual(); + } + + public static int Main() + { + try + { + var testInstance = new Test_ExplicitLayoutWithArraySegment(); + testInstance.Run(); + } + catch (TypeLoadException e) + { + Console.WriteLine("FAIL: Caught TypeLoadException: " + e.Message); + return 101; + } + catch (Exception e) + { + Console.WriteLine("FAIL: Caught unexpected exception: " + e.Message); + return 101; + } + + return 100; + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/misc/array_segment.csproj b/src/tests/Loader/classloader/explicitlayout/misc/array_segment.csproj new file mode 100644 index 0000000..451b532 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/misc/array_segment.csproj @@ -0,0 +1,8 @@ + + + Exe + + + + +