From 8b34fb0656c2514b542321107f34c0bae659ff32 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 29 Aug 2024 10:07:54 -0700 Subject: [PATCH] Remove ArrayClass::m_ElementType - get type from the type handle on MethodTable (#106787) `ArrayClass::m_ElementType` held a cached version of `MethodTable::m_ElementTypeHnd.GetSignatureCorElementType()`. This removes that member and makes `MethodTable::GetArrayElementType` (and therefore `ArrayBase::GetArrayElementType`) use the type handle to get the element type instead. This also fixes an issue where we were not always emitting a type check in array functions on arrays with elements that were object references. The added tests validate that these cases now result in an `ArrayTypeMismatchException` for mismatched element types. --- src/coreclr/vm/array.cpp | 6 +-- src/coreclr/vm/class.h | 11 ---- src/coreclr/vm/gchelpers.cpp | 8 +-- src/coreclr/vm/methodtable.h | 6 ++- src/coreclr/vm/methodtable.inl | 9 ---- .../baseservices/invalid_operations/Arrays.cs | 51 +++++++++++++++++++ .../InvalidOperations.csproj | 1 + 7 files changed, 63 insertions(+), 29 deletions(-) create mode 100644 src/tests/baseservices/invalid_operations/Arrays.cs diff --git a/src/coreclr/vm/array.cpp b/src/coreclr/vm/array.cpp index b034357ef3c13..9c2f495a844c0 100644 --- a/src/coreclr/vm/array.cpp +++ b/src/coreclr/vm/array.cpp @@ -337,7 +337,6 @@ MethodTable* Module::CreateArrayMethodTable(TypeHandle elemTypeHnd, CorElementTy pClass->SetInternalCorElementType(arrayKind); pClass->SetAttrClass (tdPublic | tdSerializable | tdSealed); // This class is public, serializable, sealed pClass->SetRank (Rank); - pClass->SetArrayElementType (elemType); pClass->SetMethodTable (pMT); // Fill In the method table @@ -658,9 +657,8 @@ class ArrayOpLinker : public ILStubLinker hiddenArgIdx = 0; } #endif - - ArrayClass *pcls = (ArrayClass*)(pMT->GetClass()); - if(pcls->GetArrayElementType() == ELEMENT_TYPE_CLASS) + CorElementType sigElementType = pMT->GetArrayElementType(); + if (CorTypeInfo::IsObjRef(sigElementType)) { // Type Check if(m_pMD->GetArrayFuncIndex() == ArrayMethodDesc::ARRAY_FUNC_SET) diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 3d936b7bf69af..743d34ca3a88a 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1953,7 +1953,6 @@ class ArrayClass : public EEClass DAC_ALIGNAS(EEClass) // Align the first member to the alignment of the base class unsigned char m_rank; - CorElementType m_ElementType;// Cache of element type in m_ElementTypeHnd public: DWORD GetRank() { @@ -1969,16 +1968,6 @@ class ArrayClass : public EEClass m_rank = (unsigned char)Rank; } - CorElementType GetArrayElementType() { - LIMITED_METHOD_CONTRACT; - return m_ElementType; - } - void SetArrayElementType(CorElementType ElementType) { - LIMITED_METHOD_CONTRACT; - m_ElementType = ElementType; - } - - // Allocate a new MethodDesc for the methods we add to this class void InitArrayMethodDesc( ArrayMethodDesc* pNewMD, diff --git a/src/coreclr/vm/gchelpers.cpp b/src/coreclr/vm/gchelpers.cpp index e9b0245867a87..d69796f010410 100644 --- a/src/coreclr/vm/gchelpers.cpp +++ b/src/coreclr/vm/gchelpers.cpp @@ -402,7 +402,7 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS #endif #ifdef FEATURE_DOUBLE_ALIGNMENT_HINT - if ((pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8) && + if ((pArrayMT->GetArrayElementTypeHandle() == CoreLibBinder::GetElementType(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); @@ -425,7 +425,7 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS else { #ifdef FEATURE_DOUBLE_ALIGNMENT_HINT - if (pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8) + if (pArrayMT->GetArrayElementTypeHandle() == CoreLibBinder::GetElementType(ELEMENT_TYPE_R8)) { flags |= GC_ALLOC_ALIGN8; } @@ -497,7 +497,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)) && (pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8)) + if ((DATA_ALIGNMENT < sizeof(double)) && (pArrayMT->GetArrayElementTypeHandle() == CoreLibBinder::GetElementType(ELEMENT_TYPE_R8))) { return NULL; } @@ -665,7 +665,7 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs, #endif #ifdef FEATURE_DOUBLE_ALIGNMENT_HINT - if ((pArrayMT->GetArrayElementType() == ELEMENT_TYPE_R8) && + if ((pArrayMT->GetArrayElementTypeHandle() == CoreLibBinder::GetElementType(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/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index d4823538b11aa..e7e44470da3f9 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -2789,7 +2789,11 @@ class MethodTable } // The following methods are only valid for the method tables for array types. - CorElementType GetArrayElementType(); + CorElementType GetArrayElementType() + { + return GetArrayElementTypeHandle().GetSignatureCorElementType(); + } + DWORD GetRank(); TypeHandle GetArrayElementTypeHandle() diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl index 9ad333b065573..819bc0eb8d434 100644 --- a/src/coreclr/vm/methodtable.inl +++ b/src/coreclr/vm/methodtable.inl @@ -299,15 +299,6 @@ inline BOOL MethodTable::IsValueType() return GetFlag(enum_flag_Category_ValueType_Mask) == enum_flag_Category_ValueType; } -//========================================================================================== -inline CorElementType MethodTable::GetArrayElementType() -{ - WRAPPER_NO_CONTRACT; - - _ASSERTE (IsArray()); - return dac_cast(GetClass())->GetArrayElementType(); -} - //========================================================================================== inline DWORD MethodTable::GetRank() { diff --git a/src/tests/baseservices/invalid_operations/Arrays.cs b/src/tests/baseservices/invalid_operations/Arrays.cs new file mode 100644 index 0000000000000..d65c73648d126 --- /dev/null +++ b/src/tests/baseservices/invalid_operations/Arrays.cs @@ -0,0 +1,51 @@ +// 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.Reflection; + +using Xunit; + +[ActiveIssue("https://github.com/dotnet/runtime/issues/107110", typeof(TestLibrary.PlatformDetection), nameof(TestLibrary.PlatformDetection.IsMonoInterpreter))] +public class Arrays +{ + private class TestClass { } + + [Fact] + public static void TypeMismatch_ArrayElement() + { + Console.WriteLine($"Running {nameof(TypeMismatch_ArrayElement)}..."); + TestClass[][] arr = new TestClass[1][]; + MethodInfo arraySet = typeof(object[][]).GetMethod("Set"); + Exception e = Assert.Throws(() => arraySet.Invoke(arr, [0, new object[] { new object() }])); + Assert.IsType(e.InnerException); + } + + [Fact] + public static void TypeMismatch_MultidimensionalArrayElement() + { + Console.WriteLine($"Running {nameof(TypeMismatch_MultidimensionalArrayElement)}..."); + TestClass[][,] arr = new TestClass[1][,]; + MethodInfo arraySet = typeof(object[][,]).GetMethod("Set"); + Exception e = Assert.Throws(() => arraySet.Invoke(arr, [0, new object[1,1] { {new object()} }])); + Assert.IsType(e.InnerException); + } + + [Fact] + public static void TypeMismatch_ClassElement() + { + Console.WriteLine($"Running {nameof(TypeMismatch_ClassElement)}..."); + { + TestClass[] arr = new TestClass[1]; + MethodInfo arraySet = typeof(object[]).GetMethod("Set"); + Exception e = Assert.Throws(() => arraySet.Invoke(arr, [0, new object()])); + Assert.IsType(e.InnerException); + } + { + TestClass[,] arr = new TestClass[1,1]; + MethodInfo arraySet = typeof(object[,]).GetMethod("Set"); + Exception e = Assert.Throws(() => arraySet.Invoke(arr, [0, 0, new object()])); + Assert.IsType(e.InnerException); + } + } +} \ No newline at end of file diff --git a/src/tests/baseservices/invalid_operations/InvalidOperations.csproj b/src/tests/baseservices/invalid_operations/InvalidOperations.csproj index 1c5f1bf800139..7c47b196be02d 100644 --- a/src/tests/baseservices/invalid_operations/InvalidOperations.csproj +++ b/src/tests/baseservices/invalid_operations/InvalidOperations.csproj @@ -8,6 +8,7 @@ +