Skip to content

Commit

Permalink
Fix UnsafeAccessor scenario for modopts/modreqs when comparing fiel…
Browse files Browse the repository at this point in the history
…d sigs. (#111648)

* Consume custom modifiers and ByRef in RetType signature
prior to comparing field signature.
  • Loading branch information
AaronRobinsonMSFT committed Jan 21, 2025
1 parent 9efd46e commit 1313bb6
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 14 deletions.
11 changes: 9 additions & 2 deletions src/coreclr/vm/prestub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/siginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/vm/siginfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,28 +328,70 @@ 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;
}

// 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]
Expand Down

0 comments on commit 1313bb6

Please sign in to comment.