Skip to content

Commit

Permalink
Remove ArrayClass::m_ElementType - get type from the type handle on M…
Browse files Browse the repository at this point in the history
…ethodTable (#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.
  • Loading branch information
elinor-fung committed Aug 29, 2024
1 parent 1b656fe commit 8b34fb0
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 29 deletions.
6 changes: 2 additions & 4 deletions src/coreclr/vm/array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 0 additions & 11 deletions src/coreclr/vm/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/vm/gchelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/vm/methodtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/vm/methodtable.inl
Original file line number Diff line number Diff line change
Expand Up @@ -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<PTR_ArrayClass>(GetClass())->GetArrayElementType();
}

//==========================================================================================
inline DWORD MethodTable::GetRank()
{
Expand Down
51 changes: 51 additions & 0 deletions src/tests/baseservices/invalid_operations/Arrays.cs
Original file line number Diff line number Diff line change
@@ -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<TargetInvocationException>(() => arraySet.Invoke(arr, [0, new object[] { new object() }]));
Assert.IsType<ArrayTypeMismatchException>(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<TargetInvocationException>(() => arraySet.Invoke(arr, [0, new object[1,1] { {new object()} }]));
Assert.IsType<ArrayTypeMismatchException>(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<TargetInvocationException>(() => arraySet.Invoke(arr, [0, new object()]));
Assert.IsType<ArrayTypeMismatchException>(e.InnerException);
}
{
TestClass[,] arr = new TestClass[1,1];
MethodInfo arraySet = typeof(object[,]).GetMethod("Set");
Exception e = Assert.Throws<TargetInvocationException>(() => arraySet.Invoke(arr, [0, 0, new object()]));
Assert.IsType<ArrayTypeMismatchException>(e.InnerException);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
</PropertyGroup>
<ItemGroup>
<Compile Include="ManagedPointers.cs" />
<Compile Include="Arrays.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
Expand Down

0 comments on commit 8b34fb0

Please sign in to comment.