From b9b349decc9cedbc1be4cc0b918b8b190ab0e59d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 15 Feb 2019 21:25:08 -0800 Subject: [PATCH] Fix bug in FindDispatchSlot usage --- src/vm/comcallablewrapper.cpp | 2 +- src/vm/contractimpl.h | 2 +- src/vm/gccover.cpp | 2 +- src/vm/memberload.cpp | 2 +- src/vm/methodtable.cpp | 14 ++++---------- src/vm/methodtable.h | 6 ++---- src/vm/virtualcallstub.cpp | 22 +++++++++++----------- 7 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/vm/comcallablewrapper.cpp b/src/vm/comcallablewrapper.cpp index ea18275..24987a8 100644 --- a/src/vm/comcallablewrapper.cpp +++ b/src/vm/comcallablewrapper.cpp @@ -3239,7 +3239,7 @@ IUnknown * ComCallWrapper::GetComIPFromCCW_HandleExtendsCOMObject( MethodDesc *pClsMD = NULL; // Find the implementation for the first slot of the interface - DispatchSlot impl(pMT->FindDispatchSlot(intIt.GetInterface()->GetTypeID(), 0)); + DispatchSlot impl(pMT->FindDispatchSlot(intIt.GetInterface()->GetTypeID(), 0, FALSE /* throwOnConflict */)); CONSISTENCY_CHECK(!impl.IsNull()); // Get the MethodDesc for this slot in the class diff --git a/src/vm/contractimpl.h b/src/vm/contractimpl.h index 3149a4d..9b8aa22 100644 --- a/src/vm/contractimpl.h +++ b/src/vm/contractimpl.h @@ -256,7 +256,7 @@ public: m_token = INVALID_TOKEN; } - DispatchToken(UINT_PTR token) + explicit DispatchToken(UINT_PTR token) { CONSISTENCY_CHECK(token != INVALID_TOKEN); m_token = token; diff --git a/src/vm/gccover.cpp b/src/vm/gccover.cpp index d4f0bac..6b2e7a6 100644 --- a/src/vm/gccover.cpp +++ b/src/vm/gccover.cpp @@ -59,7 +59,7 @@ static MethodDesc* getTargetMethodDesc(PCODE target) if (vsdStubKind != VirtualCallStubManager::SK_BREAKPOINT && vsdStubKind != VirtualCallStubManager::SK_UNKNOWN) { // It is a VSD stub manager. - DispatchToken token = VirtualCallStubManager::GetTokenFromStubQuick(pVSDStubManager, target, vsdStubKind); + DispatchToken token(VirtualCallStubManager::GetTokenFromStubQuick(pVSDStubManager, target, vsdStubKind)); _ASSERTE(token.IsValid()); return VirtualCallStubManager::GetInterfaceMethodDescFromToken(token); } diff --git a/src/vm/memberload.cpp b/src/vm/memberload.cpp index 03081e3..4bd663a 100644 --- a/src/vm/memberload.cpp +++ b/src/vm/memberload.cpp @@ -1179,7 +1179,7 @@ MemberLoader::FindMethodForInterfaceSlot(MethodTable * pMT, MethodTable *pInterf MethodDesc *pMDRet = NULL; - DispatchSlot ds(pMT->FindDispatchSlot(pInterface->GetTypeID(), (UINT32)slotNum)); + DispatchSlot ds(pMT->FindDispatchSlot(pInterface->GetTypeID(), (UINT32)slotNum, FALSE /* throwOnConflict */)); if (!ds.IsNull()) { pMDRet = ds.GetMethodDesc(); } diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp index f922be0..f434a39 100644 --- a/src/vm/methodtable.cpp +++ b/src/vm/methodtable.cpp @@ -7337,15 +7337,6 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( //========================================================================================== DispatchSlot MethodTable::FindDispatchSlot(UINT32 typeID, UINT32 slotNumber, BOOL throwOnConflict) { - WRAPPER_NO_CONTRACT; - DispatchSlot implSlot(NULL); - FindDispatchImpl(typeID, slotNumber, &implSlot, throwOnConflict); - return implSlot; -} - -//========================================================================================== -DispatchSlot MethodTable::FindDispatchSlot(DispatchToken tok, BOOL throwOnConflict) -{ CONTRACTL { THROWS; @@ -7353,7 +7344,10 @@ DispatchSlot MethodTable::FindDispatchSlot(DispatchToken tok, BOOL throwOnConfli MODE_ANY; } CONTRACTL_END; - return FindDispatchSlot(tok.GetTypeID(), tok.GetSlotNumber(), throwOnConflict); + + DispatchSlot implSlot(NULL); + FindDispatchImpl(typeID, slotNumber, &implSlot, throwOnConflict); + return implSlot; } #ifndef DACCESS_COMPILE diff --git a/src/vm/methodtable.h b/src/vm/methodtable.h index f0c035e..9f9b25e 100644 --- a/src/vm/methodtable.h +++ b/src/vm/methodtable.h @@ -2441,14 +2441,14 @@ protected: UINT32 slotNumber, DispatchMapEntry *pEntry); -public: +private: BOOL FindDispatchImpl( UINT32 typeID, UINT32 slotNumber, DispatchSlot * pImplSlot, BOOL throwOnConflict); - +public: #ifndef DACCESS_COMPILE BOOL FindDefaultInterfaceImplementation( MethodDesc *pInterfaceMD, @@ -2460,8 +2460,6 @@ public: DispatchSlot FindDispatchSlot(UINT32 typeID, UINT32 slotNumber, BOOL throwOnConflict); - DispatchSlot FindDispatchSlot(DispatchToken tok, BOOL throwOnConflict); - // You must use the second of these two if there is any chance the pMD is a method // on a generic interface such as IComparable (which it normally can be). The // ownerType is used to provide an exact qualification in the case the pMD is diff --git a/src/vm/virtualcallstub.cpp b/src/vm/virtualcallstub.cpp index 08ad121..6eab546 100644 --- a/src/vm/virtualcallstub.cpp +++ b/src/vm/virtualcallstub.cpp @@ -1154,7 +1154,7 @@ BOOL VirtualCallStubManager::TraceManager(Thread *thread, // Get the token from the stub CONSISTENCY_CHECK(isStub(pStub)); - size_t token = GetTokenFromStub(pStub); + DispatchToken token(GetTokenFromStub(pStub)); // Get the this object from ECX Object *pObj = StubManagerHelpers::GetThisPtr(pContext); @@ -1698,7 +1698,7 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock, } #endif - target = pMgr->ResolveWorker(&callSite, protectedObj, token, stubKind); + target = pMgr->ResolveWorker(&callSite, protectedObj, representativeToken, stubKind); GCPROTECT_END(); @@ -2190,7 +2190,7 @@ VirtualCallStubManager::Resolver( if (token.IsTypedToken()) { dbg_pTokenMT = GetThread()->GetDomain()->LookupType(token.GetTypeID()); - dbg_pTokenMD = dbg_pTokenMT->FindDispatchSlot(token.GetSlotNumber(), throwOnConflict).GetMethodDesc(); + dbg_pTokenMD = dbg_pTokenMT->FindDispatchSlot(TYPE_ID_THIS_CLASS, token.GetSlotNumber(), throwOnConflict).GetMethodDesc(); } #endif // _DEBUG @@ -2205,7 +2205,7 @@ VirtualCallStubManager::Resolver( MethodDesc * pMD = NULL; BOOL fShouldPatch = FALSE; - DispatchSlot implSlot(pMT->FindDispatchSlot(token, throwOnConflict)); + DispatchSlot implSlot(pMT->FindDispatchSlot(token.GetTypeID(), token.GetSlotNumber(), throwOnConflict)); // If we found a target, then just figure out if we're allowed to create a stub around // this target and backpatch the callsite. @@ -2289,7 +2289,7 @@ VirtualCallStubManager::Resolver( else if (pMT->IsComObjectType() && IsInterfaceToken(token)) { MethodTable * pItfMT = GetTypeFromToken(token); - implSlot = pItfMT->FindDispatchSlot(token.GetSlotNumber(), throwOnConflict); + implSlot = pItfMT->FindDispatchSlot(TYPE_ID_THIS_CLASS, token.GetSlotNumber(), throwOnConflict); if (pItfMT->HasInstantiation()) { @@ -2360,7 +2360,7 @@ VirtualCallStubManager::Resolver( if (token.IsTypedToken()) { pTokenMT = GetThread()->GetDomain()->LookupType(token.GetTypeID()); - pTokenMD = pTokenMT->FindDispatchSlot(token.GetSlotNumber(), throwOnConflict).GetMethodDesc(); + pTokenMD = pTokenMT->FindDispatchSlot(TYPE_ID_THIS_CLASS, token.GetSlotNumber(), throwOnConflict).GetMethodDesc(); } #ifdef FEATURE_COMINTEROP @@ -2608,14 +2608,14 @@ VirtualCallStubManager::TraceResolver( MethodTable *pMT = pObj->GetMethodTable(); CONSISTENCY_CHECK(CheckPointer(pMT)); - - DispatchSlot slot(pMT->FindDispatchSlot(token, FALSE /* throwOnConflict */)); - + DispatchSlot slot(pMT->FindDispatchSlot(token.GetTypeID(), token.GetSlotNumber(), FALSE /* throwOnConflict */)); if (slot.IsNull() && IsInterfaceToken(token) && pMT->IsComObjectType()) { MethodDesc * pItfMD = GetInterfaceMethodDescFromToken(token); CONSISTENCY_CHECK(pItfMD->GetMethodTable()->GetSlot(pItfMD->GetSlot()) == pItfMD->GetMethodEntryPoint()); - slot = pItfMD->GetMethodTable()->FindDispatchSlot(pItfMD->GetSlot(), FALSE /* throwOnConflict */); + + // Look up the slot on the interface itself. + slot = pItfMD->GetMethodTable()->FindDispatchSlot(TYPE_ID_THIS_CLASS, pItfMD->GetSlot(), FALSE /* throwOnConflict */); } // The dispatch slot's target may change due to code versioning shortly after it was retrieved above for the trace. This @@ -4157,7 +4157,7 @@ MethodDesc *VirtualCallStubManagerManager::Entry2MethodDesc( return NULL; // Do the full resolve - size_t token = VirtualCallStubManager::GetTokenFromStubQuick(pMgr, stubStartAddress, sk); + DispatchToken token(VirtualCallStubManager::GetTokenFromStubQuick(pMgr, stubStartAddress, sk)); PCODE target = NULL; // TODO: passing NULL as protectedObj here can lead to incorrect behavior for ICastable objects -- 2.7.4