Disallow arrays of System.Void (#94835)

The CoreCLR type loader allowed creating arrays of System.Void. Many operations with these invalid array types failed, often in inscrutable ways. For example, GetInterfaces() call failed with type load exception of IEnumerable type. The exact failure modes are different between runtimes. It is better to disallow creating these invalid array types in the first place, across all runtimes, to make the behavior robust and consistent. Related to #88620 Fixes #94086
dotnet · Nov 17, 2023 · 8b04e1a · 8b04e1a
1 parent 21cb3f1
commit 8b04e1a
Show file tree

Hide file tree

Showing 14 changed files with 30 additions and 89 deletions.
diff --git a/src/coreclr/dlls/mscorrc/mscorrc.rc b/src/coreclr/dlls/mscorrc/mscorrc.rc
@@ -325,6 +325,7 @@ BEGIN
     IDS_CLASSLOAD_BADFORMAT                 "Could not load type '%1' from assembly '%2' because the format is invalid."
     IDS_CLASSLOAD_BYREFARRAY                "Could not create array type '%1' from assembly '%2' because the element type is ByRef."
     IDS_CLASSLOAD_BYREFLIKEARRAY            "Could not create array type '%1' from assembly '%2' because the element type is ByRef-like."
+    IDS_CLASSLOAD_VOIDARRAY                 "Could not create array type '%1' from assembly '%2' because the element type is System.Void."
     IDS_CLASSLOAD_FIELDTOOLARGE             "Size of field of type '%1' from assembly '%2' is too large."
     IDS_CLASSLOAD_CANTEXTEND                "Type '%1' from assembly '%2' cannot extend from any other type."
     IDS_CLASSLOAD_STATICVIRTUAL             "Method '%3' in type '%1' from assembly '%2' cannot be a static and a virtual."

diff --git a/src/coreclr/dlls/mscorrc/resource.h b/src/coreclr/dlls/mscorrc/resource.h
@@ -123,6 +123,7 @@
 #define IDS_CLASSLOAD_BADFORMAT                 0x1774
 #define IDS_CLASSLOAD_BYREFARRAY                0x1775
 #define IDS_CLASSLOAD_BYREFLIKEARRAY            0x1776
+#define IDS_CLASSLOAD_VOIDARRAY                 0x1777
 #define IDS_CLASSLOAD_STATICVIRTUAL             0x1778
 #define IDS_CLASSLOAD_REDUCEACCESS              0x1779
 #define IDS_CLASSLOAD_BADPINVOKE                0x177a

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
@@ -423,7 +423,6 @@
     <Compile Include="System\Reflection\Runtime\General\MetadataReaderExtensions.NativeFormat.cs" />
     <Compile Include="System\Reflection\Runtime\General\NamespaceChain.cs" />
     <Compile Include="System\Reflection\Runtime\General\NativeFormat\DefaultValueParser.cs" />
-    <Compile Include="System\Reflection\Runtime\General\RuntimeTypeHandleKey.cs" />
     <Compile Include="System\Reflection\Runtime\General\ThunkedApis.cs" />
     <Compile Include="System\Reflection\Runtime\General\TypeContext.cs" />
     <Compile Include="System\Reflection\Runtime\General\TypeForwardInfo.cs" />

diff --git a/...eaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/RuntimeTypeHandleKey.cs b/...eaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/RuntimeTypeHandleKey.cs
diff --git a/...clr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/TypeUnifier.cs b/...clr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/General/TypeUnifier.cs
@@ -222,7 +222,7 @@ private static void ValidateElementType(RuntimeTypeInfo elementType, bool multiD
         {
             Debug.Assert(multiDim || rank == 1);
 
-            if (elementType.IsByRef)
+            if (elementType.IsByRef || elementType.IsVoid)
                 throw new TypeLoadException(SR.Format(SR.ArgumentException_InvalidArrayElementType, elementType));
         }
     }

diff --git a/...tiveaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs b/...tiveaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
@@ -57,6 +57,8 @@ protected RuntimeTypeInfo()
 
         public bool IsGenericType => IsGenericTypeDefinition || IsConstructedGenericType;
 
+        public bool IsVoid => InternalTypeHandleIfAvailable.Equals(typeof(void).TypeHandle);
+
         public abstract string Name { get; }
 
         public abstract Assembly Assembly { get; }

diff --git a/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs b/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs
@@ -283,8 +283,11 @@ private static TypeDesc EnsureLoadableTypeUncached(TypeDesc type)
                         ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type);
                     }
 
-                    // It might seem reasonable to disallow array of void, but the CLR doesn't prevent that too hard.
-                    // E.g. "newarr void" will fail, but "newarr void[]" or "ldtoken void[]" will succeed.
+                    if (parameterType.IsVoid)
+                    {
+                        // Arrays of System.Void are not allowed
+                        ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type);
+                    }
                 }
             }
             else if (type.IsFunctionPointer)

diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp
@@ -2984,19 +2984,20 @@ TypeHandle ClassLoader::CreateTypeHandleForTypeKey(TypeKey* pKey, AllocMemTracke
             THROW_BAD_FORMAT_MAYBE((kind != ELEMENT_TYPE_SZARRAY) || rank == 1, BFA_SDARRAY_BADRANK, pLoaderModule);
 
             // Arrays of BYREFS not allowed
-            if (paramType.GetInternalCorElementType() == ELEMENT_TYPE_BYREF)
+            if (paramType.IsByRef())
             {
                 ThrowTypeLoadException(pKey, IDS_CLASSLOAD_BYREFARRAY);
             }
 
             // Arrays of ByRefLike types not allowed
-            MethodTable* pMT = paramType.GetMethodTable();
-            if (pMT != NULL)
+            if (paramType.IsByRefLike())
             {
-                if (pMT->IsByRefLike())
-                {
-                    ThrowTypeLoadException(pKey, IDS_CLASSLOAD_BYREFLIKEARRAY);
-                }
+                ThrowTypeLoadException(pKey, IDS_CLASSLOAD_BYREFLIKEARRAY);
+            }
+
+            if (paramType.GetSignatureCorElementType() == ELEMENT_TYPE_VOID)
+            {
+                ThrowTypeLoadException(pKey, IDS_CLASSLOAD_VOIDARRAY);
             }
 
             if (rank > MAX_RANK)

diff --git a/src/coreclr/vm/gchelpers.cpp b/src/coreclr/vm/gchelpers.cpp
@@ -380,12 +380,6 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS
     _ASSERTE(pArrayMT->CheckInstanceActivated());
     _ASSERTE(pArrayMT->GetInternalCorElementType() == ELEMENT_TYPE_SZARRAY);
 
-    CorElementType elemType = pArrayMT->GetArrayElementType();
-
-    // Disallow the creation of void[] (an array of System.Void)
-    if (elemType == ELEMENT_TYPE_VOID)
-        COMPlusThrow(kArgumentException);
-
     if (cElements < 0)
         COMPlusThrow(kOverflowException);
 
@@ -408,7 +402,7 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS
 #endif
 
 #ifdef FEATURE_DOUBLE_ALIGNMENT_HINT
-    if ((elemType == ELEMENT_TYPE_R8) &&
+    if ((pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8) &&
         ((DWORD)cElements >= g_pConfig->GetDoubleArrayToLargeObjectHeapThreshold()))
     {
         STRESS_LOG2(LF_GC, LL_INFO10, "Allocating double MD array of size %d and length %d to large object heap\n", totalSize, cElements);
@@ -431,7 +425,7 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS
     else
     {
 #ifndef FEATURE_64BIT_ALIGNMENT
-        if ((DATA_ALIGNMENT < sizeof(double)) && (elemType == ELEMENT_TYPE_R8) &&
+        if ((DATA_ALIGNMENT < sizeof(double)) && (pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8) &&
             (totalSize < g_pConfig->GetGCLOHThreshold() - MIN_OBJECT_SIZE))
         {
             // Creation of an array of doubles, not in the large object heap.
@@ -508,18 +502,12 @@ OBJECTREF TryAllocateFrozenSzArray(MethodTable* pArrayMT, INT32 cElements)
 
     // The initial validation is copied from AllocateSzArray impl
 
-    CorElementType elemType = pArrayMT->GetArrayElementType();
-
     if (pArrayMT->ContainsPointers() && cElements > 0)
     {
         // For arrays with GC pointers we can only work with empty arrays
         return NULL;
     }
 
-    // Disallow the creation of void[] (an array of System.Void)
-    if (elemType == ELEMENT_TYPE_VOID)
-        COMPlusThrow(kArgumentException);
-
     if (cElements < 0)
         COMPlusThrow(kOverflowException);
 
@@ -542,7 +530,7 @@ OBJECTREF TryAllocateFrozenSzArray(MethodTable* pArrayMT, INT32 cElements)
 
     // FrozenObjectHeapManager doesn't yet support objects with a custom alignment,
     // so we give up on arrays of value types requiring 8 byte alignment on 32bit platforms.
-    if ((DATA_ALIGNMENT < sizeof(double)) && (elemType == ELEMENT_TYPE_R8))
+    if ((DATA_ALIGNMENT < sizeof(double)) && (pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8))
     {
         return NULL;
     }
@@ -637,11 +625,6 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs,
     CorElementType kind = pArrayMT->GetInternalCorElementType();
     _ASSERTE(kind == ELEMENT_TYPE_ARRAY || kind == ELEMENT_TYPE_SZARRAY);
 
-    CorElementType elemType = pArrayMT->GetArrayElementType();
-    // Disallow the creation of void[,] (a multi-dim  array of System.Void)
-    if (elemType == ELEMENT_TYPE_VOID)
-        COMPlusThrow(kArgumentException);
-
     // Calculate the total number of elements in the array
     UINT32 cElements;
     bool maxArrayDimensionLengthOverflow = false;
@@ -715,7 +698,7 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs,
 #endif
 
 #ifdef FEATURE_DOUBLE_ALIGNMENT_HINT
-    if ((elemType == ELEMENT_TYPE_R8) &&
+    if ((pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8) &&
         (cElements >= g_pConfig->GetDoubleArrayToLargeObjectHeapThreshold()))
     {
         STRESS_LOG2(LF_GC, LL_INFO10, "Allocating double MD array of size %d and length %d to large object heap\n", totalSize, cElements);

diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ActivatorTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ActivatorTests.cs
@@ -154,7 +154,6 @@ public void CreateInstance_ContainsGenericParameters_ThrowsArgumentException(Typ
         public static IEnumerable<object[]> CreateInstance_InvalidType_TestData()
         {
             yield return new object[] { typeof(void) };
-            yield return new object[] { typeof(void).MakeArrayType() };
             yield return new object[] { typeof(ArgIterator) };
         }
 

diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs
@@ -47,7 +47,6 @@ static TypeTests()
             NonArrayBaseTypes = new List<Type>()
             {
                 typeof(int),
-                typeof(void),
                 typeof(int*),
                 typeof(Outside),
                 typeof(Outside<int>),
@@ -244,15 +243,13 @@ public void MakeArrayType_Invoke_ReturnsExpected(Type t, Type tArrayExpected)
         public static IEnumerable<object[]> MakeArray_UnusualTypes_TestData()
         {
             yield return new object[] { typeof(StaticClass) };
-            yield return new object[] { typeof(void) };
             yield return new object[] { typeof(GenericClass<>) };
             yield return new object[] { typeof(GenericClass<>).MakeGenericType(typeof(GenericClass<>)) };
             yield return new object[] { typeof(GenericClass<>).GetTypeInfo().GetGenericArguments()[0] };
         }
 
         [Theory]
         [MemberData(nameof(MakeArray_UnusualTypes_TestData))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/52072", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
         public void MakeArrayType_UnusualTypes_ReturnsExpected(Type t)
         {
             Type tArray = t.MakeArrayType();
@@ -298,18 +295,19 @@ public void MakeArrayType_InvokeRankTwo_ReturnsExpected(Type t, Type tArrayExpec
             Assert.Equal(t.ToString() + "[,]", tArray.ToString());
         }
 
-        public static IEnumerable<object[]> MakeArrayType_ByRef_TestData()
+        public static IEnumerable<object[]> MakeArrayType_InvalidElementType_TestData()
         {
             yield return new object[] { typeof(int).MakeByRefType() };
             yield return new object[] { typeof(TypedReference) };
             yield return new object[] { typeof(ArgIterator) };
             yield return new object[] { typeof(RuntimeArgumentHandle) };
             yield return new object[] { typeof(Span<int>) };
+            yield return new object[] { typeof(void) };
         }
 
         [Theory]
-        [MemberData(nameof(MakeArrayType_ByRef_TestData))]
-        public void MakeArrayType_ByRef_ThrowsTypeLoadException(Type t)
+        [MemberData(nameof(MakeArrayType_InvalidElementType_TestData))]
+        public void MakeArrayType_InvalidElementType_ThrowsTypeLoadException(Type t)
         {
             Assert.Throws<TypeLoadException>(() => t.MakeArrayType());
         }
@@ -607,7 +605,6 @@ public void IsSZArray_FalseForNonArrayTypes()
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/52072", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
         public void IsSZArray_TrueForSZArrayTypes()
         {
             foreach (Type type in NonArrayBaseTypes.Select(nonArrayBaseType => nonArrayBaseType.MakeArrayType()))
@@ -659,7 +656,6 @@ public void IsVariableBoundArray_FalseForNonArrayTypes()
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/52072", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
         public void IsVariableBoundArray_FalseForSZArrayTypes()
         {
             foreach (Type type in NonArrayBaseTypes.Select(nonArrayBaseType => nonArrayBaseType.MakeArrayType()))

diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c
@@ -1170,12 +1170,8 @@ mono_class_create_bounded_array (MonoClass *eclass, guint32 rank, gboolean bound
 	klass->rank = GUINT32_TO_UINT8 (rank);
 	klass->element_class = eclass;
 
-	if (m_class_get_byval_arg (eclass)->type == MONO_TYPE_TYPEDBYREF) {
-		/*Arrays of those two types are invalid.*/
-		ERROR_DECL (prepared_error);
-		mono_error_set_invalid_program (prepared_error, "Arrays of System.TypedReference types are invalid.");
-		mono_class_set_failure (klass, mono_error_box (prepared_error, klass->image));
-		mono_error_cleanup (prepared_error);
+	if (MONO_TYPE_IS_VOID (m_class_get_byval_arg (eclass))) {
+		mono_class_set_type_load_failure (klass, "Arrays of System.Void types are invalid.");
 	} else if (m_class_is_byreflike (eclass)) {
 		/* .NET Core throws a type load exception: "Could not create array type 'fullname[]'" */
 		char *full_name = mono_type_get_full_name (eclass);

diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c
@@ -5966,9 +5966,9 @@ check_for_invalid_array_type (MonoType *type, MonoError *error)
 	gboolean allowed = TRUE;
 	char *name;
 
-	if (m_type_is_byref (type))
+	if (MONO_TYPE_IS_VOID (type))
 		allowed = FALSE;
-	else if (type->type == MONO_TYPE_TYPEDBYREF)
+	else if (m_type_is_byref (type))
 		allowed = FALSE;
 
 	MonoClass *klass = mono_class_from_mono_type_internal (type);

diff --git a/src/tests/JIT/Regression/CLR-x86-EJIT/v1-m10/b02353/b02353.cs b/src/tests/JIT/Regression/CLR-x86-EJIT/v1-m10/b02353/b02353.cs
@@ -42,7 +42,6 @@ public class Bug
           Type.GetType("System.Object"),
           Type.GetType("Simple"),
           Type.GetType("System.Empty[]"),
-          Type.GetType("System.Void[]"),
           Type.GetType("System.Boolean[]"),
           Type.GetType("System.Char[]"),
           Type.GetType("System.SByte[]"),
@@ -66,7 +65,6 @@ public class Bug
           Type.GetType("System.Object[]"),
           Type.GetType("Simple[]"),
           Type.GetType("System.Empty[][]"),
-          Type.GetType("System.Void[][]"),
           Type.GetType("System.Boolean[][]"),
           Type.GetType("System.Char[][]"),
           Type.GetType("System.SByte[][]"),
@@ -90,7 +88,6 @@ public class Bug
           Type.GetType("System.Object[][]"),
           Type.GetType("Simple[][]"),
           Type.GetType("System.Empty[][][]"),
-          Type.GetType("System.Void[][][]"),
           Type.GetType("System.Boolean[][][]"),
           Type.GetType("System.Char[][][]"),
           Type.GetType("System.SByte[][][]"),