From 84be45b4f75d05111e5cb9d7d69134459a735e42 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 21 Aug 2024 15:23:21 -0700 Subject: [PATCH 1/6] Remove ArrayClass::m_ElementType - get type from the type handle on MethodTable --- src/coreclr/vm/array.cpp | 6 ++---- src/coreclr/vm/class.h | 11 ----------- src/coreclr/vm/methodtable.h | 6 +++++- src/coreclr/vm/methodtable.inl | 9 --------- 4 files changed, 7 insertions(+), 25 deletions(-) diff --git a/src/coreclr/vm/array.cpp b/src/coreclr/vm/array.cpp index b034357ef3c13..0acfb9dd69720 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(sigElementType == ELEMENT_TYPE_CLASS || sigElementType == ELEMENT_TYPE_ARRAY) { // 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/methodtable.h b/src/coreclr/vm/methodtable.h index 9e209f7c5e50d..6b266b6c110e2 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -2783,7 +2783,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() { From 90e67fddeed5c5aad6319302984830a3fc6edc94 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 23 Aug 2024 20:15:00 -0700 Subject: [PATCH 2/6] Update gchelpers to check element type handle --- src/coreclr/vm/gchelpers.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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); From eb11c160df26f3ad5332256a9150e41acedb949a Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 23 Aug 2024 23:58:33 -0700 Subject: [PATCH 3/6] Fix condition for adding type check for array elements --- src/coreclr/vm/array.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/array.cpp b/src/coreclr/vm/array.cpp index 0acfb9dd69720..9c2f495a844c0 100644 --- a/src/coreclr/vm/array.cpp +++ b/src/coreclr/vm/array.cpp @@ -658,7 +658,7 @@ class ArrayOpLinker : public ILStubLinker } #endif CorElementType sigElementType = pMT->GetArrayElementType(); - if(sigElementType == ELEMENT_TYPE_CLASS || sigElementType == ELEMENT_TYPE_ARRAY) + if (CorTypeInfo::IsObjRef(sigElementType)) { // Type Check if(m_pMD->GetArrayFuncIndex() == ArrayMethodDesc::ARRAY_FUNC_SET) From ded0a70dd4c815210c63ebd20255dfb8848f6690 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 26 Aug 2024 10:10:10 -0700 Subject: [PATCH 4/6] Add tests --- .../baseservices/invalid_operations/Arrays.cs | 50 +++++++++++++++++++ .../InvalidOperations.csproj | 1 + 2 files changed, 51 insertions(+) create mode 100644 src/tests/baseservices/invalid_operations/Arrays.cs diff --git a/src/tests/baseservices/invalid_operations/Arrays.cs b/src/tests/baseservices/invalid_operations/Arrays.cs new file mode 100644 index 0000000000000..d635312a7c2a0 --- /dev/null +++ b/src/tests/baseservices/invalid_operations/Arrays.cs @@ -0,0 +1,50 @@ +// 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; + +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 @@ + From ec544fdff85e81d1e20010b2801c85b04969a765 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 28 Aug 2024 14:03:01 -0700 Subject: [PATCH 5/6] Skip test on monointerpreter --- src/tests/baseservices/invalid_operations/Arrays.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/baseservices/invalid_operations/Arrays.cs b/src/tests/baseservices/invalid_operations/Arrays.cs index d635312a7c2a0..ed93164c30759 100644 --- a/src/tests/baseservices/invalid_operations/Arrays.cs +++ b/src/tests/baseservices/invalid_operations/Arrays.cs @@ -6,6 +6,7 @@ using Xunit; +[ConditionalClass(typeof(TestLibrary.PlatformDetection), nameof(TestLibrary.PlatformDetection.IsMonoInterpreter))] public class Arrays { private class TestClass { } From facd67a18bb11cc86701c0bb390760a5b612471a Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 28 Aug 2024 14:11:19 -0700 Subject: [PATCH 6/6] Update src/tests/baseservices/invalid_operations/Arrays.cs --- src/tests/baseservices/invalid_operations/Arrays.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/baseservices/invalid_operations/Arrays.cs b/src/tests/baseservices/invalid_operations/Arrays.cs index ed93164c30759..d65c73648d126 100644 --- a/src/tests/baseservices/invalid_operations/Arrays.cs +++ b/src/tests/baseservices/invalid_operations/Arrays.cs @@ -6,7 +6,7 @@ using Xunit; -[ConditionalClass(typeof(TestLibrary.PlatformDetection), nameof(TestLibrary.PlatformDetection.IsMonoInterpreter))] +[ActiveIssue("https://github.com/dotnet/runtime/issues/107110", typeof(TestLibrary.PlatformDetection), nameof(TestLibrary.PlatformDetection.IsMonoInterpreter))] public class Arrays { private class TestClass { }