diff --git a/src/coreclr/src/vm/invokeutil.cpp b/src/coreclr/src/vm/invokeutil.cpp index 3d78bc685a625d..a42f638cc84d99 100644 --- a/src/coreclr/src/vm/invokeutil.cpp +++ b/src/coreclr/src/vm/invokeutil.cpp @@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType, } CONTRACTL_END; - // We don't allow setting the field of nullable (hasValue and value) - // Because you can't independantly set them for this type. - if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable())) - COMPlusThrow(kNotSupportedException); - // call the OBJECTREF Throwable = NULL; @@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType, { pDeclMT = declaringType.GetMethodTable(); + // We don't allow setting the field of nullable (hasValue and value) + // Because you can't independently set them for this type. + if (Nullable::IsNullableType(pDeclMT)) + COMPlusThrow(kNotSupportedException); + if (pDeclMT->IsSharedByGenericInstantiations()) COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); } + else + { + pDeclMT = pField->GetModule()->GetGlobalMethodTable(); + } if (*pDomainInitialized == FALSE) { EX_TRY - { - if (declaringType.IsNull()) - { - pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive(); - pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing(); - } - else { pDeclMT->EnsureInstanceActive(); pDeclMT->CheckRunClassInitThrowing(); *pDomainInitialized = TRUE; } - } EX_CATCH_THROWABLE(&Throwable); } #ifdef _DEBUG @@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType, GCPROTECT_END(); } + // Verify we're not trying to set the value of a static initonly field + // once the class has been initialized. + if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes())) + { + DefineFullyQualifiedNameForClassW(); + SString ssFieldName(SString::Utf8, pField->GetName()); + COMPlusThrow(kFieldAccessException, + IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD, + ssFieldName.GetUnicode(), + GetFullyQualifiedNameForClassW(pDeclMT)); + } + // Set the field ARG_SLOT value; @@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ { pDeclMT = declaringType.GetMethodTable(); + // We don't allow getting the field just so we don't have more specical + // cases than we need to. Then we need at least the throw check to ensure + // we don't allow data corruption. + if (Nullable::IsNullableType(pDeclMT)) + COMPlusThrow(kNotSupportedException); + if (pDeclMT->IsSharedByGenericInstantiations()) COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); } + else + { + pDeclMT = pField->GetModule()->GetGlobalMethodTable(); + } if (*pDomainInitialized == FALSE) { EX_TRY - { - if (declaringType.IsNull()) - { - pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive(); - pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing(); - } - else { pDeclMT->EnsureInstanceActive(); pDeclMT->CheckRunClassInitThrowing(); *pDomainInitialized = TRUE; } - } EX_CATCH_THROWABLE(&Throwable); } #ifdef _DEBUG @@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ GCPROTECT_END(); } - // We don't allow getting the field just so we don't have more specical - // cases than we need to. The we need at least the throw check to insure - // we don't allow data corruption, but - if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT)) - COMPlusThrow(kNotSupportedException); - CorElementType fieldElementType = pField->GetFieldType(); switch (fieldElementType) { diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index c530786aa38796..d25c06a275629d 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); - // Verify we're not trying to set the value of a static initonly field - // once the class has been initialized. - if (pFieldDesc->IsStatic()) - { - MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable(); - if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes())) - { - DefineFullyQualifiedNameForClassW(); - SString ssFieldName(SString::Utf8, pFieldDesc->GetName()); - COMPlusThrow(kFieldAccessException, - IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD, - ssFieldName.GetUnicode(), - GetFullyQualifiedNameForClassW(pEnclosingMT)); - } - } - - //TODO: cleanup this function InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized); HELPER_METHOD_FRAME_END(); @@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa // Validate the target/fld type relationship InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref); - InvokeUtil::ValidField(fieldType, pValue); InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized); GCPROTECT_END(); } @@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA TypeHandle targetType = pTarget->type; MethodTable *pEnclosingMT = contextType.GetMethodTable(); - { - // Verify that the value passed can be widened into the target - InvokeUtil::ValidField(fieldType, &gc.oValue); - - // Verify we're not trying to set the value of a static initonly field - // once the class has been initialized. - if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes())) - { - DefineFullyQualifiedNameForClassW(); - SString ssFieldName(SString::Utf8, pField->GetName()); - COMPlusThrow(kFieldAccessException, - IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD, - ssFieldName.GetUnicode(), - GetFullyQualifiedNameForClassW(pEnclosingMT)); - } - } + // Verify that the value passed can be widened into the target + InvokeUtil::ValidField(fieldType, &gc.oValue); CLR_BOOL domainInitialized = FALSE; if (pField->IsStatic() || !targetType.IsValueType()) { diff --git a/src/coreclr/tests/issues.targets b/src/coreclr/tests/issues.targets index 7ddfdc09fed5f0..fc52aac5a77813 100644 --- a/src/coreclr/tests/issues.targets +++ b/src/coreclr/tests/issues.targets @@ -1571,6 +1571,9 @@ needs triage + + https://github.com/dotnet/runtime/issues/37899 + needs triage diff --git a/src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs b/src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs new file mode 100644 index 00000000000000..80b9b497b315a3 --- /dev/null +++ b/src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.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. +// See the LICENSE file in the project root for more information. + +using System; +using System.Reflection; + +public class TestSetValue +{ + public static readonly long MagicNumber = 42; +} + +public class TestSetValueDirect +{ + public static readonly string MagicString = ""; +} + +class Test +{ + public static int Main() + { + // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered + // by the reflection SetValue operation itself. + + try + { + typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789); + Console.WriteLine("FAILED: TestSetValue - Exception expected"); + return -1; + } + catch (FieldAccessException) + { + Console.WriteLine("TestSetValue - Caught expected exception"); + } + + try + { + int i = 0; + typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello"); + Console.WriteLine("FAILED: TestSetValueDirect - Exception expected"); + return -1; + } + catch (FieldAccessException) + { + Console.WriteLine("TestSetValueDirect - Caught expected exception"); + } + return 100; + } +} + + diff --git a/src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj b/src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj new file mode 100644 index 00000000000000..2e607efdde537a --- /dev/null +++ b/src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj @@ -0,0 +1,9 @@ + + + Exe + true + + + + +