From 3b079cf5cd7baa78542d900689ac492c7ca604ef Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 12 Jan 2024 16:08:44 -0600 Subject: [PATCH 1/9] Fast reflection field access --- .../src/System/Reflection/RtFieldInfo.cs | 135 +----- .../src/System/Reflection/RuntimeFieldInfo.cs | 2 +- .../src/System/RuntimeHandles.cs | 14 +- src/coreclr/vm/corelib.h | 8 +- src/coreclr/vm/ecalllist.h | 3 + src/coreclr/vm/field.cpp | 12 +- src/coreclr/vm/invokeutil.cpp | 6 +- src/coreclr/vm/object.h | 1 + src/coreclr/vm/reflectioninvocation.cpp | 111 +++-- src/coreclr/vm/runtimehandles.h | 7 +- src/coreclr/vm/typehandle.cpp | 2 +- .../src/Resources/Strings.resx | 3 + .../System.Private.CoreLib.Shared.projitems | 4 +- .../src/System/Reflection/FieldAccessor.cs | 440 ++++++++++++++++++ .../System.Reflection.Tests/FieldInfoTests.cs | 152 +++++- 15 files changed, 707 insertions(+), 193 deletions(-) create mode 100644 src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs index 15ce2971bed747..953ba012dea22f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs @@ -18,46 +18,18 @@ internal sealed unsafe class RtFieldInfo : RuntimeFieldInfo, IRuntimeFieldInfo // lazy caching private string? m_name; private RuntimeType? m_fieldType; - private InvocationFlags m_invocationFlags; - internal InvocationFlags InvocationFlags - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => (m_invocationFlags & InvocationFlags.Initialized) != 0 ? - m_invocationFlags : InitializeInvocationFlags(); - } + private FieldAccessor? m_fieldAccessor; - [MethodImpl(MethodImplOptions.NoInlining)] - private InvocationFlags InitializeInvocationFlags() + internal FieldAccessor FieldAccessor { - Type? declaringType = DeclaringType; - - InvocationFlags invocationFlags = 0; - - // first take care of all the NO_INVOKE cases - if (declaringType != null && declaringType.ContainsGenericParameters) - { - invocationFlags |= InvocationFlags.NoInvoke; - } - - // If the invocationFlags are still 0, then - // this should be an usable field, determine the other flags - if (invocationFlags == 0) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get { - if ((m_fieldAttributes & FieldAttributes.InitOnly) != 0) - invocationFlags |= InvocationFlags.SpecialField; - - if ((m_fieldAttributes & FieldAttributes.HasFieldRVA) != 0) - invocationFlags |= InvocationFlags.SpecialField; - - // find out if the field type is one of the following: Primitive, Enum or Pointer - Type fieldType = FieldType; - if (fieldType.IsPointer || fieldType.IsEnum || fieldType.IsPrimitive) - invocationFlags |= InvocationFlags.FieldSpecialCast; + m_fieldAccessor ??= new FieldAccessor(this); + return m_fieldAccessor; } - - // must be last to avoid threading problems - return m_invocationFlags = invocationFlags | InvocationFlags.Initialized; } + #endregion #region Constructor @@ -75,28 +47,6 @@ internal RtFieldInfo( #endregion #region Internal Members - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void CheckConsistency(object? target) - { - // only test instance fields - if ((m_fieldAttributes & FieldAttributes.Static) != FieldAttributes.Static) - { - if (!m_declaringType.IsInstanceOfType(target)) - { - if (target == null) - { - throw new TargetException(SR.RFLCT_Targ_StatFldReqTarg); - } - else - { - throw new ArgumentException( - SR.Format(SR.Arg_FieldDeclTarget, - Name, m_declaringType, target.GetType())); - } - } - } - } - internal override bool CacheEquals(object? o) { return o is RtFieldInfo m && m.m_fieldHandle == m_fieldHandle; @@ -131,36 +81,7 @@ public override int GetHashCode() => #region FieldInfo Overrides [DebuggerStepThrough] [DebuggerHidden] - public override object? GetValue(object? obj) - { - InvocationFlags invocationFlags = InvocationFlags; - RuntimeType? declaringType = DeclaringType as RuntimeType; - - if ((invocationFlags & InvocationFlags.NoInvoke) != 0) - { - if (declaringType != null && DeclaringType!.ContainsGenericParameters) - throw new InvalidOperationException(SR.Arg_UnboundGenField); - - throw new FieldAccessException(); - } - - CheckConsistency(obj); - - RuntimeType fieldType = (RuntimeType)FieldType; - - bool domainInitialized = false; - if (declaringType == null) - { - return RuntimeFieldHandle.GetValue(this, obj, fieldType, null, ref domainInitialized); - } - else - { - domainInitialized = declaringType.DomainInitialized; - object? retVal = RuntimeFieldHandle.GetValue(this, obj, fieldType, declaringType, ref domainInitialized); - declaringType.DomainInitialized = domainInitialized; - return retVal; - } - } + public override object? GetValue(object? obj) => FieldAccessor.GetValue(obj); public override object GetRawConstantValue() { throw new InvalidOperationException(); } @@ -180,45 +101,7 @@ public override int GetHashCode() => [DebuggerStepThrough] [DebuggerHidden] public override void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) - { - InvocationFlags invocationFlags = InvocationFlags; - RuntimeType? declaringType = DeclaringType as RuntimeType; - - if ((invocationFlags & InvocationFlags.NoInvoke) != 0) - { - if (declaringType != null && declaringType.ContainsGenericParameters) - throw new InvalidOperationException(SR.Arg_UnboundGenField); - - throw new FieldAccessException(); - } - - CheckConsistency(obj); - - RuntimeType fieldType = (RuntimeType)FieldType; - if (value is null) - { - if (fieldType.IsActualValueType) - { - fieldType.CheckValue(ref value, binder, culture, invokeAttr); - } - } - else if (!ReferenceEquals(value.GetType(), fieldType)) - { - fieldType.CheckValue(ref value, binder, culture, invokeAttr); - } - - bool domainInitialized = false; - if (declaringType is null) - { - RuntimeFieldHandle.SetValue(this, obj, value, fieldType, m_fieldAttributes, null, ref domainInitialized); - } - else - { - domainInitialized = declaringType.DomainInitialized; - RuntimeFieldHandle.SetValue(this, obj, value, fieldType, m_fieldAttributes, declaringType, ref domainInitialized); - declaringType.DomainInitialized = domainInitialized; - } - } + => FieldAccessor.SetValue(obj, value, invokeAttr, binder, culture); [DebuggerStepThrough] [DebuggerHidden] diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs index dab1d07145dbd9..a314edaab2a15f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs @@ -11,7 +11,7 @@ internal abstract class RuntimeFieldInfo : FieldInfo #region Private Data Members private readonly BindingFlags m_bindingFlags; protected readonly RuntimeTypeCache m_reflectedTypeCache; - protected readonly RuntimeType m_declaringType; + protected internal readonly RuntimeType m_declaringType; #endregion #region Constructor diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 73b9bb167f7a7f..32b68bb68455a1 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -1087,6 +1087,7 @@ public RuntimeFieldInfoStub(RuntimeFieldHandleInternal fieldHandle, object keepa private object? m_d; private int m_b; private object? m_e; + private object? m_f; private RuntimeFieldHandleInternal m_fieldHandle; #pragma warning restore 414, 169, IDE0044 @@ -1189,17 +1190,26 @@ internal static RuntimeType GetApproxDeclaringType(IRuntimeFieldInfo field) return type; } + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern bool IsFastPathSupported(RtFieldInfo field); + + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern int GetInstanceFieldAddress(RtFieldInfo field); + + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern IntPtr GetStaticFieldAddress(RtFieldInfo field, ref bool isBoxed); + [MethodImpl(MethodImplOptions.InternalCall)] internal static extern int GetToken(RtFieldInfo field); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); + internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object? GetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, RuntimeType? contextType); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, FieldAttributes fieldAttr, RuntimeType? declaringType, ref bool domainInitialized); + internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void SetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, object? value, RuntimeType? contextType); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 2fc34d04372c5e..e3211fe3d5d0f5 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -362,13 +362,13 @@ DEFINE_FIELD(RT_TYPE_HANDLE, M_TYPE, m_type) DEFINE_CLASS(TYPE_NAME_PARSER, Reflection, TypeNameParser) DEFINE_METHOD(TYPE_NAME_PARSER, GET_TYPE_HELPER, GetTypeHelper, SM_Type_CharPtr_RuntimeAssembly_Bool_Bool_RetRuntimeType) -DEFINE_CLASS_U(Reflection, RtFieldInfo, NoClass) -DEFINE_FIELD_U(m_fieldHandle, ReflectFieldObject, m_pFD) +DEFINE_CLASS_U(Reflection, RtFieldInfo, NoClass) +DEFINE_FIELD_U(m_fieldHandle, ReflectFieldObject, m_pFD) DEFINE_CLASS(RT_FIELD_INFO, Reflection, RtFieldInfo) DEFINE_FIELD(RT_FIELD_INFO, HANDLE, m_fieldHandle) -DEFINE_CLASS_U(System, RuntimeFieldInfoStub, ReflectFieldObject) -DEFINE_FIELD_U(m_fieldHandle, ReflectFieldObject, m_pFD) +DEFINE_CLASS_U(System, RuntimeFieldInfoStub, ReflectFieldObject) +DEFINE_FIELD_U(m_fieldHandle, ReflectFieldObject, m_pFD) DEFINE_CLASS(STUBFIELDINFO, System, RuntimeFieldInfoStub) #if FOR_ILLINK DEFINE_METHOD(STUBFIELDINFO, CTOR, .ctor, IM_RetVoid) diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 88d9b03adcbc8b..cfaa20e581c61f 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -242,6 +242,9 @@ FCFuncStart(gCOMFieldHandleNewFuncs) FCFuncElement("GetStaticFieldForGenericType", RuntimeFieldHandle::GetStaticFieldForGenericType) FCFuncElement("AcquiresContextFromThis", RuntimeFieldHandle::AcquiresContextFromThis) FCFuncElement("GetLoaderAllocator", RuntimeFieldHandle::GetLoaderAllocator) + FCFuncElement("IsFastPathSupported", RuntimeFieldHandle::IsFastPathSupported) + FCFuncElement("GetInstanceFieldAddress", RuntimeFieldHandle::GetInstanceFieldAddress) + FCFuncElement("GetStaticFieldAddress", RuntimeFieldHandle::GetStaticFieldAddress) FCFuncEnd() FCFuncStart(gCOMModuleHandleFuncs) diff --git a/src/coreclr/vm/field.cpp b/src/coreclr/vm/field.cpp index 9b70f06e38da5b..bcbb679e20783c 100644 --- a/src/coreclr/vm/field.cpp +++ b/src/coreclr/vm/field.cpp @@ -180,8 +180,8 @@ void* FieldDesc::GetStaticAddress(void *base) void* ret = GetStaticAddressHandle(base); // Get the handle - // For value classes, the handle points at an OBJECTREF - // which holds the boxed value class, so dereference and unbox. + // For value classes, the handle points at an OBJECTREF + // which holds the boxed value class, so dereference and unbox. if (GetFieldType() == ELEMENT_TYPE_VALUETYPE && !IsRVA()) { OBJECTREF obj = ObjectToOBJECTREF(*(Object**) ret); @@ -211,11 +211,10 @@ MethodTable * FieldDesc::GetExactDeclaringType(MethodTable * ownerOrSubType) #endif // #ifndef DACCESS_COMPILE - // static value classes are actually stored in their boxed form. - // this means that their address moves. +// Static value classes are actually stored in their boxed form. +// This means that their address moves. PTR_VOID FieldDesc::GetStaticAddressHandle(PTR_VOID base) { - CONTRACTL { INSTANCE_CHECK; @@ -256,7 +255,6 @@ PTR_VOID FieldDesc::GetStaticAddressHandle(PTR_VOID base) } #endif // FEATURE_METADATA_UPDATER - if (IsRVA()) { Module* pModule = GetModule(); @@ -271,12 +269,10 @@ PTR_VOID FieldDesc::GetStaticAddressHandle(PTR_VOID base) PTR_VOID ret = PTR_VOID(dac_cast(base) + GetOffset()); - return ret; } - // These routines encapsulate the operation of getting and setting // fields. void FieldDesc::GetInstanceField(OBJECTREF o, VOID * pOutVal) diff --git a/src/coreclr/vm/invokeutil.cpp b/src/coreclr/vm/invokeutil.cpp index eb8462ed16f245..055a8d0497ed10 100644 --- a/src/coreclr/vm/invokeutil.cpp +++ b/src/coreclr/vm/invokeutil.cpp @@ -971,8 +971,6 @@ void InvokeUtil::SetValidField(CorElementType fldType, } } -// GetFieldValue -// This method will return an ARG_SLOT containing the value of the field. // GetFieldValue // This method will return an ARG_SLOT containing the value of the field. OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL *pDomainInitialized) { @@ -999,7 +997,7 @@ 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 + // We don't allow getting the field just so we don't have more special // 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)) @@ -1084,7 +1082,7 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ case ELEMENT_TYPE_VALUETYPE: { - // Value classes require createing a boxed version of the field and then + // Value classes require creating a boxed version of the field and then // copying from the source... // Allocate an object to return... _ASSERTE(!fieldType.IsTypeDesc()); diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index 91bbc95304ce1e..915e45deca0636 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -1130,6 +1130,7 @@ class ReflectFieldObject : public BaseObjectWithCachedData INT32 m_empty2; OBJECTREF m_empty3; OBJECTREF m_empty4; + OBJECTREF m_empty5; FieldDesc * m_pFD; public: diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index 72cf39758b1fdb..c5d81ee538e4c4 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -25,7 +25,7 @@ #include "dbginterface.h" #include "argdestination.h" -FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { +FCIMPL4(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE) { CONTRACTL { FCALL_CHECK; } @@ -50,22 +50,14 @@ FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, TypeHandle fieldType = gc.pFieldType->GetType(); TypeHandle declaringType = (gc.pDeclaringType != NULL) ? gc.pDeclaringType->GetType() : TypeHandle(); - Assembly *pAssem; - if (declaringType.IsNull()) - { - // global field - pAssem = gc.refField->GetField()->GetModule()->GetAssembly(); - } - else - { - pAssem = declaringType.GetAssembly(); - } - OBJECTREF rv = NULL; // not protected + // The caller should have ran the cctor already. + CLR_BOOL domainInitialized = TRUE; + HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); // There can be no GC after this until the Object is returned. - rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, pDomainInitialized); + rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, &domainInitialized); HELPER_METHOD_FRAME_END(); return OBJECTREFToObject(rv); @@ -169,7 +161,7 @@ FCIMPL2(Object*, ReflectionInvocation::AllocateValueType, ReflectClassBaseObject } FCIMPLEND -FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, DWORD attr, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { +FCIMPL5(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE) { CONTRACTL { FCALL_CHECK; } @@ -195,24 +187,16 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob TypeHandle fieldType = gc.fieldType->GetType(); TypeHandle declaringType = gc.declaringType != NULL ? gc.declaringType->GetType() : TypeHandle(); - Assembly *pAssem; - if (declaringType.IsNull()) - { - // global field - pAssem = gc.refField->GetField()->GetModule()->GetAssembly(); - } - else - { - pAssem = declaringType.GetAssembly(); - } - FC_GC_POLL_NOT_NEEDED(); FieldDesc* pFieldDesc = gc.refField->GetField(); + // The caller should have ran the cctor already. + CLR_BOOL domainInitialized = TRUE; + HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); - InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized); + InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, &domainInitialized); HELPER_METHOD_FRAME_END(); } @@ -1243,6 +1227,81 @@ lExit: ; } FCIMPLEND +FCIMPL1(FC_BOOL_RET, RuntimeFieldHandle::IsFastPathSupported, ReflectFieldObject *pFieldUNSAFE) +{ + CONTRACTL { + FCALL_CHECK; + } + CONTRACTL_END; + + REFLECTFIELDREF refField = (REFLECTFIELDREF)ObjectToOBJECTREF(pFieldUNSAFE); + if (refField == NULL) + FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); + + FieldDesc* pFieldDesc = refField->GetField(); + return pFieldDesc->IsEnCNew() ? FALSE : TRUE; +} +FCIMPLEND + +FCIMPL1(INT32, RuntimeFieldHandle::GetInstanceFieldAddress, ReflectFieldObject *pFieldUNSAFE) +{ + CONTRACTL { + FCALL_CHECK; + PRECONDITION(CheckPointer(pFieldUNSAFE)); + } + CONTRACTL_END; + + REFLECTFIELDREF refField = (REFLECTFIELDREF)ObjectToOBJECTREF(pFieldUNSAFE); + if (refField == NULL) + FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); + + FieldDesc* pFieldDesc = refField->GetField(); + + // IsFastPathSupported needs to checked before calling this method. + _ASSERTE(!pFieldDesc->IsEnCNew()); + + return pFieldDesc->GetOffset(); +} +FCIMPLEND + +FCIMPL2(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pFieldUNSAFE, CLR_BOOL *isBoxed) +{ + CONTRACTL { + FCALL_CHECK; + PRECONDITION(CheckPointer(pFieldUNSAFE)); + } + CONTRACTL_END; + + REFLECTFIELDREF refField = (REFLECTFIELDREF)ObjectToOBJECTREF(pFieldUNSAFE); + if (refField == NULL) + FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); + + void *ret = NULL; + FieldDesc* pFieldDesc = refField->GetField(); + + // IsFastPathSupported needs to checked before calling this method. + _ASSERTE(!pFieldDesc->IsEnCNew()); + + GCPROTECT_BEGIN(refField); + + if (refField->GetField()->IsThreadStatic()) { + ret = Thread::GetStaticFieldAddress(pFieldDesc); + } + else { + PTR_BYTE base = 0; + if (!pFieldDesc->IsRVA()) // For RVA the base is ignored. + base = pFieldDesc->GetBase(); + + ret = pFieldDesc->GetStaticAddressHandle(base); + } + + *isBoxed = ((pFieldDesc->GetFieldType() == ELEMENT_TYPE_VALUETYPE && !pFieldDesc->IsRVA())) ? TRUE : FALSE; + + GCPROTECT_END(); + return ret; +} +FCIMPLEND + extern "C" void QCALLTYPE ReflectionInvocation_CompileMethod(MethodDesc * pMD) { QCALL_CONTRACT; diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 694a25a30623da..c950e26538efee 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -292,10 +292,13 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_Destroy(MethodDesc * pMethod); class RuntimeFieldHandle { public: - static FCDECL5(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); - static FCDECL7(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, DWORD attr, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); + static FCDECL4(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType); + static FCDECL5(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType); static FCDECL4(Object*, GetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, ReflectClassBaseObject *pDeclaringType); static FCDECL5(void, SetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, Object *valueUNSAFE, ReflectClassBaseObject *pContextType); + static FCDECL1(FC_BOOL_RET, IsFastPathSupported, ReflectFieldObject *pField); + static FCDECL1(INT32, GetInstanceFieldAddress, ReflectFieldObject *pField); + static FCDECL2(PVOID, GetStaticFieldAddress, ReflectFieldObject *pField, CLR_BOOL* isBoxed); static FCDECL1(StringObject*, GetName, ReflectFieldObject *pFieldUNSAFE); static FCDECL1(LPCUTF8, GetUtf8Name, FieldDesc *pField); diff --git a/src/coreclr/vm/typehandle.cpp b/src/coreclr/vm/typehandle.cpp index 053cc759217c76..48855c8b3ec3b3 100644 --- a/src/coreclr/vm/typehandle.cpp +++ b/src/coreclr/vm/typehandle.cpp @@ -1516,7 +1516,7 @@ TypeKey TypeHandle::GetTypeKey() const #ifdef _DEBUG // Check that a type handle matches the key provided -CHECK TypeHandle::CheckMatchesKey(TypeKey *pKey) const +CHECK TypeHandle::CheckMatchesKey(const TypeKey *pKey) const { WRAPPER_NO_CONTRACT; CONTRACT_VIOLATION(TakesLockViolation); // this is debug-only code diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 53cc0c13d1abb6..db68df00b8d15f 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -4283,4 +4283,7 @@ This operation is not available because the reflection support was disabled at compile time. + + Cannot set initonly static field '{0}' after type '{1}' is initialized. + \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 1c1d6d3530110a..322e6d145a2bf9 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -4,7 +4,6 @@ c5ed3c1d-b572-46f1-8f96-522a85ce1179 System.Private.CoreLib.Strings true - true @@ -702,6 +701,7 @@ + @@ -2743,4 +2743,4 @@ - + \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs new file mode 100644 index 00000000000000..970331ce8e023c --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -0,0 +1,440 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Runtime.CompilerServices; +using System.Threading; + +namespace System.Reflection +{ + internal sealed class FieldAccessor + { + private readonly IntPtr _addressOrOffset; + private readonly RtFieldInfo _fieldInfo; + private readonly unsafe MethodTable* _methodTable; + private readonly FieldAccessorType _fieldAccessType; + private readonly PrimitiveFieldSize _primitiveFieldSize; + internal FieldAccessor(FieldInfo fieldInfo) + { + _fieldInfo = (RtFieldInfo)fieldInfo; + RuntimeType? declaringType = (RuntimeType?)_fieldInfo.DeclaringType; + + // Call the class constructor if not already. + if (declaringType is null) + { + InvokeModuleConstructor(fieldInfo.Module); + } + else if (!declaringType.DomainInitialized) + { + InvokeClassConstructor(); + } + + // Cached the method table for performance. + unsafe + { + _methodTable = (MethodTable*)_fieldInfo.FieldType.TypeHandle.Value; + } + + // Initialize for the type of field. + if (declaringType is null || declaringType.ContainsGenericParameters) + { + _addressOrOffset = default; + _fieldAccessType = FieldAccessorType.NoInvoke; + } + else if (!RuntimeFieldHandle.IsFastPathSupported(_fieldInfo)) + { + // Currently this is only true for newly added fields from EnC. + _addressOrOffset = default; + _fieldAccessType = FieldAccessorType.SlowPath; + } + else + { + Type fieldType = _fieldInfo.FieldType; + + if (fieldInfo.IsStatic) + { + bool isBoxed = false; + _addressOrOffset = RuntimeFieldHandle.GetStaticFieldAddress(_fieldInfo, ref isBoxed); + + if (fieldType.IsValueType) + { + // The runtime stores non-primitive value types as boxed. + _fieldAccessType = isBoxed ? FieldAccessorType.StaticValueTypeBoxed : FieldAccessorType.StaticValueType; + } + else if (fieldType.IsPointer) + { + Debug.Assert(isBoxed == false); + _fieldAccessType = FieldAccessorType.StaticPointerType; + } + else if (fieldType.IsFunctionPointer) + { + Debug.Assert(isBoxed == false); + _fieldAccessType = FieldAccessorType.StaticValueType; + unsafe + { + _methodTable = (MethodTable*)typeof(IntPtr).TypeHandle.Value; + } + } + else + { + Debug.Assert(isBoxed == false); + _fieldAccessType = FieldAccessorType.StaticReferenceType; + } + } + else + { + _addressOrOffset = RuntimeFieldHandle.GetInstanceFieldAddress(_fieldInfo); + + if (fieldType.IsValueType) + { + _fieldAccessType = FieldAccessorType.InstanceValueType; + } + else if (fieldType.IsPointer) + { + _fieldAccessType = FieldAccessorType.InstancePointerType; + } + else if (fieldType.IsFunctionPointer) + { + _fieldAccessType = FieldAccessorType.InstanceValueType; + unsafe + { + _methodTable = (MethodTable*)typeof(IntPtr).TypeHandle.Value; + } + } + else + { + _fieldAccessType = FieldAccessorType.InstanceReferenceType; + } + } + + _primitiveFieldSize = GetPrimitiveFieldSize(fieldType, _fieldAccessType); + } + } + + public object? GetValue(object? obj) + { + object? ret = null; + + unsafe + { + switch (_fieldAccessType) + { + case FieldAccessorType.InstanceReferenceType: + VerifyTarget(obj); + Debug.Assert(obj != null); + ret = Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)); + break; + + case FieldAccessorType.InstanceValueType: + VerifyTarget(obj); + Debug.Assert(obj != null); + ret = RuntimeHelpers.Box( + _methodTable, + ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)); + + break; + + case FieldAccessorType.InstancePointerType: + VerifyTarget(obj); + Debug.Assert(obj != null); + ret = Pointer.Box( + (void*)Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + _fieldInfo.FieldType); + + break; + + case FieldAccessorType.StaticReferenceType: + ret = Volatile.Read(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset)); + break; + + case FieldAccessorType.StaticValueType: + ret = RuntimeHelpers.Box(_methodTable, ref Unsafe.AsRef(_addressOrOffset.ToPointer())); + break; + + case FieldAccessorType.StaticValueTypeBoxed: + // Re-box the value. + ret = RuntimeHelpers.Box( + _methodTable, + ref Unsafe.As(ref *(IntPtr*)_addressOrOffset).GetRawData()); + + break; + + case FieldAccessorType.StaticPointerType: + ret = Pointer.Box((void*)Unsafe.As( + ref Unsafe.AsRef(_addressOrOffset.ToPointer())), _fieldInfo.FieldType); + + break; + + case FieldAccessorType.SlowPath: + if (!IsStatic()) + { + VerifyTarget(obj); + } + + ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType); + break; + + case FieldAccessorType.NoInvoke: + if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) + throw new InvalidOperationException(SR.Arg_UnboundGenField); + + throw new FieldAccessException(); +#if DEBUG + default: + throw new Exception("Unknown enum value"); +#endif + } + } + + return ret; + } + + public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) + { + RuntimeType fieldType = (RuntimeType)_fieldInfo.FieldType; + + switch (_fieldAccessType) + { + case FieldAccessorType.InstanceReferenceType: + VerifyTarget(obj); + CheckValue(); + Debug.Assert(obj != null); + Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)) = value; + return; + + case FieldAccessorType.StaticReferenceType: + VerifyInitOnly(); + CheckValue(); + unsafe + { + Unsafe.As(ref *(IntPtr*)_addressOrOffset) = value; + } + return; + + case FieldAccessorType.NoInvoke: + if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) + throw new InvalidOperationException(SR.Arg_UnboundGenField); + + throw new FieldAccessException(); + + case FieldAccessorType.InstanceValueType: + VerifyTarget(obj); + CheckValue(); + Debug.Assert(obj != null); + switch (_primitiveFieldSize) + { + case PrimitiveFieldSize.Size1: + Volatile.Write( + ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset), + value!.GetRawData()); + + return; + + case PrimitiveFieldSize.Size2: + Volatile.Write( + ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + Unsafe.As(ref value!.GetRawData())); + + return; + + case PrimitiveFieldSize.Size4: + Volatile.Write( + ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + Unsafe.As(ref value!.GetRawData())); + + return; + + case PrimitiveFieldSize.Size8: + Volatile.Write( + ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + Unsafe.As(ref value!.GetRawData())); + + return; + + default: + RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType); + return; + } + + + case FieldAccessorType.StaticValueType: + VerifyInitOnly(); + CheckValue(); + unsafe + { + switch (_primitiveFieldSize) + { + case PrimitiveFieldSize.Size1: + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + value!.GetRawData()); + + break; + + case PrimitiveFieldSize.Size2: + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + + break; + + case PrimitiveFieldSize.Size4: + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + + break; + + case PrimitiveFieldSize.Size8: + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + break; + + case PrimitiveFieldSize.NotPrimitive: + RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType); + break; + } + + return; + } + } + + if (!IsStatic()) + { + VerifyTarget(obj); + } + + CheckValue(); + RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType); + + void CheckValue() + { + if (value is null) + { + if (fieldType.IsActualValueType) + { + fieldType.CheckValue(ref value, binder, culture, invokeAttr); + } + } + else if (!ReferenceEquals(value.GetType(), fieldType)) + { + fieldType.CheckValue(ref value, binder, culture, invokeAttr); + } + } + } + + private bool IsStatic() => (_fieldInfo.Attributes & FieldAttributes.Static) == FieldAttributes.Static; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void VerifyTarget(object? target) + { + Debug.Assert(!IsStatic()); + + if (!_fieldInfo.m_declaringType.IsInstanceOfType(target)) + { + if (target == null) + { + ThrowHelperTargetException(); + } + else + { + ThrowHelperArgumentException(target); + } + } + } + + private static void ThrowHelperTargetException() => throw new TargetException(SR.RFLCT_Targ_StatFldReqTarg); + private void ThrowHelperArgumentException(object target) => throw new ArgumentException( + SR.Format(SR.Arg_FieldDeclTarget, _fieldInfo.Name, _fieldInfo.DeclaringType, target.GetType())); + private static void ThrowHelperFieldAccessException(string fieldName, string? declaringTypeName) => throw new FieldAccessException(SR.Format(SR.RFLCT_CannotSetInitonlyStaticField, fieldName, declaringTypeName)); + + internal void VerifyInitOnly() + { + Debug.Assert(IsStatic()); + if ((_fieldInfo.Attributes & FieldAttributes.InitOnly) == FieldAttributes.InitOnly) + { + ThrowHelperFieldAccessException(_fieldInfo.Name, _fieldInfo.DeclaringType!.FullName); + } + } + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2059:RunClassConstructor", + Justification = "This represents the static constructor, so if this object was created, the static constructor exists.")] + private void InvokeClassConstructor() + { + RuntimeHelpers.RunClassConstructor(_fieldInfo.DeclaringType!.TypeHandle); + } + + private static void InvokeModuleConstructor(Module module) + { + RuntimeHelpers.RunModuleConstructor(module.ModuleHandle); + } + + /// + /// Currently we only optimize for primitive types; these are simple because they are atomic write operations, are + /// not boxed by the runtime when stored as a static field, and don't need special nullable, GC or alignment checks. + /// + private static PrimitiveFieldSize GetPrimitiveFieldSize(Type fieldType, FieldAccessorType fieldAccessType) + { + PrimitiveFieldSize size = PrimitiveFieldSize.NotPrimitive; + + if (fieldAccessType == FieldAccessorType.InstanceValueType || + fieldAccessType == FieldAccessorType.StaticValueType) + { + if (fieldType == typeof(byte) || + fieldType == typeof(sbyte) || + fieldType == typeof(bool)) + size = PrimitiveFieldSize.Size1; + else if (fieldType == typeof(short) || + fieldType == typeof(ushort) || + fieldType == typeof(char)) + size = PrimitiveFieldSize.Size2; + else if (fieldType == typeof(int) || + fieldType == typeof(uint) || + fieldType == typeof(float)) + size = PrimitiveFieldSize.Size4; + else if (fieldType == typeof(long) || + fieldType == typeof(ulong) || + fieldType == typeof(double)) + size = PrimitiveFieldSize.Size8; + else if (fieldType.IsFunctionPointer) + { + if (IntPtr.Size == 4) + { + size = PrimitiveFieldSize.Size4; + } + else if (IntPtr.Size == 8) + { + size = PrimitiveFieldSize.Size8; + } + } + } + + return size; + } + + private enum FieldAccessorType : short + { + InstanceReferenceType = 0, + InstanceValueType = 1, + InstancePointerType = 2, + StaticReferenceType = 3, + StaticValueType = 4, + StaticValueTypeBoxed = 5, + StaticPointerType = 6, + SlowPath = 7, + NoInvoke = 8, + } + + private enum PrimitiveFieldSize : short + { + Size1, + Size2, + Size4, + Size8, + NotPrimitive + } + } +} diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index 5f107d9b44e585..0308c3e6baff1c 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Xunit; @@ -29,7 +31,7 @@ public void SetValue_ConstantField_ThrowsFieldAccessException(string field, obje [Fact] public void SetValue_ReadonlyField() { - FieldInfo fieldInfo = typeof(FieldInfoTests).GetTypeInfo().GetDeclaredField("readonlyIntField"); + FieldInfo fieldInfo = typeof(FieldInfoTests).GetTypeInfo().GetDeclaredField(nameof(readonlyIntField)); FieldInfoTests myInstance = new FieldInfoTests(); object current = fieldInfo.GetValue(myInstance); @@ -55,11 +57,34 @@ public static void CustomAttributes(Type type, string expectedToString) public static IEnumerable GetValue_TestData() { - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.s_intField), new FieldInfoTests(), 100 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.s_intField), null, 100 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.intField), new FieldInfoTests(), 101 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.s_stringField), new FieldInfoTests(), "static" }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.stringField), new FieldInfoTests(), "non static" }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_boolField), null, true }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_intField), new FieldInfoTests(), 100 }; // Non-null 'obj' ignored. + yield return new object[] { typeof(FieldInfoTests), nameof(s_intField), null, 100 }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_stringField), null, "static" }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_myStruct), null, new MyStruct() }; + + yield return new object[] { typeof(FieldInfoTests), nameof(boolField), new FieldInfoTests(), true }; + yield return new object[] { typeof(FieldInfoTests), nameof(intField), new FieldInfoTests(), 101 }; + yield return new object[] { typeof(FieldInfoTests), nameof(stringField), new FieldInfoTests(), "non static" }; + yield return new object[] { typeof(FieldInfoTests), nameof(_myStruct), new FieldInfoTests(), new MyStruct() }; + + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_boolField), null, true }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intField), null, 100 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intField), null, 100 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_constIntField), null, 102 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_stringField), null, "static" }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_objectField), null, MyStruct.s_objectField }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_fcnPtr), null, (IntPtr)45 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intPtr), null, MyStruct.s_intPtrForComparison }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), new MyStruct(), MyStruct.s_rvaIntField }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.threadStatic_intField), null, 100 }; + + yield return new object[] { typeof(MyStruct), nameof(MyStruct.stringField), new MyStruct(), "non static" }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.intField), new MyStruct(), 101 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.intPtr), new MyStruct(), MyStruct.intPtrForComparison }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.fcnPtr), new MyStruct(), (IntPtr)44 }; + + yield return new object[] { typeof(MyStruct_OnlyPrimitiveTypes), nameof(MyStruct_OnlyPrimitiveTypes.intField), new MyStruct_OnlyPrimitiveTypes(), 101 }; } [Theory] @@ -70,6 +95,22 @@ public void GetValue(Type type, string name, object obj, object expected) Assert.Equal(expected, fieldInfo.GetValue(obj)); } + [Fact] + public void GetAndSetValueTypeFromStatic() + { + FieldInfo fieldInfo = GetField(typeof(FieldInfoTests), nameof(s_myStruct_GetAndSet)); + s_myStruct_GetAndSet.intField = 10; + object obj = fieldInfo.GetValue(null); + Assert.Equal(10, ((MyStruct)obj).intField); + s_myStruct_GetAndSet.intField = 11; + + // Make sure the previously boxed value didn't change. The runtime boxes non-primitive value types internally. + Assert.Equal(10, ((MyStruct)obj).intField); + + obj = fieldInfo.GetValue(null); + Assert.Equal(11, ((MyStruct)obj).intField); + } + public static IEnumerable GetValue_Invalid_TestData() { yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.stringField), null, typeof(TargetException) }; @@ -86,14 +127,37 @@ public void GetValue_Invalid(Type type, string name, object obj, Type exceptionT public static IEnumerable SetValue_TestData() { - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.s_intField), new FieldInfoTests(), 1000, 1000 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.s_intField), null, 1000, 1000 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.intField), new FieldInfoTests(), 1000, 1000 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.s_stringField), new FieldInfoTests(), "new", "new" }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.stringField), new FieldInfoTests(), "new", "new" }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.shortEnumField), new FieldInfoTests(), (byte)1, (ShortEnum)1 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.intEnumField), new FieldInfoTests(), (short)2, (IntEnum)2 }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.longEnumField), new FieldInfoTests(), (int)3, (LongEnum)3 }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_boolField_Set), null, true, true }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_boolField_Set), null, false, false }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_intField_Set), new FieldInfoTests(), 1000, 1000 }; // Non-null 'obj' ignored. + yield return new object[] { typeof(FieldInfoTests), nameof(s_intField_Set), null, 1001, 1001 }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_stringField_Set), null, "new", "new" }; + yield return new object[] { typeof(FieldInfoTests), nameof(s_myStruct_Set), null, s_myStruct_Set, s_myStruct_Set }; + + yield return new object[] { typeof(FieldInfoTests), nameof(boolField), new FieldInfoTests(), true, true }; + yield return new object[] { typeof(FieldInfoTests), nameof(boolField), new FieldInfoTests(), false, false }; + yield return new object[] { typeof(FieldInfoTests), nameof(stringField), new FieldInfoTests(), "new", "new" }; + yield return new object[] { typeof(FieldInfoTests), nameof(shortEnumField), new FieldInfoTests(), (byte)1, (ShortEnum)1 }; + yield return new object[] { typeof(FieldInfoTests), nameof(intEnumField), new FieldInfoTests(), (short)2, (IntEnum)2 }; + yield return new object[] { typeof(FieldInfoTests), nameof(longEnumField), new FieldInfoTests(), (int)3, (LongEnum)3 }; + yield return new object[] { typeof(FieldInfoTests), nameof(_myStruct), new FieldInfoTests(), s_myStruct_Set, s_myStruct_Set }; + + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_boolField_Set), null, true, true }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_boolField_Set), null, false, false }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intField_Set), null, 1001, 1001 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_stringField_Set), null, "new", "new" }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_objectField_Set), null, MyStruct.s_objectField, MyStruct.s_objectField }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intPtr_Set), null, MyStruct.s_intPtrForComparison, MyStruct.s_intPtrForComparison }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_fcnPtr_Set), null, (IntPtr)201, (IntPtr)201 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.threadStatic_intField_Set), null, 100, 100 }; + + yield return new object[] { typeof(MyStruct), nameof(MyStruct.boolField), new MyStruct(), true, true }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.boolField), new MyStruct(), false, false }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.intField), new MyStruct(), 1002, 1002 }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.stringField), new MyStruct(), "new", "new" }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.objectField), new MyStruct(), MyStruct.s_objectField, MyStruct.s_objectField }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.intPtr), new MyStruct(), MyStruct.s_intPtrForComparison, MyStruct.s_intPtrForComparison }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.fcnPtr), new MyStruct(), (IntPtr)200, (IntPtr)200 }; } [Theory] @@ -115,9 +179,11 @@ public void SetValue(Type type, string name, object obj, object value, object ex public static IEnumerable SetValue_Invalid_TestData() { - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.stringField), null, "new", typeof(TargetException) }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.stringField), new object(), "new", typeof(ArgumentException) }; - yield return new object[] { typeof(FieldInfoTests), nameof(FieldInfoTests.stringField), new FieldInfoTests(), 100, typeof(ArgumentException) }; + yield return new object[] { typeof(FieldInfoTests), nameof(stringField), null, "new", typeof(TargetException) }; + yield return new object[] { typeof(FieldInfoTests), nameof(stringField), new object(), "new", typeof(ArgumentException) }; + yield return new object[] { typeof(FieldInfoTests), nameof(stringField), new FieldInfoTests(), 100, typeof(ArgumentException) }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_constIntField), null, 100, typeof(FieldAccessException) }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), null, new int[] { 3, 4 }, typeof(FieldAccessException) }; } [Theory] @@ -239,6 +305,7 @@ public void IsPrivate(Type type, string name, bool expected) [Theory] [InlineData(typeof(FieldInfoTests), nameof(FieldInfoTests.readonlyIntField), true)] + [InlineData(typeof(FieldInfoTests), nameof(FieldInfoTests.s_readonlyIntField), true)] [InlineData(typeof(FieldInfoTests), nameof(FieldInfoTests.intField), false)] public void IsInitOnly(Type type, string name, bool expected) { @@ -462,12 +529,23 @@ private static FieldInfo GetField(Type type, string name) public const long ConstInt64Field = 1000; public const byte ConstByteField = 0; + public static bool s_boolField = true; + public static bool s_boolField_Set = false; public static int s_intField = 100; + public static int s_intField_Set = 0; public static string s_stringField = "static"; + public static string s_stringField_Set = "static"; + public static readonly int s_readonlyIntField = 100; + public bool boolField = true; public int intField = 101; public string stringField = "non static"; + public MyStruct _myStruct = new MyStruct(); + public static MyStruct s_myStruct = new MyStruct(); + public static MyStruct s_myStruct_Set = new MyStruct(); + public static MyStruct s_myStruct_GetAndSet = new MyStruct(); + public enum ShortEnum : short {} public enum IntEnum {} public enum LongEnum : long {} @@ -586,5 +664,45 @@ public static void SetValueDirect_GetValueDirectRoundDataTest(object value) Assert.Equal(value, result); } + + + public struct MyStruct_OnlyPrimitiveTypes + { + public int intField = 101; + + public MyStruct_OnlyPrimitiveTypes() + { + } + } + + public struct MyStruct + { + public static bool s_boolField = true; + public static bool s_boolField_Set = false; + public static int s_intField = 100; + public static int s_intField_Set = 0; + [ThreadStatic] public static int threadStatic_intField = 100; + [ThreadStatic] public static int threadStatic_intField_Set = 0; + public static string s_stringField = "static"; + public static string s_stringField_Set = null; + public static object s_objectField = new MyClass1(); + public static object s_objectField_Set = null; + public static readonly int[] s_rvaIntField = [1, 2]; + public unsafe static object intPtrForComparison = Pointer.Box((void*)42, typeof(int*)); + public unsafe static int* s_intPtr = (int*)43; + public unsafe static int* s_intPtr_Set = (int*)0; + public unsafe static object s_intPtrForComparison = Pointer.Box((void*)43, typeof(int*)); + public unsafe static delegate* s_fcnPtr = (delegate*)45; + public unsafe static delegate* s_fcnPtr_Set = (delegate*)0; + public bool boolField = true; + public int intField = 101; + public object objectField = null; + public string stringField = "non static"; + public const int s_constIntField = 102; + public unsafe int* intPtr = (int*)42; + public unsafe delegate* fcnPtr = (delegate*)44; + + public MyStruct() { } + } } } From 4c4236a9ada1cacec461002a683024fa75bc98b3 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 31 Jan 2024 15:45:30 -0600 Subject: [PATCH 2/9] Fix merge issue --- src/libraries/System.Private.CoreLib/src/Resources/Strings.resx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 3b2c2210fa6114..2d2957f9263f05 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -4297,6 +4297,7 @@ Cannot set initonly static field '{0}' after type '{1}' is initialized. + This AssemblyBuilder instance doesn't support saving. Use AssemblyBuilder.DefinePersistedAssembly to create an AssemblyBuilder instance that supports saving. From e2507de011283740f425940f8f2e23cbe3c39d90 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 31 Jan 2024 16:10:47 -0600 Subject: [PATCH 3/9] Fix mono build --- .../src/System.Private.CoreLib.Shared.projitems | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index cd5a43afa0bbcc..7b2c1f90c7aae0 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -701,7 +701,7 @@ - + From 77ba6ea3d90d07c3168b50618db1eea994e89475 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 1 Feb 2024 14:06:14 -0600 Subject: [PATCH 4/9] Add [ActiveIssue]s; use slow path for [ThreadStatic] --- src/coreclr/vm/reflectioninvocation.cpp | 16 ++--- .../src/System/Reflection/FieldAccessor.cs | 2 +- .../System.Reflection.Tests/FieldInfoTests.cs | 65 +++++++++++++++---- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index c5d81ee538e4c4..a21d59e1d1fdfd 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1239,7 +1239,7 @@ FCIMPL1(FC_BOOL_RET, RuntimeFieldHandle::IsFastPathSupported, ReflectFieldObject FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); FieldDesc* pFieldDesc = refField->GetField(); - return pFieldDesc->IsEnCNew() ? FALSE : TRUE; + return pFieldDesc->IsThreadStatic() || pFieldDesc->IsEnCNew() ? FALSE : TRUE; } FCIMPLEND @@ -1281,19 +1281,15 @@ FCIMPL2(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pF // IsFastPathSupported needs to checked before calling this method. _ASSERTE(!pFieldDesc->IsEnCNew()); + _ASSERTE(!refField->GetField()->IsThreadStatic()); GCPROTECT_BEGIN(refField); - if (refField->GetField()->IsThreadStatic()) { - ret = Thread::GetStaticFieldAddress(pFieldDesc); - } - else { - PTR_BYTE base = 0; - if (!pFieldDesc->IsRVA()) // For RVA the base is ignored. - base = pFieldDesc->GetBase(); + PTR_BYTE base = 0; + if (!pFieldDesc->IsRVA()) // For RVA the base is ignored. + base = pFieldDesc->GetBase(); - ret = pFieldDesc->GetStaticAddressHandle(base); - } + ret = pFieldDesc->GetStaticAddressHandle(base); *isBoxed = ((pFieldDesc->GetFieldType() == ELEMENT_TYPE_VALUETYPE && !pFieldDesc->IsRVA())) ? TRUE : FALSE; diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index 970331ce8e023c..eccf0880adf4ef 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -45,7 +45,7 @@ internal FieldAccessor(FieldInfo fieldInfo) } else if (!RuntimeFieldHandle.IsFastPathSupported(_fieldInfo)) { - // Currently this is only true for newly added fields from EnC. + // Currently this is true for [ThreadStatic] cases and for fields added from EnC. _addressOrOffset = default; _fieldAccessType = FieldAccessorType.SlowPath; } diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index 0308c3e6baff1c..0d10c50d95448c 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -74,7 +74,6 @@ public static IEnumerable GetValue_TestData() yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_constIntField), null, 102 }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_stringField), null, "static" }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_objectField), null, MyStruct.s_objectField }; - yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_fcnPtr), null, (IntPtr)45 }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intPtr), null, MyStruct.s_intPtrForComparison }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), new MyStruct(), MyStruct.s_rvaIntField }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.threadStatic_intField), null, 100 }; @@ -82,8 +81,6 @@ public static IEnumerable GetValue_TestData() yield return new object[] { typeof(MyStruct), nameof(MyStruct.stringField), new MyStruct(), "non static" }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.intField), new MyStruct(), 101 }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.intPtr), new MyStruct(), MyStruct.intPtrForComparison }; - yield return new object[] { typeof(MyStruct), nameof(MyStruct.fcnPtr), new MyStruct(), (IntPtr)44 }; - yield return new object[] { typeof(MyStruct_OnlyPrimitiveTypes), nameof(MyStruct_OnlyPrimitiveTypes.intField), new MyStruct_OnlyPrimitiveTypes(), 101 }; } @@ -95,6 +92,22 @@ public void GetValue(Type type, string name, object obj, object expected) Assert.Equal(expected, fieldInfo.GetValue(obj)); } + public static IEnumerable GetValue_TestData_WithFunctionPointers() + { + yield return new object[] { typeof(MyStructWithFunctionPointers), nameof(MyStructWithFunctionPointers.s_fcnPtr), null, (IntPtr)45 }; + yield return new object[] { typeof(MyStructWithFunctionPointers), nameof(MyStructWithFunctionPointers.fcnPtr), new MyStructWithFunctionPointers(), (IntPtr)44 }; + } + + [Theory] + [ActiveIssue("https://github.com/dotnet/runtime/issues/97830", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/97833", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))] + [MemberData(nameof(GetValue_TestData_WithFunctionPointers))] + public void GetValueWithFunctionPointers(Type type, string name, object obj, object expected) + { + FieldInfo fieldInfo = GetField(type, name); + Assert.Equal(expected, fieldInfo.GetValue(obj)); + } + [Fact] public void GetAndSetValueTypeFromStatic() { @@ -148,7 +161,6 @@ public static IEnumerable SetValue_TestData() yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_stringField_Set), null, "new", "new" }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_objectField_Set), null, MyStruct.s_objectField, MyStruct.s_objectField }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intPtr_Set), null, MyStruct.s_intPtrForComparison, MyStruct.s_intPtrForComparison }; - yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_fcnPtr_Set), null, (IntPtr)201, (IntPtr)201 }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.threadStatic_intField_Set), null, 100, 100 }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.boolField), new MyStruct(), true, true }; @@ -157,7 +169,6 @@ public static IEnumerable SetValue_TestData() yield return new object[] { typeof(MyStruct), nameof(MyStruct.stringField), new MyStruct(), "new", "new" }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.objectField), new MyStruct(), MyStruct.s_objectField, MyStruct.s_objectField }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.intPtr), new MyStruct(), MyStruct.s_intPtrForComparison, MyStruct.s_intPtrForComparison }; - yield return new object[] { typeof(MyStruct), nameof(MyStruct.fcnPtr), new MyStruct(), (IntPtr)200, (IntPtr)200 }; } [Theory] @@ -177,6 +188,31 @@ public void SetValue(Type type, string name, object obj, object value, object ex } } + public static IEnumerable SetValue_TestData_FunctionPointers() + { + yield return new object[] { typeof(MyStructWithFunctionPointers), nameof(MyStructWithFunctionPointers.s_fcnPtr_Set), null, (IntPtr)201, (IntPtr)201 }; + yield return new object[] { typeof(MyStructWithFunctionPointers), nameof(MyStructWithFunctionPointers.fcnPtr), new MyStructWithFunctionPointers(), (IntPtr)200, (IntPtr)200 }; + } + + [Theory] + [ActiveIssue("https://github.com/dotnet/runtime/issues/97830", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/97833", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))] + [MemberData(nameof(SetValue_TestData_FunctionPointers))] + public void SetValueWithFunctionPointers(Type type, string name, object obj, object value, object expected) + { + FieldInfo fieldInfo = GetField(type, name); + object original = fieldInfo.GetValue(obj); + try + { + fieldInfo.SetValue(obj, value); + Assert.Equal(expected, fieldInfo.GetValue(obj)); + } + finally + { + fieldInfo.SetValue(obj, original); + } + } + public static IEnumerable SetValue_Invalid_TestData() { yield return new object[] { typeof(FieldInfoTests), nameof(stringField), null, "new", typeof(TargetException) }; @@ -187,6 +223,7 @@ public static IEnumerable SetValue_Invalid_TestData() } [Theory] + [ActiveIssue("https://github.com/dotnet/runtime/issues/97829", TestRuntimes.Mono)] [MemberData(nameof(SetValue_Invalid_TestData))] public void SetValue_Invalid(Type type, string name, object obj, object value, Type exceptionType) { @@ -546,9 +583,9 @@ private static FieldInfo GetField(Type type, string name) public static MyStruct s_myStruct_Set = new MyStruct(); public static MyStruct s_myStruct_GetAndSet = new MyStruct(); - public enum ShortEnum : short {} - public enum IntEnum {} - public enum LongEnum : long {} + public enum ShortEnum : short { } + public enum IntEnum { } + public enum LongEnum : long { } public ShortEnum shortEnumField; public IntEnum intEnumField; public LongEnum longEnumField; @@ -692,17 +729,23 @@ public struct MyStruct public unsafe static int* s_intPtr = (int*)43; public unsafe static int* s_intPtr_Set = (int*)0; public unsafe static object s_intPtrForComparison = Pointer.Box((void*)43, typeof(int*)); - public unsafe static delegate* s_fcnPtr = (delegate*)45; - public unsafe static delegate* s_fcnPtr_Set = (delegate*)0; public bool boolField = true; public int intField = 101; public object objectField = null; public string stringField = "non static"; public const int s_constIntField = 102; public unsafe int* intPtr = (int*)42; - public unsafe delegate* fcnPtr = (delegate*)44; public MyStruct() { } } + + public struct MyStructWithFunctionPointers + { + public unsafe static delegate* s_fcnPtr = (delegate*)45; + public unsafe static delegate* s_fcnPtr_Set = (delegate*)0; + public unsafe delegate* fcnPtr = (delegate*)44; + + public MyStructWithFunctionPointers() { } + } } } From 177fc2a41720c5205b777ab79ace3dc66e5c1f09 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 1 Feb 2024 16:57:59 -0600 Subject: [PATCH 5/9] Fix issue with module --- .../src/System/Reflection/FieldAccessor.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index eccf0880adf4ef..ede94a917e5ca5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -16,6 +16,7 @@ internal sealed class FieldAccessor private readonly unsafe MethodTable* _methodTable; private readonly FieldAccessorType _fieldAccessType; private readonly PrimitiveFieldSize _primitiveFieldSize; + internal FieldAccessor(FieldInfo fieldInfo) { _fieldInfo = (RtFieldInfo)fieldInfo; @@ -29,6 +30,7 @@ internal FieldAccessor(FieldInfo fieldInfo) else if (!declaringType.DomainInitialized) { InvokeClassConstructor(); + declaringType.DomainInitialized = true; } // Cached the method table for performance. @@ -38,7 +40,7 @@ internal FieldAccessor(FieldInfo fieldInfo) } // Initialize for the type of field. - if (declaringType is null || declaringType.ContainsGenericParameters) + if (declaringType is not null && declaringType.ContainsGenericParameters) { _addressOrOffset = default; _fieldAccessType = FieldAccessorType.NoInvoke; @@ -341,19 +343,24 @@ internal void VerifyTarget(object? target) } else { - ThrowHelperArgumentException(target); + ThrowHelperArgumentException(target, _fieldInfo); } } } private static void ThrowHelperTargetException() => throw new TargetException(SR.RFLCT_Targ_StatFldReqTarg); - private void ThrowHelperArgumentException(object target) => throw new ArgumentException( - SR.Format(SR.Arg_FieldDeclTarget, _fieldInfo.Name, _fieldInfo.DeclaringType, target.GetType())); - private static void ThrowHelperFieldAccessException(string fieldName, string? declaringTypeName) => throw new FieldAccessException(SR.Format(SR.RFLCT_CannotSetInitonlyStaticField, fieldName, declaringTypeName)); + + private static void ThrowHelperArgumentException(object target, FieldInfo fieldInfo) => + throw new ArgumentException(SR.Format(SR.Arg_FieldDeclTarget, fieldInfo.Name, fieldInfo.DeclaringType, target.GetType())); + + private static void ThrowHelperFieldAccessException(string fieldName, string? declaringTypeName) => + throw new FieldAccessException(SR.Format(SR.RFLCT_CannotSetInitonlyStaticField, fieldName, declaringTypeName)); internal void VerifyInitOnly() { Debug.Assert(IsStatic()); + Debug.Assert(((RuntimeType)_fieldInfo.DeclaringType!).DomainInitialized); + if ((_fieldInfo.Attributes & FieldAttributes.InitOnly) == FieldAttributes.InitOnly) { ThrowHelperFieldAccessException(_fieldInfo.Name, _fieldInfo.DeclaringType!.FullName); From d0859df13ef3db2d8510602bff6583a61a864f15 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Sun, 4 Feb 2024 11:57:26 -0600 Subject: [PATCH 6/9] Make fields from collectible types use slow path; other misc --- .../src/System/RuntimeHandles.cs | 4 +- src/coreclr/vm/ecalllist.h | 2 +- src/coreclr/vm/field.h | 8 ++++ src/coreclr/vm/reflectioninvocation.cpp | 46 +++++++++++++------ src/coreclr/vm/runtimehandles.h | 4 +- .../src/System/Reflection/FieldAccessor.cs | 41 +++++++---------- .../System.Reflection.Tests/FieldInfoTests.cs | 9 ++-- 7 files changed, 67 insertions(+), 47 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 32b68bb68455a1..8868894d592516 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -1194,10 +1194,10 @@ internal static RuntimeType GetApproxDeclaringType(IRuntimeFieldInfo field) internal static extern bool IsFastPathSupported(RtFieldInfo field); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern int GetInstanceFieldAddress(RtFieldInfo field); + internal static extern int GetInstanceFieldOffset(RtFieldInfo field); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern IntPtr GetStaticFieldAddress(RtFieldInfo field, ref bool isBoxed); + internal static extern IntPtr GetStaticFieldAddress(RtFieldInfo field, out bool isBoxed); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern int GetToken(RtFieldInfo field); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index c3d249891e4914..cbee36613dcccf 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -243,7 +243,7 @@ FCFuncStart(gCOMFieldHandleNewFuncs) FCFuncElement("AcquiresContextFromThis", RuntimeFieldHandle::AcquiresContextFromThis) FCFuncElement("GetLoaderAllocator", RuntimeFieldHandle::GetLoaderAllocator) FCFuncElement("IsFastPathSupported", RuntimeFieldHandle::IsFastPathSupported) - FCFuncElement("GetInstanceFieldAddress", RuntimeFieldHandle::GetInstanceFieldAddress) + FCFuncElement("GetInstanceFieldOffset", RuntimeFieldHandle::GetInstanceFieldOffset) FCFuncElement("GetStaticFieldAddress", RuntimeFieldHandle::GetStaticFieldAddress) FCFuncEnd() diff --git a/src/coreclr/vm/field.h b/src/coreclr/vm/field.h index e2324787febaf5..1bc8885c9faabf 100644 --- a/src/coreclr/vm/field.h +++ b/src/coreclr/vm/field.h @@ -285,6 +285,14 @@ class FieldDesc SetOffset(FIELD_OFFSET_NEW_ENC); } + BOOL IsCollectible() + { + LIMITED_METHOD_DAC_CONTRACT; + + LoaderAllocator *pLoaderAllocatorOfMethod = GetApproxEnclosingMethodTable()->GetLoaderAllocator(); + return pLoaderAllocatorOfMethod->IsCollectible(); + } + // Was this field added by EnC? // If this is true, then this object is an instance of EnCFieldDesc BOOL IsEnCNew() diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index a21d59e1d1fdfd..549670715bf42d 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1227,6 +1227,21 @@ lExit: ; } FCIMPLEND +static FC_BOOL_RET IsFastPathSupportedHelper(FieldDesc* pFieldDesc) +{ + CONTRACTL { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + PRECONDITION(CheckPointer(pFieldDesc)); + } + CONTRACTL_END; + + return pFieldDesc->IsThreadStatic() || + pFieldDesc->IsEnCNew() || + pFieldDesc->IsCollectible() ? FALSE : TRUE; +} + FCIMPL1(FC_BOOL_RET, RuntimeFieldHandle::IsFastPathSupported, ReflectFieldObject *pFieldUNSAFE) { CONTRACTL { @@ -1239,11 +1254,11 @@ FCIMPL1(FC_BOOL_RET, RuntimeFieldHandle::IsFastPathSupported, ReflectFieldObject FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); FieldDesc* pFieldDesc = refField->GetField(); - return pFieldDesc->IsThreadStatic() || pFieldDesc->IsEnCNew() ? FALSE : TRUE; + return IsFastPathSupportedHelper(pFieldDesc); } FCIMPLEND -FCIMPL1(INT32, RuntimeFieldHandle::GetInstanceFieldAddress, ReflectFieldObject *pFieldUNSAFE) +FCIMPL1(INT32, RuntimeFieldHandle::GetInstanceFieldOffset, ReflectFieldObject *pFieldUNSAFE) { CONTRACTL { FCALL_CHECK; @@ -1258,7 +1273,7 @@ FCIMPL1(INT32, RuntimeFieldHandle::GetInstanceFieldAddress, ReflectFieldObject * FieldDesc* pFieldDesc = refField->GetField(); // IsFastPathSupported needs to checked before calling this method. - _ASSERTE(!pFieldDesc->IsEnCNew()); + _ASSERTE(IsFastPathSupportedHelper(pFieldDesc)); return pFieldDesc->GetOffset(); } @@ -1276,25 +1291,26 @@ FCIMPL2(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pF if (refField == NULL) FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); - void *ret = NULL; FieldDesc* pFieldDesc = refField->GetField(); // IsFastPathSupported needs to checked before calling this method. - _ASSERTE(!pFieldDesc->IsEnCNew()); - _ASSERTE(!refField->GetField()->IsThreadStatic()); - - GCPROTECT_BEGIN(refField); + _ASSERTE(IsFastPathSupportedHelper(pFieldDesc)); PTR_BYTE base = 0; - if (!pFieldDesc->IsRVA()) // For RVA the base is ignored. + if (pFieldDesc->IsRVA()) + { + // For RVA the base is ignored and offset is used. + *isBoxed = FALSE; + } + else + { base = pFieldDesc->GetBase(); + CorElementType fieldType = pFieldDesc->GetFieldType(); + // Non-primitive value types need to be unboxed to get the base address. + *isBoxed = (pFieldDesc->GetFieldType() == ELEMENT_TYPE_VALUETYPE) ? TRUE : FALSE; + } - ret = pFieldDesc->GetStaticAddressHandle(base); - - *isBoxed = ((pFieldDesc->GetFieldType() == ELEMENT_TYPE_VALUETYPE && !pFieldDesc->IsRVA())) ? TRUE : FALSE; - - GCPROTECT_END(); - return ret; + return pFieldDesc->GetStaticAddressHandle(base); } FCIMPLEND diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index c950e26538efee..1c84683a023185 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -297,8 +297,8 @@ class RuntimeFieldHandle { static FCDECL4(Object*, GetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, ReflectClassBaseObject *pDeclaringType); static FCDECL5(void, SetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, Object *valueUNSAFE, ReflectClassBaseObject *pContextType); static FCDECL1(FC_BOOL_RET, IsFastPathSupported, ReflectFieldObject *pField); - static FCDECL1(INT32, GetInstanceFieldAddress, ReflectFieldObject *pField); - static FCDECL2(PVOID, GetStaticFieldAddress, ReflectFieldObject *pField, CLR_BOOL* isBoxed); + static FCDECL1(INT32, GetInstanceFieldOffset, ReflectFieldObject *pField); + static FCDECL2(void*, GetStaticFieldAddress, ReflectFieldObject *pField, CLR_BOOL *isBoxed); static FCDECL1(StringObject*, GetName, ReflectFieldObject *pFieldUNSAFE); static FCDECL1(LPCUTF8, GetUtf8Name, FieldDesc *pField); diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index ede94a917e5ca5..1c9ed4e05f4d4b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -29,7 +29,7 @@ internal FieldAccessor(FieldInfo fieldInfo) } else if (!declaringType.DomainInitialized) { - InvokeClassConstructor(); + InvokeClassConstructor(fieldInfo); declaringType.DomainInitialized = true; } @@ -42,13 +42,11 @@ internal FieldAccessor(FieldInfo fieldInfo) // Initialize for the type of field. if (declaringType is not null && declaringType.ContainsGenericParameters) { - _addressOrOffset = default; _fieldAccessType = FieldAccessorType.NoInvoke; } else if (!RuntimeFieldHandle.IsFastPathSupported(_fieldInfo)) { // Currently this is true for [ThreadStatic] cases and for fields added from EnC. - _addressOrOffset = default; _fieldAccessType = FieldAccessorType.SlowPath; } else @@ -57,22 +55,21 @@ internal FieldAccessor(FieldInfo fieldInfo) if (fieldInfo.IsStatic) { - bool isBoxed = false; - _addressOrOffset = RuntimeFieldHandle.GetStaticFieldAddress(_fieldInfo, ref isBoxed); + _addressOrOffset = RuntimeFieldHandle.GetStaticFieldAddress(_fieldInfo, out bool isBoxed); if (fieldType.IsValueType) { - // The runtime stores non-primitive value types as boxed. + // The runtime stores non-primitive value types as a boxed value. _fieldAccessType = isBoxed ? FieldAccessorType.StaticValueTypeBoxed : FieldAccessorType.StaticValueType; } else if (fieldType.IsPointer) { - Debug.Assert(isBoxed == false); + Debug.Assert(!isBoxed); _fieldAccessType = FieldAccessorType.StaticPointerType; } else if (fieldType.IsFunctionPointer) { - Debug.Assert(isBoxed == false); + Debug.Assert(!isBoxed); _fieldAccessType = FieldAccessorType.StaticValueType; unsafe { @@ -81,13 +78,13 @@ internal FieldAccessor(FieldInfo fieldInfo) } else { - Debug.Assert(isBoxed == false); + Debug.Assert(!isBoxed); _fieldAccessType = FieldAccessorType.StaticReferenceType; } } else { - _addressOrOffset = RuntimeFieldHandle.GetInstanceFieldAddress(_fieldInfo); + _addressOrOffset = RuntimeFieldHandle.GetInstanceFieldOffset(_fieldInfo); if (fieldType.IsValueType) { @@ -135,7 +132,6 @@ internal FieldAccessor(FieldInfo fieldInfo) ret = RuntimeHelpers.Box( _methodTable, ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)); - break; case FieldAccessorType.InstancePointerType: @@ -144,7 +140,6 @@ internal FieldAccessor(FieldInfo fieldInfo) ret = Pointer.Box( (void*)Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), _fieldInfo.FieldType); - break; case FieldAccessorType.StaticReferenceType: @@ -160,13 +155,11 @@ internal FieldAccessor(FieldInfo fieldInfo) ret = RuntimeHelpers.Box( _methodTable, ref Unsafe.As(ref *(IntPtr*)_addressOrOffset).GetRawData()); - break; case FieldAccessorType.StaticPointerType: ret = Pointer.Box((void*)Unsafe.As( ref Unsafe.AsRef(_addressOrOffset.ToPointer())), _fieldInfo.FieldType); - break; case FieldAccessorType.SlowPath: @@ -215,12 +208,6 @@ public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder } return; - case FieldAccessorType.NoInvoke: - if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) - throw new InvalidOperationException(SR.Arg_UnboundGenField); - - throw new FieldAccessException(); - case FieldAccessorType.InstanceValueType: VerifyTarget(obj); CheckValue(); @@ -302,6 +289,12 @@ ref Unsafe.AsRef(_addressOrOffset.ToPointer()), return; } + + case FieldAccessorType.NoInvoke: + if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) + throw new InvalidOperationException(SR.Arg_UnboundGenField); + + throw new FieldAccessException(); } if (!IsStatic()) @@ -359,19 +352,19 @@ private static void ThrowHelperFieldAccessException(string fieldName, string? de internal void VerifyInitOnly() { Debug.Assert(IsStatic()); - Debug.Assert(((RuntimeType)_fieldInfo.DeclaringType!).DomainInitialized); + Debug.Assert(_fieldInfo.DeclaringType == null || ((RuntimeType)_fieldInfo.DeclaringType).DomainInitialized); if ((_fieldInfo.Attributes & FieldAttributes.InitOnly) == FieldAttributes.InitOnly) { - ThrowHelperFieldAccessException(_fieldInfo.Name, _fieldInfo.DeclaringType!.FullName); + ThrowHelperFieldAccessException(_fieldInfo.Name, _fieldInfo.DeclaringType?.FullName); } } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2059:RunClassConstructor", Justification = "This represents the static constructor, so if this object was created, the static constructor exists.")] - private void InvokeClassConstructor() + private static void InvokeClassConstructor(FieldInfo fieldInfo) { - RuntimeHelpers.RunClassConstructor(_fieldInfo.DeclaringType!.TypeHandle); + RuntimeHelpers.RunClassConstructor(fieldInfo.DeclaringType!.TypeHandle); } private static void InvokeModuleConstructor(Module module) diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index 0d10c50d95448c..a91834ca9f022d 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -75,7 +75,7 @@ public static IEnumerable GetValue_TestData() yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_stringField), null, "static" }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_objectField), null, MyStruct.s_objectField }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_intPtr), null, MyStruct.s_intPtrForComparison }; - yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), new MyStruct(), MyStruct.s_rvaIntField }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), null, new int[] { 1, 2, 3 } }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.threadStatic_intField), null, 100 }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.stringField), new MyStruct(), "non static" }; @@ -219,7 +219,7 @@ public static IEnumerable SetValue_Invalid_TestData() yield return new object[] { typeof(FieldInfoTests), nameof(stringField), new object(), "new", typeof(ArgumentException) }; yield return new object[] { typeof(FieldInfoTests), nameof(stringField), new FieldInfoTests(), 100, typeof(ArgumentException) }; yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_constIntField), null, 100, typeof(FieldAccessException) }; - yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), null, new int[] { 3, 4 }, typeof(FieldAccessException) }; + yield return new object[] { typeof(MyStruct), nameof(MyStruct.s_rvaIntField), null, new int[] { 3, 4, 5 }, typeof(FieldAccessException) }; } [Theory] @@ -724,7 +724,10 @@ public struct MyStruct public static string s_stringField_Set = null; public static object s_objectField = new MyClass1(); public static object s_objectField_Set = null; - public static readonly int[] s_rvaIntField = [1, 2]; + + // This does not report FieldAttributes.HasFieldRVA since Roslyn wraps thd .data with generated helper class. + public static readonly int[] s_rvaIntField = [1, 2, 3]; + public unsafe static object intPtrForComparison = Pointer.Box((void*)42, typeof(int*)); public unsafe static int* s_intPtr = (int*)43; public unsafe static int* s_intPtr_Set = (int*)0; From ff55de20c83340c400b84a89645a311ded2dab0c Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 5 Feb 2024 09:33:26 -0600 Subject: [PATCH 7/9] Fix cctor check --- .../src/System/RuntimeHandles.cs | 4 +- src/coreclr/vm/reflectioninvocation.cpp | 20 ++-- src/coreclr/vm/runtimehandles.h | 4 +- .../src/System/Reflection/FieldAccessor.cs | 95 +++++++++++-------- 4 files changed, 65 insertions(+), 58 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 8868894d592516..6e6217bd33b5ac 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -1203,13 +1203,13 @@ internal static RuntimeType GetApproxDeclaringType(IRuntimeFieldInfo field) internal static extern int GetToken(RtFieldInfo field); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType); + internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object? GetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, RuntimeType? contextType); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType); + internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void SetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, object? value, RuntimeType? contextType); diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index 549670715bf42d..c88c094e280dd9 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -25,7 +25,7 @@ #include "dbginterface.h" #include "argdestination.h" -FCIMPL4(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE) { +FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { CONTRACTL { FCALL_CHECK; } @@ -52,12 +52,9 @@ FCIMPL4(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, OBJECTREF rv = NULL; // not protected - // The caller should have ran the cctor already. - CLR_BOOL domainInitialized = TRUE; - HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); // There can be no GC after this until the Object is returned. - rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, &domainInitialized); + rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, pDomainInitialized); HELPER_METHOD_FRAME_END(); return OBJECTREFToObject(rv); @@ -161,7 +158,7 @@ FCIMPL2(Object*, ReflectionInvocation::AllocateValueType, ReflectClassBaseObject } FCIMPLEND -FCIMPL5(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE) { +FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { CONTRACTL { FCALL_CHECK; } @@ -191,12 +188,9 @@ FCIMPL5(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob FieldDesc* pFieldDesc = gc.refField->GetField(); - // The caller should have ran the cctor already. - CLR_BOOL domainInitialized = TRUE; - HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); - InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, &domainInitialized); + InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized); HELPER_METHOD_FRAME_END(); } @@ -1304,13 +1298,15 @@ FCIMPL2(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pF } else { - base = pFieldDesc->GetBase(); CorElementType fieldType = pFieldDesc->GetFieldType(); + // Non-primitive value types need to be unboxed to get the base address. *isBoxed = (pFieldDesc->GetFieldType() == ELEMENT_TYPE_VALUETYPE) ? TRUE : FALSE; + + base = pFieldDesc->GetBase(); } - return pFieldDesc->GetStaticAddressHandle(base); + return PTR_VOID(base + pFieldDesc->GetOffset()); } FCIMPLEND diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 1c84683a023185..c27b6c6f2aaae9 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -292,8 +292,8 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_Destroy(MethodDesc * pMethod); class RuntimeFieldHandle { public: - static FCDECL4(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType); - static FCDECL5(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType); + static FCDECL5(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); + static FCDECL6(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); static FCDECL4(Object*, GetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, ReflectClassBaseObject *pDeclaringType); static FCDECL5(void, SetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, Object *valueUNSAFE, ReflectClassBaseObject *pContextType); static FCDECL1(FC_BOOL_RET, IsFastPathSupported, ReflectFieldObject *pField); diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index 1c9ed4e05f4d4b..bc8c9a986af26a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -20,18 +20,8 @@ internal sealed class FieldAccessor internal FieldAccessor(FieldInfo fieldInfo) { _fieldInfo = (RtFieldInfo)fieldInfo; - RuntimeType? declaringType = (RuntimeType?)_fieldInfo.DeclaringType; - // Call the class constructor if not already. - if (declaringType is null) - { - InvokeModuleConstructor(fieldInfo.Module); - } - else if (!declaringType.DomainInitialized) - { - InvokeClassConstructor(fieldInfo); - declaringType.DomainInitialized = true; - } + InitializeClass(); // Cached the method table for performance. unsafe @@ -40,13 +30,14 @@ internal FieldAccessor(FieldInfo fieldInfo) } // Initialize for the type of field. + RuntimeType? declaringType = (RuntimeType?)_fieldInfo.DeclaringType; if (declaringType is not null && declaringType.ContainsGenericParameters) { _fieldAccessType = FieldAccessorType.NoInvoke; } else if (!RuntimeFieldHandle.IsFastPathSupported(_fieldInfo)) { - // Currently this is true for [ThreadStatic] cases and for fields added from EnC. + // Currently this is true for [ThreadStatic] cases, for fields added from EnC, and for fields on unloadable types. _fieldAccessType = FieldAccessorType.SlowPath; } else @@ -168,7 +159,9 @@ internal FieldAccessor(FieldInfo fieldInfo) VerifyTarget(obj); } - ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType); + bool domainInitialized = _fieldInfo.m_declaringType.DomainInitialized; + ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType, ref domainInitialized); + _fieldInfo.m_declaringType.DomainInitialized = domainInitialized; break; case FieldAccessorType.NoInvoke: @@ -196,7 +189,7 @@ public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder VerifyTarget(obj); CheckValue(); Debug.Assert(obj != null); - Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)) = value; + Volatile.Write(ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), value); return; case FieldAccessorType.StaticReferenceType: @@ -204,7 +197,7 @@ public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder CheckValue(); unsafe { - Unsafe.As(ref *(IntPtr*)_addressOrOffset) = value; + Volatile.Write(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset), value); } return; @@ -218,76 +211,67 @@ public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder Volatile.Write( ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset), value!.GetRawData()); - return; case PrimitiveFieldSize.Size2: Volatile.Write( ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), Unsafe.As(ref value!.GetRawData())); - return; case PrimitiveFieldSize.Size4: Volatile.Write( ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), Unsafe.As(ref value!.GetRawData())); - return; case PrimitiveFieldSize.Size8: Volatile.Write( ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), Unsafe.As(ref value!.GetRawData())); - - return; - - default: - RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType); return; } - + break; case FieldAccessorType.StaticValueType: - VerifyInitOnly(); - CheckValue(); unsafe { switch (_primitiveFieldSize) { case PrimitiveFieldSize.Size1: + VerifyInitOnly(); + CheckValue(); Volatile.Write( ref Unsafe.AsRef(_addressOrOffset.ToPointer()), value!.GetRawData()); - - break; + return; case PrimitiveFieldSize.Size2: + VerifyInitOnly(); + CheckValue(); Volatile.Write( ref Unsafe.AsRef(_addressOrOffset.ToPointer()), Unsafe.As(ref value!.GetRawData())); - - break; + return; case PrimitiveFieldSize.Size4: + VerifyInitOnly(); + CheckValue(); Volatile.Write( ref Unsafe.AsRef(_addressOrOffset.ToPointer()), Unsafe.As(ref value!.GetRawData())); - - break; + return; case PrimitiveFieldSize.Size8: + VerifyInitOnly(); + CheckValue(); Volatile.Write( ref Unsafe.AsRef(_addressOrOffset.ToPointer()), Unsafe.As(ref value!.GetRawData())); - break; - - case PrimitiveFieldSize.NotPrimitive: - RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType); - break; + return; } - return; + break; } case FieldAccessorType.NoInvoke: @@ -297,13 +281,19 @@ ref Unsafe.AsRef(_addressOrOffset.ToPointer()), throw new FieldAccessException(); } - if (!IsStatic()) + if (IsStatic()) + { + VerifyInitOnly(); + } + else { VerifyTarget(obj); } CheckValue(); - RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType); + bool domainInitialized = _fieldInfo.m_declaringType.DomainInitialized; + RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType, ref domainInitialized); + _fieldInfo.m_declaringType.DomainInitialized = domainInitialized; void CheckValue() { @@ -321,6 +311,28 @@ void CheckValue() } } + private void InitializeClass() + { + RtFieldInfo fieldInfo = _fieldInfo; + RuntimeType declaringType = _fieldInfo.m_declaringType; + + // Call the class constructor if not already. + if (!declaringType.DomainInitialized) + { + if (_fieldInfo.DeclaringType is null) + { + InvokeModuleConstructor(fieldInfo.Module); + } + else + { + InvokeClassConstructor(fieldInfo); + } + + // A constructor is allowed to set init-only fields, so set this after. + declaringType.DomainInitialized = true; + } + } + private bool IsStatic() => (_fieldInfo.Attributes & FieldAttributes.Static) == FieldAttributes.Static; [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -352,9 +364,8 @@ private static void ThrowHelperFieldAccessException(string fieldName, string? de internal void VerifyInitOnly() { Debug.Assert(IsStatic()); - Debug.Assert(_fieldInfo.DeclaringType == null || ((RuntimeType)_fieldInfo.DeclaringType).DomainInitialized); - if ((_fieldInfo.Attributes & FieldAttributes.InitOnly) == FieldAttributes.InitOnly) + if ((_fieldInfo.Attributes & FieldAttributes.InitOnly) == FieldAttributes.InitOnly && _fieldInfo.m_declaringType.DomainInitialized) { ThrowHelperFieldAccessException(_fieldInfo.Name, _fieldInfo.DeclaringType?.FullName); } From 6b0f7fabcd97edd568f8ce3b910a241fc74cfb6e Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 5 Feb 2024 17:28:47 -0600 Subject: [PATCH 8/9] Combine the 2 enums; remove [ActiveIssue] for mono fix --- .../src/System/RuntimeHandles.cs | 4 +- src/coreclr/vm/invokeutil.cpp | 18 +- src/coreclr/vm/invokeutil.h | 4 +- src/coreclr/vm/reflectioninvocation.cpp | 19 +- src/coreclr/vm/runtimehandles.h | 4 +- .../src/System/Reflection/FieldAccessor.cs | 470 ++++++++++-------- .../System.Reflection.Tests/FieldInfoTests.cs | 2 - 7 files changed, 275 insertions(+), 246 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 6e6217bd33b5ac..23950cd93e2868 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -1203,13 +1203,13 @@ internal static RuntimeType GetApproxDeclaringType(IRuntimeFieldInfo field) internal static extern int GetToken(RtFieldInfo field); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); + internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType, bool domainInitialized); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object? GetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, RuntimeType? contextType); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); + internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType, bool domainInitialized); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void SetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, object? value, RuntimeType? contextType); diff --git a/src/coreclr/vm/invokeutil.cpp b/src/coreclr/vm/invokeutil.cpp index 055a8d0497ed10..8d3ffa0b73a9eb 100644 --- a/src/coreclr/vm/invokeutil.cpp +++ b/src/coreclr/vm/invokeutil.cpp @@ -741,14 +741,14 @@ void InvokeUtil::ValidateObjectTarget(FieldDesc *pField, TypeHandle enclosingTyp // SetValidField // Given an target object, a value object and a field this method will set the field -// on the target object. The field must be validate before calling this. +// on the target object. The field must be validated before calling this. void InvokeUtil::SetValidField(CorElementType fldType, TypeHandle fldTH, FieldDesc *pField, OBJECTREF *target, OBJECTREF *valueObj, TypeHandle declaringType, - CLR_BOOL *pDomainInitialized) { + CLR_BOOL domainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -786,19 +786,17 @@ void InvokeUtil::SetValidField(CorElementType fldType, pDeclMT = pField->GetModule()->GetGlobalMethodTable(); } - if (*pDomainInitialized == FALSE) + if (domainInitialized == FALSE) { EX_TRY { pDeclMT->EnsureInstanceActive(); pDeclMT->CheckRunClassInitThrowing(); - - *pDomainInitialized = TRUE; } EX_CATCH_THROWABLE(&Throwable); } #ifdef _DEBUG - else if (*pDomainInitialized == TRUE && !declaringType.IsNull()) + else if (!declaringType.IsNull()) CONSISTENCY_CHECK(declaringType.GetMethodTable()->CheckActivated()); #endif @@ -973,7 +971,7 @@ void InvokeUtil::SetValidField(CorElementType fldType, // GetFieldValue // This method will return an ARG_SLOT containing the value of the field. -OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL *pDomainInitialized) { +OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL domainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -1011,19 +1009,17 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ pDeclMT = pField->GetModule()->GetGlobalMethodTable(); } - if (*pDomainInitialized == FALSE) + if (domainInitialized == FALSE) { EX_TRY { pDeclMT->EnsureInstanceActive(); pDeclMT->CheckRunClassInitThrowing(); - - *pDomainInitialized = TRUE; } EX_CATCH_THROWABLE(&Throwable); } #ifdef _DEBUG - else if (*pDomainInitialized == TRUE && !declaringType.IsNull()) + else if (!declaringType.IsNull()) CONSISTENCY_CHECK(declaringType.GetMethodTable()->CheckActivated()); #endif diff --git a/src/coreclr/vm/invokeutil.h b/src/coreclr/vm/invokeutil.h index 0bd1577c7a19d3..cdadc381fc15e6 100644 --- a/src/coreclr/vm/invokeutil.h +++ b/src/coreclr/vm/invokeutil.h @@ -138,9 +138,9 @@ class InvokeUtil // SetValidField // Given an target object, a value object and a field this method will set the field // on the target object. The field must be validate before calling this. - static void SetValidField(CorElementType fldType, TypeHandle fldTH, FieldDesc* pField, OBJECTREF* target, OBJECTREF* value, TypeHandle declaringType, CLR_BOOL *pDomainInitialized); + static void SetValidField(CorElementType fldType, TypeHandle fldTH, FieldDesc* pField, OBJECTREF* target, OBJECTREF* value, TypeHandle declaringType, CLR_BOOL domainInitialized); - static OBJECTREF GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL *pDomainInitialized); + static OBJECTREF GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL domainInitialized); // ValidateObjectTarget // This method will validate the Object/Target relationship diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index c88c094e280dd9..08ccb07cf30b9f 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -25,7 +25,7 @@ #include "dbginterface.h" #include "argdestination.h" -FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { +FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL domainInitialized) { CONTRACTL { FCALL_CHECK; } @@ -54,7 +54,7 @@ FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); // There can be no GC after this until the Object is returned. - rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, pDomainInitialized); + rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, domainInitialized); HELPER_METHOD_FRAME_END(); return OBJECTREFToObject(rv); @@ -158,7 +158,7 @@ FCIMPL2(Object*, ReflectionInvocation::AllocateValueType, ReflectClassBaseObject } FCIMPLEND -FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { +FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL domainInitialized) { CONTRACTL { FCALL_CHECK; } @@ -190,7 +190,7 @@ FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); - InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized); + InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, domainInitialized); HELPER_METHOD_FRAME_END(); } @@ -914,7 +914,7 @@ FCIMPL1(ReflectMethodObject*, RuntimeMethodHandle::GetCurrentMethod, StackCrawlM } FCIMPLEND -static OBJECTREF DirectObjectFieldGet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, CLR_BOOL *pDomainInitialized) { +static OBJECTREF DirectObjectFieldGet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, CLR_BOOL domainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -932,7 +932,7 @@ static OBJECTREF DirectObjectFieldGet(FieldDesc *pField, TypeHandle fieldType, T } InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref); - refRet = InvokeUtil::GetFieldValue(pField, fieldType, &objref, enclosingType, pDomainInitialized); + refRet = InvokeUtil::GetFieldValue(pField, fieldType, &objref, enclosingType, domainInitialized); GCPROTECT_END(); return refRet; } @@ -1037,7 +1037,7 @@ lExit: ; } FCIMPLEND -static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, OBJECTREF *pValue, CLR_BOOL *pDomainInitialized) { +static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, OBJECTREF *pValue, CLR_BOOL domainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -1056,7 +1056,7 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa // Validate the target/fld type relationship InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref); - InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized); + InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, domainInitialized); GCPROTECT_END(); } @@ -1102,9 +1102,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA // 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()) { - DirectObjectFieldSet(pField, fieldType, TypeHandle(pEnclosingMT), pTarget, &gc.oValue, &domainInitialized); + DirectObjectFieldSet(pField, fieldType, TypeHandle(pEnclosingMT), pTarget, &gc.oValue, FALSE); goto lExit; } diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index c27b6c6f2aaae9..17140ee4a73968 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -292,8 +292,8 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_Destroy(MethodDesc * pMethod); class RuntimeFieldHandle { public: - static FCDECL5(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); - static FCDECL6(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); + static FCDECL5(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL domainInitialized); + static FCDECL6(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL domainInitialized); static FCDECL4(Object*, GetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, ReflectClassBaseObject *pDeclaringType); static FCDECL5(void, SetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, Object *valueUNSAFE, ReflectClassBaseObject *pContextType); static FCDECL1(FC_BOOL_RET, IsFastPathSupported, ReflectFieldObject *pField); diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index bc8c9a986af26a..95e1a0dfbf4660 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -15,7 +15,6 @@ internal sealed class FieldAccessor private readonly RtFieldInfo _fieldInfo; private readonly unsafe MethodTable* _methodTable; private readonly FieldAccessorType _fieldAccessType; - private readonly PrimitiveFieldSize _primitiveFieldSize; internal FieldAccessor(FieldInfo fieldInfo) { @@ -35,6 +34,10 @@ internal FieldAccessor(FieldInfo fieldInfo) { _fieldAccessType = FieldAccessorType.NoInvoke; } + else if (declaringType is not null && declaringType.IsNullableOfT) + { + _fieldAccessType = FieldAccessorType.NoInvoke; + } else if (!RuntimeFieldHandle.IsFastPathSupported(_fieldInfo)) { // Currently this is true for [ThreadStatic] cases, for fields added from EnC, and for fields on unloadable types. @@ -51,7 +54,14 @@ internal FieldAccessor(FieldInfo fieldInfo) if (fieldType.IsValueType) { // The runtime stores non-primitive value types as a boxed value. - _fieldAccessType = isBoxed ? FieldAccessorType.StaticValueTypeBoxed : FieldAccessorType.StaticValueType; + if (isBoxed) + { + _fieldAccessType = FieldAccessorType.StaticValueTypeBoxed; + } + else + { + _fieldAccessType = GetPrimitiveAccessorTypeForStatic(fieldType); + } } else if (fieldType.IsPointer) { @@ -61,7 +71,7 @@ internal FieldAccessor(FieldInfo fieldInfo) else if (fieldType.IsFunctionPointer) { Debug.Assert(!isBoxed); - _fieldAccessType = FieldAccessorType.StaticValueType; + _fieldAccessType = GetFunctionPointerAccessorTypeForStatic(); unsafe { _methodTable = (MethodTable*)typeof(IntPtr).TypeHandle.Value; @@ -79,7 +89,7 @@ internal FieldAccessor(FieldInfo fieldInfo) if (fieldType.IsValueType) { - _fieldAccessType = FieldAccessorType.InstanceValueType; + _fieldAccessType = GetPrimitiveAccessorTypeForInstance(fieldType); } else if (fieldType.IsPointer) { @@ -87,7 +97,7 @@ internal FieldAccessor(FieldInfo fieldInfo) } else if (fieldType.IsFunctionPointer) { - _fieldAccessType = FieldAccessorType.InstanceValueType; + _fieldAccessType = GetFunctionPointerAccessorTypeForInstance(); unsafe { _methodTable = (MethodTable*)typeof(IntPtr).TypeHandle.Value; @@ -98,15 +108,11 @@ internal FieldAccessor(FieldInfo fieldInfo) _fieldAccessType = FieldAccessorType.InstanceReferenceType; } } - - _primitiveFieldSize = GetPrimitiveFieldSize(fieldType, _fieldAccessType); } } public object? GetValue(object? obj) { - object? ret = null; - unsafe { switch (_fieldAccessType) @@ -114,227 +120,200 @@ internal FieldAccessor(FieldInfo fieldInfo) case FieldAccessorType.InstanceReferenceType: VerifyTarget(obj); Debug.Assert(obj != null); - ret = Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)); - break; + return Volatile.Read(ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset))); case FieldAccessorType.InstanceValueType: + case FieldAccessorType.InstanceValueTypeSize1: + case FieldAccessorType.InstanceValueTypeSize2: + case FieldAccessorType.InstanceValueTypeSize4: + case FieldAccessorType.InstanceValueTypeSize8: VerifyTarget(obj); Debug.Assert(obj != null); - ret = RuntimeHelpers.Box( + return RuntimeHelpers.Box( _methodTable, ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)); - break; case FieldAccessorType.InstancePointerType: VerifyTarget(obj); Debug.Assert(obj != null); - ret = Pointer.Box( + return Pointer.Box( (void*)Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), _fieldInfo.FieldType); - break; case FieldAccessorType.StaticReferenceType: - ret = Volatile.Read(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset)); - break; + return Volatile.Read(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset)); case FieldAccessorType.StaticValueType: - ret = RuntimeHelpers.Box(_methodTable, ref Unsafe.AsRef(_addressOrOffset.ToPointer())); - break; + case FieldAccessorType.StaticValueTypeSize1: + case FieldAccessorType.StaticValueTypeSize2: + case FieldAccessorType.StaticValueTypeSize4: + case FieldAccessorType.StaticValueTypeSize8: + return RuntimeHelpers.Box(_methodTable, ref Unsafe.AsRef(_addressOrOffset.ToPointer())); case FieldAccessorType.StaticValueTypeBoxed: // Re-box the value. - ret = RuntimeHelpers.Box( + return RuntimeHelpers.Box( _methodTable, ref Unsafe.As(ref *(IntPtr*)_addressOrOffset).GetRawData()); - break; case FieldAccessorType.StaticPointerType: - ret = Pointer.Box((void*)Unsafe.As( + return Pointer.Box((void*)Unsafe.As( ref Unsafe.AsRef(_addressOrOffset.ToPointer())), _fieldInfo.FieldType); - break; - - case FieldAccessorType.SlowPath: - if (!IsStatic()) - { - VerifyTarget(obj); - } - - bool domainInitialized = _fieldInfo.m_declaringType.DomainInitialized; - ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType, ref domainInitialized); - _fieldInfo.m_declaringType.DomainInitialized = domainInitialized; - break; case FieldAccessorType.NoInvoke: if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) throw new InvalidOperationException(SR.Arg_UnboundGenField); + if (_fieldInfo.DeclaringType is not null && ((RuntimeType)_fieldInfo.FieldType).IsNullableOfT) + throw new NotSupportedException(); + throw new FieldAccessException(); -#if DEBUG - default: - throw new Exception("Unknown enum value"); -#endif } - } - return ret; + // Slow path + if (!IsStatic()) + { + VerifyTarget(obj); + } + + object? ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType, _fieldInfo.m_declaringType.DomainInitialized); + _fieldInfo.m_declaringType.DomainInitialized = true; + return ret; + } } public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) { - RuntimeType fieldType = (RuntimeType)_fieldInfo.FieldType; - - switch (_fieldAccessType) + unsafe { - case FieldAccessorType.InstanceReferenceType: - VerifyTarget(obj); - CheckValue(); - Debug.Assert(obj != null); - Volatile.Write(ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), value); - return; - - case FieldAccessorType.StaticReferenceType: - VerifyInitOnly(); - CheckValue(); - unsafe - { + switch (_fieldAccessType) + { + case FieldAccessorType.InstanceReferenceType: + VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); + Debug.Assert(obj != null); + Volatile.Write(ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), value); + return; + + case FieldAccessorType.StaticReferenceType: + VerifyStaticField(ref value, invokeAttr, binder, culture); Volatile.Write(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset), value); - } - return; + return; - case FieldAccessorType.InstanceValueType: - VerifyTarget(obj); - CheckValue(); - Debug.Assert(obj != null); - switch (_primitiveFieldSize) - { - case PrimitiveFieldSize.Size1: - Volatile.Write( - ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset), - value!.GetRawData()); - return; - - case PrimitiveFieldSize.Size2: - Volatile.Write( - ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), - Unsafe.As(ref value!.GetRawData())); - return; - - case PrimitiveFieldSize.Size4: - Volatile.Write( - ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), - Unsafe.As(ref value!.GetRawData())); - return; - - case PrimitiveFieldSize.Size8: - Volatile.Write( - ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), - Unsafe.As(ref value!.GetRawData())); - return; - } - break; + case FieldAccessorType.InstanceValueTypeSize1: + VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); + Debug.Assert(obj != null); + Volatile.Write( + ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset), + value!.GetRawData()); + return; - case FieldAccessorType.StaticValueType: - unsafe - { - switch (_primitiveFieldSize) - { - case PrimitiveFieldSize.Size1: - VerifyInitOnly(); - CheckValue(); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - value!.GetRawData()); - return; - - case PrimitiveFieldSize.Size2: - VerifyInitOnly(); - CheckValue(); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - Unsafe.As(ref value!.GetRawData())); - return; - - case PrimitiveFieldSize.Size4: - VerifyInitOnly(); - CheckValue(); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - Unsafe.As(ref value!.GetRawData())); - return; - - case PrimitiveFieldSize.Size8: - VerifyInitOnly(); - CheckValue(); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - Unsafe.As(ref value!.GetRawData())); - return; - } + case FieldAccessorType.InstanceValueTypeSize2: + VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); + Debug.Assert(obj != null); + Volatile.Write( + ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + Unsafe.As(ref value!.GetRawData())); + return; - break; - } + case FieldAccessorType.InstanceValueTypeSize4: + VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); + Debug.Assert(obj != null); + Volatile.Write( + ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + Unsafe.As(ref value!.GetRawData())); + return; - case FieldAccessorType.NoInvoke: - if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) - throw new InvalidOperationException(SR.Arg_UnboundGenField); + case FieldAccessorType.InstanceValueTypeSize8: + VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); + Debug.Assert(obj != null); + Volatile.Write( + ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), + Unsafe.As(ref value!.GetRawData())); + return; + + case FieldAccessorType.StaticValueTypeSize1: + VerifyStaticField(ref value, invokeAttr, binder, culture); + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + value!.GetRawData()); + return; + + case FieldAccessorType.StaticValueTypeSize2: + VerifyStaticField(ref value, invokeAttr, binder, culture); + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + return; + + case FieldAccessorType.StaticValueTypeSize4: + VerifyStaticField(ref value, invokeAttr, binder, culture); + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + return; + + case FieldAccessorType.StaticValueTypeSize8: + VerifyStaticField(ref value, invokeAttr, binder, culture); + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + return; - throw new FieldAccessException(); + case FieldAccessorType.NoInvoke: + if (_fieldInfo.DeclaringType is not null && _fieldInfo.DeclaringType.ContainsGenericParameters) + throw new InvalidOperationException(SR.Arg_UnboundGenField); + + throw new FieldAccessException(); + } } + // Slow path if (IsStatic()) { - VerifyInitOnly(); + VerifyStaticField(ref value, invokeAttr, binder, culture); } else { - VerifyTarget(obj); + VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); } - CheckValue(); - bool domainInitialized = _fieldInfo.m_declaringType.DomainInitialized; - RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, fieldType, (RuntimeType?)_fieldInfo.DeclaringType, ref domainInitialized); - _fieldInfo.m_declaringType.DomainInitialized = domainInitialized; - - void CheckValue() - { - if (value is null) - { - if (fieldType.IsActualValueType) - { - fieldType.CheckValue(ref value, binder, culture, invokeAttr); - } - } - else if (!ReferenceEquals(value.GetType(), fieldType)) - { - fieldType.CheckValue(ref value, binder, culture, invokeAttr); - } - } + RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType, _fieldInfo.m_declaringType.DomainInitialized); + _fieldInfo.m_declaringType.DomainInitialized = true; } private void InitializeClass() { - RtFieldInfo fieldInfo = _fieldInfo; - RuntimeType declaringType = _fieldInfo.m_declaringType; - // Call the class constructor if not already. - if (!declaringType.DomainInitialized) + if (!_fieldInfo.m_declaringType.DomainInitialized) { if (_fieldInfo.DeclaringType is null) { - InvokeModuleConstructor(fieldInfo.Module); + RunModuleConstructor(_fieldInfo.Module); } else { - InvokeClassConstructor(fieldInfo); + RunClassConstructor(_fieldInfo); } // A constructor is allowed to set init-only fields, so set this after. - declaringType.DomainInitialized = true; + _fieldInfo.m_declaringType.DomainInitialized = true; } } private bool IsStatic() => (_fieldInfo.Attributes & FieldAttributes.Static) == FieldAttributes.Static; + private void VerifyStaticField(ref object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) + { + VerifyInitOnly(); + CheckValue(ref value, invokeAttr, binder, culture); + } + + private void VerifyInstanceField(object? obj, ref object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) + { + VerifyTarget(obj); + CheckValue(ref value, invokeAttr, binder, culture); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void VerifyTarget(object? target) { @@ -353,14 +332,23 @@ internal void VerifyTarget(object? target) } } - private static void ThrowHelperTargetException() => throw new TargetException(SR.RFLCT_Targ_StatFldReqTarg); - - private static void ThrowHelperArgumentException(object target, FieldInfo fieldInfo) => - throw new ArgumentException(SR.Format(SR.Arg_FieldDeclTarget, fieldInfo.Name, fieldInfo.DeclaringType, target.GetType())); - - private static void ThrowHelperFieldAccessException(string fieldName, string? declaringTypeName) => - throw new FieldAccessException(SR.Format(SR.RFLCT_CannotSetInitonlyStaticField, fieldName, declaringTypeName)); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void CheckValue(ref object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) + { + if (value is null) + { + if (((RuntimeType)_fieldInfo.FieldType).IsActualValueType) + { + ((RuntimeType)_fieldInfo.FieldType).CheckValue(ref value, binder, culture, invokeAttr); + } + } + else if (!ReferenceEquals(value.GetType(), _fieldInfo.FieldType)) + { + ((RuntimeType)_fieldInfo.FieldType).CheckValue(ref value, binder, culture, invokeAttr); + } + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void VerifyInitOnly() { Debug.Assert(IsStatic()); @@ -373,79 +361,127 @@ internal void VerifyInitOnly() [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2059:RunClassConstructor", Justification = "This represents the static constructor, so if this object was created, the static constructor exists.")] - private static void InvokeClassConstructor(FieldInfo fieldInfo) + private static void RunClassConstructor(FieldInfo fieldInfo) { RuntimeHelpers.RunClassConstructor(fieldInfo.DeclaringType!.TypeHandle); } - private static void InvokeModuleConstructor(Module module) + private static void RunModuleConstructor(Module module) { RuntimeHelpers.RunModuleConstructor(module.ModuleHandle); } /// - /// Currently we only optimize for primitive types; these are simple because they are atomic write operations, are + /// Currently we only optimize for primitive types and not all value types. Primitive types support atomic write operations, are /// not boxed by the runtime when stored as a static field, and don't need special nullable, GC or alignment checks. /// - private static PrimitiveFieldSize GetPrimitiveFieldSize(Type fieldType, FieldAccessorType fieldAccessType) + private static FieldAccessorType GetPrimitiveAccessorTypeForInstance(Type fieldType) + { + FieldAccessorType accessorType = FieldAccessorType.InstanceValueType; + + if (fieldType == typeof(byte) || + fieldType == typeof(sbyte) || + fieldType == typeof(bool)) + accessorType = FieldAccessorType.InstanceValueTypeSize1; + else if (fieldType == typeof(short) || + fieldType == typeof(ushort) || + fieldType == typeof(char)) + accessorType = FieldAccessorType.InstanceValueTypeSize2; + else if (fieldType == typeof(int) || + fieldType == typeof(uint) || + fieldType == typeof(float)) + accessorType = FieldAccessorType.InstanceValueTypeSize4; + else if (fieldType == typeof(long) || + fieldType == typeof(ulong) || + fieldType == typeof(double)) + accessorType = FieldAccessorType.InstanceValueTypeSize8; + + return accessorType; + } + + private static FieldAccessorType GetPrimitiveAccessorTypeForStatic(Type fieldType) { - PrimitiveFieldSize size = PrimitiveFieldSize.NotPrimitive; + FieldAccessorType accessorType = FieldAccessorType.StaticValueType; + + if (fieldType == typeof(byte) || + fieldType == typeof(sbyte) || + fieldType == typeof(bool)) + accessorType = FieldAccessorType.StaticValueTypeSize1; + else if (fieldType == typeof(short) || + fieldType == typeof(ushort) || + fieldType == typeof(char)) + accessorType = FieldAccessorType.StaticValueTypeSize2; + else if (fieldType == typeof(int) || + fieldType == typeof(uint) || + fieldType == typeof(float)) + accessorType = FieldAccessorType.StaticValueTypeSize4; + else if (fieldType == typeof(long) || + fieldType == typeof(ulong) || + fieldType == typeof(double)) + accessorType = FieldAccessorType.StaticValueTypeSize8; + + return accessorType; + } - if (fieldAccessType == FieldAccessorType.InstanceValueType || - fieldAccessType == FieldAccessorType.StaticValueType) + private static FieldAccessorType GetFunctionPointerAccessorTypeForInstance() + { + FieldAccessorType accessorType = FieldAccessorType.InstanceValueType; + + if (IntPtr.Size == 4) { - if (fieldType == typeof(byte) || - fieldType == typeof(sbyte) || - fieldType == typeof(bool)) - size = PrimitiveFieldSize.Size1; - else if (fieldType == typeof(short) || - fieldType == typeof(ushort) || - fieldType == typeof(char)) - size = PrimitiveFieldSize.Size2; - else if (fieldType == typeof(int) || - fieldType == typeof(uint) || - fieldType == typeof(float)) - size = PrimitiveFieldSize.Size4; - else if (fieldType == typeof(long) || - fieldType == typeof(ulong) || - fieldType == typeof(double)) - size = PrimitiveFieldSize.Size8; - else if (fieldType.IsFunctionPointer) - { - if (IntPtr.Size == 4) - { - size = PrimitiveFieldSize.Size4; - } - else if (IntPtr.Size == 8) - { - size = PrimitiveFieldSize.Size8; - } - } + accessorType = FieldAccessorType.InstanceValueTypeSize4; + } + else if (IntPtr.Size == 8) + { + accessorType = FieldAccessorType.InstanceValueTypeSize8; } - return size; + return accessorType; } - private enum FieldAccessorType : short + private static FieldAccessorType GetFunctionPointerAccessorTypeForStatic() { - InstanceReferenceType = 0, - InstanceValueType = 1, - InstancePointerType = 2, - StaticReferenceType = 3, - StaticValueType = 4, - StaticValueTypeBoxed = 5, - StaticPointerType = 6, - SlowPath = 7, - NoInvoke = 8, + FieldAccessorType accessorType = FieldAccessorType.StaticValueType; + + if (IntPtr.Size == 4) + { + accessorType = FieldAccessorType.StaticValueTypeSize4; + } + else if (IntPtr.Size == 8) + { + accessorType = FieldAccessorType.StaticValueTypeSize8; + } + + return accessorType; } - private enum PrimitiveFieldSize : short + private static void ThrowHelperTargetException() => throw new TargetException(SR.RFLCT_Targ_StatFldReqTarg); + + private static void ThrowHelperArgumentException(object target, FieldInfo fieldInfo) => + throw new ArgumentException(SR.Format(SR.Arg_FieldDeclTarget, fieldInfo.Name, fieldInfo.DeclaringType, target.GetType())); + + private static void ThrowHelperFieldAccessException(string fieldName, string? declaringTypeName) => + throw new FieldAccessException(SR.Format(SR.RFLCT_CannotSetInitonlyStaticField, fieldName, declaringTypeName)); + + private enum FieldAccessorType { - Size1, - Size2, - Size4, - Size8, - NotPrimitive + InstanceReferenceType, + InstanceValueType, + InstanceValueTypeSize1, + InstanceValueTypeSize2, + InstanceValueTypeSize4, + InstanceValueTypeSize8, + InstancePointerType, + StaticReferenceType, + StaticValueType, + StaticValueTypeSize1, + StaticValueTypeSize2, + StaticValueTypeSize4, + StaticValueTypeSize8, + StaticValueTypeBoxed, + StaticPointerType, + NoInvoke, + SlowPath, } } } diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index a91834ca9f022d..9b6951e185c978 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -99,7 +99,6 @@ public static IEnumerable GetValue_TestData_WithFunctionPointers() } [Theory] - [ActiveIssue("https://github.com/dotnet/runtime/issues/97830", TestRuntimes.Mono)] [ActiveIssue("https://github.com/dotnet/runtime/issues/97833", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))] [MemberData(nameof(GetValue_TestData_WithFunctionPointers))] public void GetValueWithFunctionPointers(Type type, string name, object obj, object expected) @@ -195,7 +194,6 @@ public static IEnumerable SetValue_TestData_FunctionPointers() } [Theory] - [ActiveIssue("https://github.com/dotnet/runtime/issues/97830", TestRuntimes.Mono)] [ActiveIssue("https://github.com/dotnet/runtime/issues/97833", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))] [MemberData(nameof(SetValue_TestData_FunctionPointers))] public void SetValueWithFunctionPointers(Type type, string name, object obj, object value, object expected) From a104f1de8758f37aac7df4521f72793682888e45 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 7 Feb 2024 16:15:39 -0600 Subject: [PATCH 9/9] Fix issue when cctor calls FieldInfo.SetValue() --- .../src/System/RuntimeHandles.cs | 4 +- src/coreclr/vm/invokeutil.cpp | 16 +- src/coreclr/vm/invokeutil.h | 4 +- src/coreclr/vm/reflectioninvocation.cpp | 23 +-- src/coreclr/vm/runtimehandles.h | 4 +- .../src/System/Reflection/FieldAccessor.cs | 147 +++++++++++++----- 6 files changed, 136 insertions(+), 62 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 23950cd93e2868..6e6217bd33b5ac 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -1203,13 +1203,13 @@ internal static RuntimeType GetApproxDeclaringType(IRuntimeFieldInfo field) internal static extern int GetToken(RtFieldInfo field); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType, bool domainInitialized); + internal static extern object? GetValue(RtFieldInfo field, object? instance, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object? GetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, RuntimeType? contextType); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType, bool domainInitialized); + internal static extern void SetValue(RtFieldInfo field, object? obj, object? value, RuntimeType fieldType, RuntimeType? declaringType, ref bool domainInitialized); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void SetValueDirect(RtFieldInfo field, RuntimeType fieldType, void* pTypedRef, object? value, RuntimeType? contextType); diff --git a/src/coreclr/vm/invokeutil.cpp b/src/coreclr/vm/invokeutil.cpp index 8d3ffa0b73a9eb..0d7994a330ca75 100644 --- a/src/coreclr/vm/invokeutil.cpp +++ b/src/coreclr/vm/invokeutil.cpp @@ -748,7 +748,7 @@ void InvokeUtil::SetValidField(CorElementType fldType, OBJECTREF *target, OBJECTREF *valueObj, TypeHandle declaringType, - CLR_BOOL domainInitialized) { + CLR_BOOL *pDomainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -786,17 +786,19 @@ void InvokeUtil::SetValidField(CorElementType fldType, pDeclMT = pField->GetModule()->GetGlobalMethodTable(); } - if (domainInitialized == FALSE) + if (*pDomainInitialized == FALSE) { EX_TRY { pDeclMT->EnsureInstanceActive(); pDeclMT->CheckRunClassInitThrowing(); + + *pDomainInitialized = TRUE; } EX_CATCH_THROWABLE(&Throwable); } #ifdef _DEBUG - else if (!declaringType.IsNull()) + else if (*pDomainInitialized == TRUE && !declaringType.IsNull()) CONSISTENCY_CHECK(declaringType.GetMethodTable()->CheckActivated()); #endif @@ -971,7 +973,7 @@ void InvokeUtil::SetValidField(CorElementType fldType, // GetFieldValue // This method will return an ARG_SLOT containing the value of the field. -OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL domainInitialized) { +OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL *pDomainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -1009,17 +1011,19 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ pDeclMT = pField->GetModule()->GetGlobalMethodTable(); } - if (domainInitialized == FALSE) + if (*pDomainInitialized == FALSE) { EX_TRY { pDeclMT->EnsureInstanceActive(); pDeclMT->CheckRunClassInitThrowing(); + + *pDomainInitialized = TRUE; } EX_CATCH_THROWABLE(&Throwable); } #ifdef _DEBUG - else if (!declaringType.IsNull()) + else if (*pDomainInitialized == TRUE && !declaringType.IsNull()) CONSISTENCY_CHECK(declaringType.GetMethodTable()->CheckActivated()); #endif diff --git a/src/coreclr/vm/invokeutil.h b/src/coreclr/vm/invokeutil.h index cdadc381fc15e6..0bd1577c7a19d3 100644 --- a/src/coreclr/vm/invokeutil.h +++ b/src/coreclr/vm/invokeutil.h @@ -138,9 +138,9 @@ class InvokeUtil // SetValidField // Given an target object, a value object and a field this method will set the field // on the target object. The field must be validate before calling this. - static void SetValidField(CorElementType fldType, TypeHandle fldTH, FieldDesc* pField, OBJECTREF* target, OBJECTREF* value, TypeHandle declaringType, CLR_BOOL domainInitialized); + static void SetValidField(CorElementType fldType, TypeHandle fldTH, FieldDesc* pField, OBJECTREF* target, OBJECTREF* value, TypeHandle declaringType, CLR_BOOL *pDomainInitialized); - static OBJECTREF GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL domainInitialized); + static OBJECTREF GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJECTREF* target, TypeHandle declaringType, CLR_BOOL *pDomainInitialized); // ValidateObjectTarget // This method will validate the Object/Target relationship diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index 08ccb07cf30b9f..d6e1d98b609435 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -25,7 +25,7 @@ #include "dbginterface.h" #include "argdestination.h" -FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL domainInitialized) { +FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { CONTRACTL { FCALL_CHECK; } @@ -54,7 +54,7 @@ FCIMPL5(Object*, RuntimeFieldHandle::GetValue, ReflectFieldObject *pFieldUNSAFE, HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); // There can be no GC after this until the Object is returned. - rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, domainInitialized); + rv = InvokeUtil::GetFieldValue(gc.refField->GetField(), fieldType, &gc.target, declaringType, pDomainInitialized); HELPER_METHOD_FRAME_END(); return OBJECTREFToObject(rv); @@ -158,7 +158,7 @@ FCIMPL2(Object*, ReflectionInvocation::AllocateValueType, ReflectClassBaseObject } FCIMPLEND -FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL domainInitialized) { +FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pDomainInitialized) { CONTRACTL { FCALL_CHECK; } @@ -190,7 +190,7 @@ FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); - InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, domainInitialized); + InvokeUtil::SetValidField(fieldType.GetVerifierCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized); HELPER_METHOD_FRAME_END(); } @@ -914,7 +914,7 @@ FCIMPL1(ReflectMethodObject*, RuntimeMethodHandle::GetCurrentMethod, StackCrawlM } FCIMPLEND -static OBJECTREF DirectObjectFieldGet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, CLR_BOOL domainInitialized) { +static OBJECTREF DirectObjectFieldGet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, CLR_BOOL *pDomainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -932,7 +932,7 @@ static OBJECTREF DirectObjectFieldGet(FieldDesc *pField, TypeHandle fieldType, T } InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref); - refRet = InvokeUtil::GetFieldValue(pField, fieldType, &objref, enclosingType, domainInitialized); + refRet = InvokeUtil::GetFieldValue(pField, fieldType, &objref, enclosingType, pDomainInitialized); GCPROTECT_END(); return refRet; } @@ -1037,7 +1037,7 @@ lExit: ; } FCIMPLEND -static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, OBJECTREF *pValue, CLR_BOOL domainInitialized) { +static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHandle enclosingType, TypedByRef *pTarget, OBJECTREF *pValue, CLR_BOOL *pDomainInitialized) { CONTRACTL { THROWS; GC_TRIGGERS; @@ -1056,7 +1056,7 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa // Validate the target/fld type relationship InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref); - InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, domainInitialized); + InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized); GCPROTECT_END(); } @@ -1102,8 +1102,9 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA // 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()) { - DirectObjectFieldSet(pField, fieldType, TypeHandle(pEnclosingMT), pTarget, &gc.oValue, FALSE); + DirectObjectFieldSet(pField, fieldType, TypeHandle(pEnclosingMT), pTarget, &gc.oValue, &domainInitialized); goto lExit; } @@ -1264,6 +1265,7 @@ FCIMPL1(INT32, RuntimeFieldHandle::GetInstanceFieldOffset, ReflectFieldObject *p FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); FieldDesc* pFieldDesc = refField->GetField(); + _ASSERTE(!pFieldDesc->IsStatic()); // IsFastPathSupported needs to checked before calling this method. _ASSERTE(IsFastPathSupportedHelper(pFieldDesc)); @@ -1285,6 +1287,7 @@ FCIMPL2(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pF FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); FieldDesc* pFieldDesc = refField->GetField(); + _ASSERTE(pFieldDesc->IsStatic()); // IsFastPathSupported needs to checked before calling this method. _ASSERTE(IsFastPathSupportedHelper(pFieldDesc)); @@ -1297,8 +1300,6 @@ FCIMPL2(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pF } else { - CorElementType fieldType = pFieldDesc->GetFieldType(); - // Non-primitive value types need to be unboxed to get the base address. *isBoxed = (pFieldDesc->GetFieldType() == ELEMENT_TYPE_VALUETYPE) ? TRUE : FALSE; diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 17140ee4a73968..c27b6c6f2aaae9 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -292,8 +292,8 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_Destroy(MethodDesc * pMethod); class RuntimeFieldHandle { public: - static FCDECL5(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL domainInitialized); - static FCDECL6(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL domainInitialized); + static FCDECL5(Object*, GetValue, ReflectFieldObject *pFieldUNSAFE, Object *instanceUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); + static FCDECL6(void, SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldType, ReflectClassBaseObject *pDeclaringType, CLR_BOOL *pDomainInitialized); static FCDECL4(Object*, GetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, ReflectClassBaseObject *pDeclaringType); static FCDECL5(void, SetValueDirect, ReflectFieldObject *pFieldUNSAFE, ReflectClassBaseObject *pFieldType, TypedByRef *pTarget, Object *valueUNSAFE, ReflectClassBaseObject *pContextType); static FCDECL1(FC_BOOL_RET, IsFastPathSupported, ReflectFieldObject *pField); diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index 95e1a0dfbf4660..08c3189deab997 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -29,12 +29,12 @@ internal FieldAccessor(FieldInfo fieldInfo) } // Initialize for the type of field. - RuntimeType? declaringType = (RuntimeType?)_fieldInfo.DeclaringType; - if (declaringType is not null && declaringType.ContainsGenericParameters) + Debug.Assert(_fieldInfo.m_declaringType != null); + if (_fieldInfo.m_declaringType.ContainsGenericParameters) { _fieldAccessType = FieldAccessorType.NoInvoke; } - else if (declaringType is not null && declaringType.IsNullableOfT) + else if (_fieldInfo.m_declaringType.IsNullableOfT) { _fieldAccessType = FieldAccessorType.NoInvoke; } @@ -120,6 +120,10 @@ internal FieldAccessor(FieldInfo fieldInfo) case FieldAccessorType.InstanceReferenceType: VerifyTarget(obj); Debug.Assert(obj != null); + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } return Volatile.Read(ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset))); case FieldAccessorType.InstanceValueType: @@ -129,6 +133,10 @@ internal FieldAccessor(FieldInfo fieldInfo) case FieldAccessorType.InstanceValueTypeSize8: VerifyTarget(obj); Debug.Assert(obj != null); + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } return RuntimeHelpers.Box( _methodTable, ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)); @@ -136,11 +144,19 @@ internal FieldAccessor(FieldInfo fieldInfo) case FieldAccessorType.InstancePointerType: VerifyTarget(obj); Debug.Assert(obj != null); + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } return Pointer.Box( (void*)Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addressOrOffset)), _fieldInfo.FieldType); case FieldAccessorType.StaticReferenceType: + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } return Volatile.Read(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset)); case FieldAccessorType.StaticValueType: @@ -148,15 +164,27 @@ internal FieldAccessor(FieldInfo fieldInfo) case FieldAccessorType.StaticValueTypeSize2: case FieldAccessorType.StaticValueTypeSize4: case FieldAccessorType.StaticValueTypeSize8: + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } return RuntimeHelpers.Box(_methodTable, ref Unsafe.AsRef(_addressOrOffset.ToPointer())); case FieldAccessorType.StaticValueTypeBoxed: + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } // Re-box the value. return RuntimeHelpers.Box( _methodTable, ref Unsafe.As(ref *(IntPtr*)_addressOrOffset).GetRawData()); case FieldAccessorType.StaticPointerType: + if (!_fieldInfo.m_declaringType.DomainInitialized) + { + return SlowPath(); + } return Pointer.Box((void*)Unsafe.As( ref Unsafe.AsRef(_addressOrOffset.ToPointer())), _fieldInfo.FieldType); @@ -176,9 +204,17 @@ internal FieldAccessor(FieldInfo fieldInfo) VerifyTarget(obj); } - object? ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType, _fieldInfo.m_declaringType.DomainInitialized); - _fieldInfo.m_declaringType.DomainInitialized = true; - return ret; + return SlowPath(); + + object? SlowPath() + { + object? ret; + RuntimeType declaringType = _fieldInfo.m_declaringType; + bool domainInitialized = declaringType.DomainInitialized; + ret = RuntimeFieldHandle.GetValue(_fieldInfo, obj, (RuntimeType)_fieldInfo.FieldType, declaringType, ref domainInitialized); + declaringType.DomainInitialized = domainInitialized; + return ret; + } } } @@ -195,8 +231,14 @@ public void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder return; case FieldAccessorType.StaticReferenceType: - VerifyStaticField(ref value, invokeAttr, binder, culture); - Volatile.Write(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset), value); + if (!VerifyStaticField(ref value, invokeAttr, binder, culture)) + { + SlowPath(); + } + else + { + Volatile.Write(ref Unsafe.As(ref *(IntPtr*)_addressOrOffset), value); + } return; case FieldAccessorType.InstanceValueTypeSize1: @@ -232,31 +274,55 @@ ref Unsafe.As(ref Unsafe.AddByteOffset(ref obj.GetRawData(), _addres return; case FieldAccessorType.StaticValueTypeSize1: - VerifyStaticField(ref value, invokeAttr, binder, culture); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - value!.GetRawData()); + if (!VerifyStaticField(ref value, invokeAttr, binder, culture)) + { + SlowPath(); + } + else + { + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + value!.GetRawData()); + } return; case FieldAccessorType.StaticValueTypeSize2: - VerifyStaticField(ref value, invokeAttr, binder, culture); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - Unsafe.As(ref value!.GetRawData())); + if (!VerifyStaticField(ref value, invokeAttr, binder, culture)) + { + SlowPath(); + } + else + { + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + } return; case FieldAccessorType.StaticValueTypeSize4: - VerifyStaticField(ref value, invokeAttr, binder, culture); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - Unsafe.As(ref value!.GetRawData())); + if (!VerifyStaticField(ref value, invokeAttr, binder, culture)) + { + SlowPath(); + } + else + { + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + } return; case FieldAccessorType.StaticValueTypeSize8: - VerifyStaticField(ref value, invokeAttr, binder, culture); - Volatile.Write( - ref Unsafe.AsRef(_addressOrOffset.ToPointer()), - Unsafe.As(ref value!.GetRawData())); + if (!VerifyStaticField(ref value, invokeAttr, binder, culture)) + { + SlowPath(); + } + else + { + Volatile.Write( + ref Unsafe.AsRef(_addressOrOffset.ToPointer()), + Unsafe.As(ref value!.GetRawData())); + } return; case FieldAccessorType.NoInvoke: @@ -277,35 +343,38 @@ ref Unsafe.AsRef(_addressOrOffset.ToPointer()), VerifyInstanceField(obj, ref value, invokeAttr, binder, culture); } - RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, (RuntimeType)_fieldInfo.FieldType, (RuntimeType?)_fieldInfo.DeclaringType, _fieldInfo.m_declaringType.DomainInitialized); - _fieldInfo.m_declaringType.DomainInitialized = true; + SlowPath(); + + void SlowPath() + { + RuntimeType declaringType = _fieldInfo.m_declaringType; + bool domainInitialized = declaringType.DomainInitialized; + RuntimeFieldHandle.SetValue(_fieldInfo, obj, value, (RuntimeType)_fieldInfo.FieldType, declaringType, ref domainInitialized); + declaringType.DomainInitialized = domainInitialized; + } } private void InitializeClass() { // Call the class constructor if not already. - if (!_fieldInfo.m_declaringType.DomainInitialized) + if (_fieldInfo.DeclaringType is null) { - if (_fieldInfo.DeclaringType is null) - { - RunModuleConstructor(_fieldInfo.Module); - } - else - { - RunClassConstructor(_fieldInfo); - } - - // A constructor is allowed to set init-only fields, so set this after. - _fieldInfo.m_declaringType.DomainInitialized = true; + RunModuleConstructor(_fieldInfo.Module); + } + else + { + RunClassConstructor(_fieldInfo); } } private bool IsStatic() => (_fieldInfo.Attributes & FieldAttributes.Static) == FieldAttributes.Static; - private void VerifyStaticField(ref object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) + private bool VerifyStaticField(ref object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture) { VerifyInitOnly(); CheckValue(ref value, invokeAttr, binder, culture); + + return _fieldInfo.m_declaringType.DomainInitialized; } private void VerifyInstanceField(object? obj, ref object? value, BindingFlags invokeAttr, Binder? binder, CultureInfo? culture)