Error out when struct size is bigger than int.MaxValue (#104393)

* Error out when field size is too big * Error out during type loading and add a test * Move test to a different file * Make things loadable in the managed type system * Error out when size overflow the value that int could hold * Update the threshold to FIELD_OFFSET_LAST_REAL_OFFSET * Change the threashold to int.MaxValue * Fix mono struct size overflow issue * Add signed/unsigned type conversion * Use gint64 instead of long, due to windows * Update src/mono/mono/metadata/class-init.c Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com> * Fix test * Add CoreCLR test for Explicit struct * Address review feedback * Fixes Explicit struct size for Mono * Disable explicit struct tests on 32bit platforms, due to out of memory issue. * Disable CoreCLR test for NativeAOT and crossgen * Fix test * Add RequiresProcessIsolation for CoreCLR test --------- Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com> Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
dotnet · Aug 21, 2024 · 423faed · 423faed
1 parent d315414
commit 423faed
Show file tree

Hide file tree

Showing 11 changed files with 232 additions and 7 deletions.
diff --git a/src/coreclr/tools/Common/TypeSystem/Common/LayoutInt.cs b/src/coreclr/tools/Common/TypeSystem/Common/LayoutInt.cs
@@ -92,6 +92,20 @@ public string ToStringInvariant()
             return new LayoutInt(checked(left._value - right._value));
         }
 
+        public static LayoutInt AddThrowing(LayoutInt left, LayoutInt right, TypeDesc loadedType)
+        {
+            if (left.IsIndeterminate || right.IsIndeterminate)
+                return Indeterminate;
+
+            int result = left._value + right._value;
+
+            // Overflow if both arguments have the opposite sign of the result
+            if (((left._value ^ result) & (right._value ^ result)) < 0)
+                ThrowHelper.ThrowTypeLoadException(loadedType);
+
+            return new LayoutInt(result);
+        }
+
         public override bool Equals(object obj)
         {
             if (obj is LayoutInt)

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs
@@ -436,7 +436,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
 
                 cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target);
                 offsets[fieldOrdinal] = new FieldAndOffset(field, cumulativeInstanceFieldPos + offsetBias);
-                cumulativeInstanceFieldPos = checked(cumulativeInstanceFieldPos + fieldSizeAndAlignment.Size);
+                cumulativeInstanceFieldPos = LayoutInt.AddThrowing(cumulativeInstanceFieldPos, fieldSizeAndAlignment.Size, type);
 
                 fieldOrdinal++;
             }

diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ComparerIntrinsics.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections;
 using System.Collections.Generic;
 
 using Internal.TypeSystem;
@@ -306,11 +307,11 @@ public static bool CanCompareValueTypeBits(MetadataType type, MethodDesc objectE
 
         private struct OverlappingFieldTracker
         {
-            private bool[] _usedBytes;
+            private BitArray _usedBytes;
 
             public OverlappingFieldTracker(MetadataType type)
             {
-                _usedBytes = new bool[type.InstanceFieldSize.AsInt];
+                _usedBytes = new BitArray(type.InstanceFieldSize.AsInt);
             }
 
             public bool TrackField(FieldDesc field)

diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp
@@ -1807,6 +1807,12 @@ MethodTableBuilder::BuildMethodTableThrowing(
         }
     }
 
+    if (IsValueClass())
+    {
+        if ((int)bmtFP->NumInstanceFieldBytes != (INT64)bmtFP->NumInstanceFieldBytes)
+            BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE);
+    }
+
     if (CheckIfSIMDAndUpdateSize())
     {
         totalDeclaredFieldSize = bmtFP->NumInstanceFieldBytes;

diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c
@@ -310,6 +310,10 @@ mono_class_setup_fields (MonoClass *klass)
 
 	/* Get the real size */
 	explicit_size = mono_metadata_packing_from_typedef (klass->image, klass->type_token, &packing_size, &real_size);
+
+	if (real_size > GINT32_TO_UINT32(INT32_MAX - MONO_ABI_SIZEOF (MonoObject)))
+		mono_class_set_type_load_failure (klass, "Can't load type %s. The size is too big.", m_class_get_name (klass));
+
 	if (explicit_size)
 		instance_size += real_size;
 
@@ -2322,7 +2326,12 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_
 				}
 				/*TypeBuilders produce all sort of weird things*/
 				g_assert (image_is_dynamic (klass->image) || field_offsets [i] > 0);
-				real_size = field_offsets [i] + size;
+
+				gint64 raw_real_size = (gint64)field_offsets [i] + size;
+				real_size = (gint32)raw_real_size;
+
+				if (real_size != raw_real_size)
+					mono_class_set_type_load_failure (klass, "Can't load type %s. The size is too big.", m_class_get_name (klass));
 			}
 
 			instance_size = MAX (real_size, instance_size);
@@ -2381,7 +2390,13 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_
 			/*
 			 * Calc max size.
 			 */
-			real_size = MAX (real_size, size + field_offsets [i]);
+			gint64 raw_real_size = (gint64)field_offsets [i] + size;
+			gint32 real_size_cast = (gint32)raw_real_size;
+
+			if (real_size_cast != raw_real_size)
+				mono_class_set_type_load_failure (klass, "Can't load type %s. The size is too big.", m_class_get_name (klass));
+
+			real_size = MAX (real_size, real_size_cast);
 		}
 
 		/* check for incorrectly aligned or overlapped by a non-object field */
@@ -2529,8 +2544,8 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_
 		}
 	}
 
-	/*valuetypes can't be neither bigger than 1Mb or empty. */
-	if (klass->valuetype && (klass->instance_size <= 0 || klass->instance_size > (0x100000 + MONO_ABI_SIZEOF (MonoObject)))) {
+	/*valuetypes can not be size 0 or bigger than 2gb. */
+	if (klass->valuetype && (klass->instance_size <= 0 || klass->instance_size > INT32_MAX)) {
 		/* Special case compiler generated types */
 		/* Hard to check for [CompilerGenerated] here */
 		if (!strstr (klass->name, "StaticArrayInitTypeSize") && !strstr (klass->name, "$ArrayType"))

diff --git a/src/tests/Loader/Loader.csproj b/src/tests/Loader/Loader.csproj
@@ -13,6 +13,7 @@
     <MergedWrapperProjectReference Remove="classloader/TypeGeneratorTests/**/*.??proj" />
     <MergedWrapperProjectReference Remove="classloader/StaticVirtualMethods/GenericContext/Generator/**/*.??proj" />
     <MergedWrapperProjectReference Remove="classloader/StaticVirtualMethods/TypeHierarchy/Generator/**/*.??proj" />
+    <MergedWrapperProjectReference Remove="classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono.csproj" />
   </ItemGroup>
 
   <Import Project="$(TestSourceDir)MergedTestRunner.targets" />

diff --git a/src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_CoreCLR.cs b/src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_CoreCLR.cs
@@ -0,0 +1,70 @@
+// 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.CompilerServices;
+using System.Runtime.InteropServices;
+using Xunit;
+
+[SkipOnMono("This test suite tests CoreCLR and Crossgen2/NativeAOT-specific layout rules.")]
+public unsafe class LargeStructSize
+{
+    struct X
+    {
+        byte x;
+        BigArray a;
+    }
+
+    [StructLayout(LayoutKind.Explicit)] 
+    struct X_explicit
+    {
+        [FieldOffset(0)]
+        byte x;
+        [FieldOffset(1)]
+        BigArray a;
+    }
+
+    [StructLayout(LayoutKind.Explicit)] 
+    struct X_non_blittable
+    {
+        [FieldOffset(0)]
+        bool x;
+        [FieldOffset(1)]
+        BigArray a;
+    }
+
+    struct Y
+    {
+        BigArray a;
+        byte y;
+    }
+
+    [StructLayout(LayoutKind.Explicit)]
+    struct Y_explict
+    {
+        [FieldOffset(0)]
+        BigArray b;
+        [FieldOffset(int.MaxValue)]
+        byte y;
+    }
+
+    [StructLayout(LayoutKind.Sequential, Size = int.MaxValue)]
+    struct BigArray
+    {
+    }
+
+    [Fact]
+    public static void TestLargeStructSize()
+    {
+        Assert.Equal(int.MaxValue, sizeof(BigArray));
+        Assert.Throws<TypeLoadException>(() => sizeof(X));
+        Assert.Throws<TypeLoadException>(() => sizeof(Y));
+        if (Environment.Is64BitProcess)
+        {
+            // Explicit struct of big size triggers out of memory error instead of type load exception
+            Assert.Throws<TypeLoadException>(() => sizeof(X_explicit));
+            Assert.Throws<TypeLoadException>(() => sizeof(X_non_blittable));
+            Assert.Throws<TypeLoadException>(() => sizeof(Y_explict));
+        }
+    }
+}
diff --git a/...ests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_CoreCLR.csproj b/...ests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_CoreCLR.csproj
@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <NativeAotIncompatible>true</NativeAotIncompatible>
+    <CrossGenTest>false</CrossGenTest>
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
diff --git a/src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono.cs b/src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono.cs
@@ -0,0 +1,93 @@
+// 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.CompilerServices;
+using System.Runtime.InteropServices;
+using Xunit;
+
+public unsafe class LargeStructSize
+{
+    struct X_64
+    {
+        byte x;
+        BigArray_64_1 a;
+    }
+
+    [StructLayout(LayoutKind.Explicit)] 
+    struct X_explicit_64
+    {
+        [FieldOffset(0)]
+        bool x;
+        [FieldOffset(1)]
+        BigArray_64_1 a;
+    }
+
+    struct Y_64
+    {
+        BigArray_64_1 a;
+        byte y;
+    }
+
+    struct X_32
+    {
+        byte x;
+        BigArray_32_1 a;
+    }
+
+    [StructLayout(LayoutKind.Explicit)] 
+    struct X_explicit_32
+    {
+        [FieldOffset(0)]
+        bool x;
+        [FieldOffset(1)]
+        BigArray_32_1 a;
+    }
+
+    struct Y_32
+    {
+        BigArray_32_1 a;
+        byte y;
+    }
+
+    [StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 16)]
+    struct BigArray_64_1
+    {
+    }
+
+    [StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 16 - 1)]
+    struct BigArray_64_2
+    {
+    }
+
+    [StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 8)]
+    struct BigArray_32_1
+    {
+    }
+
+    [StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 8 - 1)]
+    struct BigArray_32_2
+    {
+    }
+
+    [Fact]
+    public static void TestLargeStructSize()
+    {
+        if (Environment.Is64BitProcess)
+        {
+            Assert.Equal(int.MaxValue - (IntPtr.Size * 2), sizeof(BigArray_64_1));
+            Assert.Throws<TypeLoadException>(() => sizeof(BigArray_64_2));
+            Assert.Throws<TypeLoadException>(() => sizeof(X_64));
+            Assert.Throws<TypeLoadException>(() => sizeof(X_explicit_64));
+            Assert.Throws<TypeLoadException>(() => sizeof(Y_64));
+        }
+        else
+        {
+            Assert.Equal(int.MaxValue - (IntPtr.Size * 2), sizeof(BigArray_32_1));
+            Assert.Throws<TypeLoadException>(() => sizeof(BigArray_32_2));
+            Assert.Throws<TypeLoadException>(() => sizeof(X_32));
+            Assert.Throws<TypeLoadException>(() => sizeof(X_explicit_32));
+            Assert.Throws<TypeLoadException>(() => sizeof(Y_32));
+        }
+    }
+}
diff --git a/src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono.csproj b/src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono.csproj
@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <NativeAotIncompatible>true</NativeAotIncompatible>
+    <CrossGenTest>false</CrossGenTest>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
diff --git a/src/tests/issues.targets b/src/tests/issues.targets
@@ -24,6 +24,9 @@
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/mono/runningmono/*">
             <Issue>This test is to verify we are running mono, and therefore only makes sense on mono.</Issue>
         </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono/*">
+            <Issue>This test is designed to run on mono only.</Issue>
+        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/threading/regressions/2164/foreground-shutdown/*">
             <Issue>https://github.com/dotnet/runtime/issues/83658</Issue>
         </ExcludeList>