Fix UnsafeAccessor scenario for modopts/modreqs when comparing field sigs.

src/coreclr/vm/prestub.cpp

@@ -1411,10 +1411,27 @@ namespace
         DWORD declArgCount;
         IfFailThrow(CorSigUncompressData_EndPtr(pSig1, pEndSig1, &declArgCount));
 
-        // UnsafeAccessors for fields require return types be byref.
-        // This was explicitly checked in TryGenerateUnsafeAccessor().
         if (pSig1 >= pEndSig1)
             ThrowHR(META_E_BAD_SIGNATURE);
+
+        // 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)
+        // The ELEMENT_TYPE_BYREF was explicitly checked in TryGenerateUnsafeAccessor().
+        _ASSERTE(state.IgnoreCustomModifiers); // We should always ignore custom modifiers for field look-up.
+        PCCOR_SIGNATURE pSig1Tmp = pSig1;
+        CorElementType byRefTypeMaybe = CorSigUncompressElementType(pSig1Tmp);
+        if (byRefTypeMaybe != ELEMENT_TYPE_BYREF)
+        {
+            _ASSERTE(byRefTypeMaybe == ELEMENT_TYPE_CMOD_INTERNAL
+                || byRefTypeMaybe == ELEMENT_TYPE_CMOD_REQD
+                || byRefTypeMaybe == ELEMENT_TYPE_CMOD_OPT);
+
+            MetaSig::ConsumeCustomModifiers(pSig1, pEndSig1);
+            if (pSig1 >= pEndSig1)
+                ThrowHR(META_E_BAD_SIGNATURE);
+        }
+
         CorElementType byRefType = CorSigUncompressElementType(pSig1);
         _ASSERTE(byRefType == ELEMENT_TYPE_BYREF);
 

src/coreclr/vm/siginfo.cpp

@@ -3666,7 +3666,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;
     void* ptr;

src/coreclr/vm/siginfo.hpp

@@ -947,6 +947,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

src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.cs

@@ -328,28 +328,32 @@ public static void Verify_AccessAllFields_CorElementType()
         extern static ref delegate*<void> 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;
     }
 
-    [Fact]
-    public static void Verify_AccessFieldsWithModifiers()
+    // Accessors for fields that are volatile
+    static class AccessorsVolatile
     {
-        Console.WriteLine($"Running {nameof(Verify_AccessFieldsWithModifiers)}");
+        [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_vInt")]
+        public extern static ref int GetStaticVolatileInt(ref VolatileFields f);
 
-        FieldsWithModifiers fieldsWithModifiers = default;
+        [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_vInt")]
+        public extern static ref int GetVolatileInt(ref VolatileFields f);
+    }
 
-        GetStaticVolatileInt(ref fieldsWithModifiers) = default;
-        GetVolatileInt(ref fieldsWithModifiers) = default;
+    [Fact]
+    public static void Verify_AccessFieldsWithVolatile()
+    {
+        Console.WriteLine($"Running {nameof(Verify_AccessFieldsWithVolatile)}");
 
-        [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_vInt")]
-        extern static ref int GetStaticVolatileInt(ref FieldsWithModifiers f);
+        VolatileFields fieldsWithVolatile = default;
 
-        [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_vInt")]
-        extern static ref int GetVolatileInt(ref FieldsWithModifiers f);
+        AccessorsVolatile.GetStaticVolatileInt(ref fieldsWithVolatile) = default;
+        AccessorsVolatile.GetVolatileInt(ref fieldsWithVolatile) = default;
     }
 
     // Contains fields that are readonly
@@ -359,21 +363,35 @@ readonly struct ReadOnlyFields
         public readonly int _rInt;
     }
 
+    // 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 GetStaticReadOnlyInt(in readOnlyFields), in ReadOnlyFields.s_rInt));
-        Assert.True(Unsafe.AreSame(in GetReadOnlyInt(in readOnlyFields), in readOnlyFields._rInt));
+        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 GetStaticReadOnlyInt(ref readonly ReadOnlyFields f);
+        extern static ref readonly int GetStaticReadOnlyIntLocal(ref readonly ReadOnlyFields f);
 
         [UnsafeAccessor(UnsafeAccessorKind.Field, Name="_rInt")]
-        extern static ref readonly int GetReadOnlyInt(ref readonly ReadOnlyFields f);
+        extern static ref readonly int GetReadOnlyIntLocal(ref readonly ReadOnlyFields f);
     }
 
     [Fact]