diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index e494c74667cba1..99086a462a26fb 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -1445,11 +1445,18 @@ namespace DWORD declArgCount; IfFailThrow(CorSigUncompressData_EndPtr(pSig1, pEndSig1, &declArgCount)); + if (pSig1 >= pEndSig1) + ThrowHR(META_E_BAD_SIGNATURE); - // UnsafeAccessors for fields require return types be byref. - // This was explicitly checked in TryGenerateUnsafeAccessor(). + // UnsafeAccessors for fields require return types be byref. However, we first need to + // consume any custom modifiers which are prior to the expected ELEMENT_TYPE_BYREF in + // the RetType signature (II.23.2.11). + _ASSERTE(state.IgnoreCustomModifiers); // We should always ignore custom modifiers for field look-up. + MetaSig::ConsumeCustomModifiers(pSig1, pEndSig1); if (pSig1 >= pEndSig1) ThrowHR(META_E_BAD_SIGNATURE); + + // The ELEMENT_TYPE_BYREF was explicitly checked in TryGenerateUnsafeAccessor(). CorElementType byRefType = CorSigUncompressElementType(pSig1); _ASSERTE(byRefType == ELEMENT_TYPE_BYREF); diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index facb809cd4841a..2b50fdfcfeaa49 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -3604,7 +3604,7 @@ BOOL CompareTypeTokens(mdToken tk1, mdToken tk2, ModuleBase *pModule1, ModuleBas #endif //!DACCESS_COMPILE } // CompareTypeTokens -static void ConsumeCustomModifiers(PCCOR_SIGNATURE& pSig, PCCOR_SIGNATURE pEndSig) +void MetaSig::ConsumeCustomModifiers(PCCOR_SIGNATURE& pSig, PCCOR_SIGNATURE pEndSig) { mdToken tk; CorElementType type; diff --git a/src/coreclr/vm/siginfo.hpp b/src/coreclr/vm/siginfo.hpp index fab9a79260d2d1..49f14b57c34bee 100644 --- a/src/coreclr/vm/siginfo.hpp +++ b/src/coreclr/vm/siginfo.hpp @@ -945,6 +945,14 @@ class MetaSig //------------------------------------------------------------------ CorElementType GetByRefType(TypeHandle* pTy) const; + //------------------------------------------------------------------ + // Consume the custom modifiers, if any, in the current signature + // and update it. + // This is a non destructive operation if the current signature is not + // pointing at a sequence of ELEMENT_TYPE_CMOD_REQD or ELEMENT_TYPE_CMOD_OPT. + //------------------------------------------------------------------ + static void ConsumeCustomModifiers(PCCOR_SIGNATURE& pSig, PCCOR_SIGNATURE pEndSig); + // Struct used to capture in/out state during the comparison // of element types. struct CompareState diff --git a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs index f4efeacf80fe94..fcdb479c1a6353 100644 --- a/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs +++ b/src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs @@ -328,28 +328,70 @@ public static void Verify_AccessAllFields_CorElementType() extern static ref delegate* GetFPtr(ref AllFields f); } - // Contains fields that have modopts/modreqs - struct FieldsWithModifiers + // Contains fields that are volatile + struct VolatileFields { private static volatile int s_vInt; private volatile int _vInt; } + // Accessors for fields that are volatile + static class AccessorsVolatile + { + [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_vInt")] + public extern static ref int GetStaticVolatileInt(ref VolatileFields f); + + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_vInt")] + public extern static ref int GetVolatileInt(ref VolatileFields f); + } + [Fact] - public static void Verify_AccessFieldsWithModifiers() + public static void Verify_AccessFieldsWithVolatile() { - Console.WriteLine($"Running {nameof(Verify_AccessFieldsWithModifiers)}"); + Console.WriteLine($"Running {nameof(Verify_AccessFieldsWithVolatile)}"); - FieldsWithModifiers fieldsWithModifiers = default; + VolatileFields fieldsWithVolatile = default; - GetStaticVolatileInt(ref fieldsWithModifiers) = default; - GetVolatileInt(ref fieldsWithModifiers) = default; + AccessorsVolatile.GetStaticVolatileInt(ref fieldsWithVolatile) = default; + AccessorsVolatile.GetVolatileInt(ref fieldsWithVolatile) = default; + } - [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_vInt")] - extern static ref int GetStaticVolatileInt(ref FieldsWithModifiers f); + // Contains fields that are readonly + readonly struct ReadOnlyFields + { + public static readonly int s_rInt; + public readonly int _rInt; + } - [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_vInt")] - extern static ref int GetVolatileInt(ref FieldsWithModifiers f); + // Accessors for fields that are readonly + static class AccessorsReadOnly + { + [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_rInt")] + public extern static ref readonly int GetStaticReadOnlyInt(ref readonly ReadOnlyFields f); + + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_rInt")] + public extern static ref readonly int GetReadOnlyInt(ref readonly ReadOnlyFields f); + } + + [Fact] + public static void Verify_AccessFieldsWithReadOnlyRefs() + { + Console.WriteLine($"Running {nameof(Verify_AccessFieldsWithReadOnlyRefs)}"); + + ReadOnlyFields readOnlyFields = default; + + Assert.True(Unsafe.AreSame(in AccessorsReadOnly.GetStaticReadOnlyInt(in readOnlyFields), in ReadOnlyFields.s_rInt)); + Assert.True(Unsafe.AreSame(in AccessorsReadOnly.GetReadOnlyInt(in readOnlyFields), in readOnlyFields._rInt)); + + // Test the local declaration of the signature since it places modopts/modreqs differently. + Assert.True(Unsafe.AreSame(in GetStaticReadOnlyIntLocal(in readOnlyFields), in ReadOnlyFields.s_rInt)); + Assert.True(Unsafe.AreSame(in GetReadOnlyIntLocal(in readOnlyFields), in readOnlyFields._rInt)); + + [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_rInt")] + extern static ref readonly int GetStaticReadOnlyIntLocal(ref readonly ReadOnlyFields f); + + [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_rInt")] + extern static ref readonly int GetReadOnlyIntLocal(ref readonly ReadOnlyFields f); } [Fact]