Skip to content

Commit

Permalink
Fix issue where variance checking uses an unsafe contract by being mo…
Browse files Browse the repository at this point in the history
…re strict in what we accept while verifying the type (#21)
  • Loading branch information
davidwrighton authored May 12, 2021
1 parent a24bf8c commit 9495cbf
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
24 changes: 19 additions & 5 deletions src/coreclr/vm/methodtable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9189,7 +9189,7 @@ MethodDesc *MethodTable::GetDefaultConstructor(BOOL forceBoxedEntryPoint /* = FA
//==========================================================================================
// Finds the (non-unboxing) MethodDesc that implements the interface virtual static method pInterfaceMD.
MethodDesc *
MethodTable::ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL allowNullResult, BOOL checkDuplicates)
MethodTable::ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL allowNullResult, BOOL checkDuplicates, BOOL allowVariantMatches)
{
if (!pInterfaceMD->IsSharedByGenericMethodInstantiations() && !pInterfaceType->IsSharedByGenericInstantiations())
{
Expand Down Expand Up @@ -9225,7 +9225,7 @@ MethodTable::ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc*
return pMD;
}

if (pInterfaceType->HasVariance())
if (pInterfaceType->HasVariance() || pInterfaceType->HasTypeEquivalence())
{
// Variant interface dispatch
MethodTable::InterfaceMapIterator it = IterateInterfaceMap();
Expand All @@ -9240,12 +9240,26 @@ MethodTable::ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc*
if (!it.GetInterface()->HasSameTypeDefAs(pInterfaceType))
{
// Variance matches require a typedef match
// Equivalence isn't sufficient, and is uninteresting as equivalent interfaces cannot have static virtuals.
continue;
}

if (it.GetInterface()->CanCastTo(pInterfaceType, NULL))
BOOL equivalentOrVariantCompatible;

if (allowVariantMatches)
{
equivalentOrVariantCompatible = it.GetInterface()->CanCastTo(pInterfaceType, NULL);
}
else
{
// When performing override checking to ensure that a concrete type is valid, require the implementation
// actually implement the exact or equivalent interface.
equivalentOrVariantCompatible = it.GetInterface()->IsEquivalentTo(pInterfaceType, NULL);
}

if (equivalentOrVariantCompatible)
{
// Variant matching interface found
// Variant or equivalent matching interface found
// Attempt to resolve on variance matched interface
pMD = pMT->TryResolveVirtualStaticMethodOnThisType(it.GetInterface(), pInterfaceMD, checkDuplicates);
if (pMD != nullptr)
Expand Down Expand Up @@ -9406,7 +9420,7 @@ MethodTable::VerifyThatAllVirtualStaticMethodsAreImplemented()
MethodDesc *pMD = it.GetMethodDesc();
if (pMD->IsVirtual() &&
pMD->IsStatic() &&
!ResolveVirtualStaticMethod(pInterfaceMT, pMD, /* allowNullResult */ TRUE, /* checkDuplicates */ TRUE))
!ResolveVirtualStaticMethod(pInterfaceMT, pMD, /* allowNullResult */ TRUE, /* checkDuplicates */ TRUE, /* allowVariantMatches */ FALSE))
{
IMDInternalImport* pInternalImport = GetModule()->GetMDImport();
GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, GetCl(), pMD->GetName(), IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/methodtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -2282,7 +2282,7 @@ class MethodTable


// Resolve virtual static interface method pInterfaceMD on this type.
MethodDesc *ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL allowNullResult, BOOL checkDuplicates = FALSE);
MethodDesc *ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL allowNullResult, BOOL checkDuplicates = FALSE, BOOL allowVariantMatches = TRUE);

// Try a partial resolve of the constraint call, up to generic code sharing.
//
Expand Down

0 comments on commit 9495cbf

Please sign in to comment.