[Mono] Fix support for nested structs with explicit layout

eng/pipelines/runtime.yml

@@ -1036,7 +1036,6 @@ jobs:
       runtimeVariant: minijit
       condition: >-
         eq(variables['isFullMatrix'], true)
-
 #
 # Mono CoreCLR runtime Test executions using live libraries in interpreter mode
 # Only when Mono is changed

src/mono/mono/metadata/class-init.c

@@ -1864,6 +1864,83 @@ 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++) {
+		field = &klass->fields [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 (embedded_class->fields_inited);
+
+			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 + embedded_class->fields [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 +2231,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);
 		}

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;
+        }
+    }
+}

src/tests/Loader/classloader/explicitlayout/NestedStructs/case01.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="case01.cs" />
+  </ItemGroup>
+</Project>

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;
+        }
+    }
+}

src/tests/Loader/classloader/explicitlayout/NestedStructs/case02.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="case02.cs" />
+  </ItemGroup>
+</Project>

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;
+    }
+}

src/tests/Loader/classloader/explicitlayout/NestedStructs/case03.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="case03.cs" />
+  </ItemGroup>
+</Project>