From 5ba4ce4cf02772eab523206777958f56a345ac24 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 31 Aug 2020 20:02:37 -0700 Subject: [PATCH] Add assert to verify the correctness of the GCRefMap pulled from an R2R image (#41544) --- src/coreclr/src/vm/compile.cpp | 199 +----------------------- src/coreclr/src/vm/crossgen/CMakeLists.txt | 1 + src/coreclr/src/vm/frames.cpp | 237 +++++++++++++++++++++++++++++ src/coreclr/src/vm/frames.h | 6 + src/coreclr/src/vm/prestub.cpp | 7 + src/coreclr/src/vm/virtualcallstub.cpp | 16 ++ 6 files changed, 268 insertions(+), 198 deletions(-) diff --git a/src/coreclr/src/vm/compile.cpp b/src/coreclr/src/vm/compile.cpp index 492079e..6ba6118 100644 --- a/src/coreclr/src/vm/compile.cpp +++ b/src/coreclr/src/vm/compile.cpp @@ -699,207 +699,10 @@ void CEECompileInfo::GetAssemblyCodeBase(CORINFO_ASSEMBLY_HANDLE hAssembly, SStr COOPERATIVE_TRANSITION_END(); } -//================================================================================= - -void FakePromote(PTR_PTR_Object ppObj, ScanContext *pSC, uint32_t dwFlags) -{ - CONTRACTL { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - } CONTRACTL_END; - - CORCOMPILE_GCREFMAP_TOKENS newToken = (dwFlags & GC_CALL_INTERIOR) ? GCREFMAP_INTERIOR : GCREFMAP_REF; - - _ASSERTE((*(CORCOMPILE_GCREFMAP_TOKENS *)ppObj == NULL) || (*(CORCOMPILE_GCREFMAP_TOKENS *)ppObj == newToken)); - - *(CORCOMPILE_GCREFMAP_TOKENS *)ppObj = newToken; -} - -//================================================================================= - -void FakePromoteCarefully(promote_func *fn, Object **ppObj, ScanContext *pSC, uint32_t dwFlags) -{ - (*fn)(ppObj, pSC, dwFlags); -} - -//================================================================================= - -void FakeGcScanRoots(MetaSig& msig, ArgIterator& argit, MethodDesc * pMD, BYTE * pFrame) -{ - STANDARD_VM_CONTRACT; - - ScanContext sc; - - // Encode generic instantiation arg - if (argit.HasParamType()) - { - // Note that intrinsic array methods have hidden instantiation arg too, but it is not reported to GC - if (pMD->RequiresInstMethodDescArg()) - *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + argit.GetParamTypeArgOffset()) = GCREFMAP_METHOD_PARAM; - else - if (pMD->RequiresInstMethodTableArg()) - *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + argit.GetParamTypeArgOffset()) = GCREFMAP_TYPE_PARAM; - } - - // If the function has a this pointer, add it to the mask - if (argit.HasThis()) - { - BOOL interior = pMD->GetMethodTable()->IsValueType() && !pMD->IsUnboxingStub(); - - FakePromote((Object **)(pFrame + argit.GetThisOffset()), &sc, interior ? GC_CALL_INTERIOR : 0); - } - - if (argit.IsVarArg()) - { - *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + argit.GetVASigCookieOffset()) = GCREFMAP_VASIG_COOKIE; - - // We are done for varargs - the remaining arguments are reported via vasig cookie - return; - } - - // Also if the method has a return buffer, then it is the first argument, and could be an interior ref, - // so always promote it. - if (argit.HasRetBuffArg()) - { - FakePromote((Object **)(pFrame + argit.GetRetBuffArgOffset()), &sc, GC_CALL_INTERIOR); - } - - // - // Now iterate the arguments - // - - // Cycle through the arguments, and call msig.GcScanRoots for each - int argOffset; - while ((argOffset = argit.GetNextOffset()) != TransitionBlock::InvalidOffset) - { - ArgDestination argDest(pFrame, argOffset, argit.GetArgLocDescForStructInRegs()); - msig.GcScanRoots(&argDest, &FakePromote, &sc, &FakePromoteCarefully); - } -} - void CEECompileInfo::GetCallRefMap(CORINFO_METHOD_HANDLE hMethod, GCRefMapBuilder * pBuilder, bool isDispatchCell) { -#ifdef _DEBUG - DWORD dwInitialLength = pBuilder->GetBlobLength(); - UINT nTokensWritten = 0; -#endif - MethodDesc *pMD = (MethodDesc *)hMethod; - - SigTypeContext typeContext(pMD); - PCCOR_SIGNATURE pSig; - DWORD cbSigSize; - pMD->GetSig(&pSig, &cbSigSize); - MetaSig msig(pSig, cbSigSize, pMD->GetModule(), &typeContext); - - // - // Shared default interface methods (i.e. virtual interface methods with an implementation) require - // an instantiation argument. But if we're in a situation where we haven't resolved the method yet - // we need to pretent that unresolved default interface methods are like any other interface - // methods and don't have an instantiation argument. - // See code:CEEInfo::getMethodSigInternal - // - assert(!isDispatchCell || !pMD->RequiresInstArg() || pMD->GetMethodTable()->IsInterface()); - if (pMD->RequiresInstArg() && !isDispatchCell) - { - msig.SetHasParamTypeArg(); - } - - ArgIterator argit(&msig); - - UINT nStackBytes = argit.SizeOfFrameArgumentArray(); - - // Allocate a fake stack - CQuickBytes qbFakeStack; - qbFakeStack.AllocThrows(sizeof(TransitionBlock) + nStackBytes); - memset(qbFakeStack.Ptr(), 0, qbFakeStack.Size()); - - BYTE * pFrame = (BYTE *)qbFakeStack.Ptr(); - - // Fill it in - FakeGcScanRoots(msig, argit, pMD, pFrame); - - // - // Encode the ref map - // - - UINT nStackSlots; - -#ifdef TARGET_X86 - UINT cbStackPop = argit.CbStackPop(); - pBuilder->WriteStackPop(cbStackPop / sizeof(TADDR)); - - nStackSlots = nStackBytes / sizeof(TADDR) + NUM_ARGUMENT_REGISTERS; -#else - nStackSlots = (sizeof(TransitionBlock) + nStackBytes - TransitionBlock::GetOffsetOfFirstGCRefMapSlot()) / TARGET_POINTER_SIZE; -#endif - - for (UINT pos = 0; pos < nStackSlots; pos++) - { - int ofs; - -#ifdef TARGET_X86 - ofs = (pos < NUM_ARGUMENT_REGISTERS) ? - (TransitionBlock::GetOffsetOfArgumentRegisters() + ARGUMENTREGISTERS_SIZE - (pos + 1) * sizeof(TADDR)) : - (TransitionBlock::GetOffsetOfArgs() + (pos - NUM_ARGUMENT_REGISTERS) * sizeof(TADDR)); -#else - ofs = TransitionBlock::GetOffsetOfFirstGCRefMapSlot() + pos * TARGET_POINTER_SIZE; -#endif - - CORCOMPILE_GCREFMAP_TOKENS token = *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + ofs); - - if (token != 0) - { - INDEBUG(nTokensWritten++;) - pBuilder->WriteToken(pos, token); - } - } - - // We are done - pBuilder->Flush(); - -#ifdef _DEBUG - // - // Verify that decoder produces what got encoded - // - - DWORD dwFinalLength; - PVOID pBlob = pBuilder->GetBlob(&dwFinalLength); - - UINT nTokensDecoded = 0; - - GCRefMapDecoder decoder((BYTE *)pBlob + dwInitialLength); - -#ifdef TARGET_X86 - _ASSERTE(decoder.ReadStackPop() * sizeof(TADDR) == cbStackPop); -#endif - - while (!decoder.AtEnd()) - { - int pos = decoder.CurrentPos(); - int token = decoder.ReadToken(); - - int ofs; - -#ifdef TARGET_X86 - ofs = (pos < NUM_ARGUMENT_REGISTERS) ? - (TransitionBlock::GetOffsetOfArgumentRegisters() + ARGUMENTREGISTERS_SIZE - (pos + 1) * sizeof(TADDR)) : - (TransitionBlock::GetOffsetOfArgs() + (pos - NUM_ARGUMENT_REGISTERS) * sizeof(TADDR)); -#else - ofs = TransitionBlock::GetOffsetOfFirstGCRefMapSlot() + pos * TARGET_POINTER_SIZE; -#endif - - if (token != 0) - { - _ASSERTE(*(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + ofs) == token); - nTokensDecoded++; - } - } - - // Verify that all tokens got decoded. - _ASSERTE(nTokensWritten == nTokensDecoded); -#endif // _DEBUG + ComputeCallRefMap(pMD, pBuilder, isDispatchCell); } void CEECompileInfo::CompressDebugInfo( diff --git a/src/coreclr/src/vm/crossgen/CMakeLists.txt b/src/coreclr/src/vm/crossgen/CMakeLists.txt index e8083ca..fed6067 100644 --- a/src/coreclr/src/vm/crossgen/CMakeLists.txt +++ b/src/coreclr/src/vm/crossgen/CMakeLists.txt @@ -39,6 +39,7 @@ set(VM_CROSSGEN_SOURCES ../field.cpp ../fieldmarshaler.cpp ../formattype.cpp + ../frames.cpp ../gcinfodecoder.cpp ../genericdict.cpp ../generics.cpp diff --git a/src/coreclr/src/vm/frames.cpp b/src/coreclr/src/vm/frames.cpp index 3e6ab6d..1908bc7 100644 --- a/src/coreclr/src/vm/frames.cpp +++ b/src/coreclr/src/vm/frames.cpp @@ -41,6 +41,7 @@ #define CHECK_APP_DOMAIN 0 +#if !defined(CROSSGEN_COMPILE) //----------------------------------------------------------------------- #if _DEBUG //----------------------------------------------------------------------- @@ -1976,3 +1977,239 @@ PCODE UnmanagedToManagedFrame::GetReturnAddress() return pRetAddr; } } +#endif // !CROSSGEN_COMPILE + +#ifndef DACCESS_COMPILE +//================================================================================= + +void FakePromote(PTR_PTR_Object ppObj, ScanContext *pSC, uint32_t dwFlags) +{ + CONTRACTL { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } CONTRACTL_END; + + CORCOMPILE_GCREFMAP_TOKENS newToken = (dwFlags & GC_CALL_INTERIOR) ? GCREFMAP_INTERIOR : GCREFMAP_REF; + + _ASSERTE((*(CORCOMPILE_GCREFMAP_TOKENS *)ppObj == NULL) || (*(CORCOMPILE_GCREFMAP_TOKENS *)ppObj == newToken)); + + *(CORCOMPILE_GCREFMAP_TOKENS *)ppObj = newToken; +} + +//================================================================================= + +void FakePromoteCarefully(promote_func *fn, Object **ppObj, ScanContext *pSC, uint32_t dwFlags) +{ + (*fn)(ppObj, pSC, dwFlags); +} + +//================================================================================= + +void FakeGcScanRoots(MetaSig& msig, ArgIterator& argit, MethodDesc * pMD, BYTE * pFrame) +{ + STANDARD_VM_CONTRACT; + + ScanContext sc; + + // Encode generic instantiation arg + if (argit.HasParamType()) + { + // Note that intrinsic array methods have hidden instantiation arg too, but it is not reported to GC + if (pMD->RequiresInstMethodDescArg()) + *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + argit.GetParamTypeArgOffset()) = GCREFMAP_METHOD_PARAM; + else + if (pMD->RequiresInstMethodTableArg()) + *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + argit.GetParamTypeArgOffset()) = GCREFMAP_TYPE_PARAM; + } + + // If the function has a this pointer, add it to the mask + if (argit.HasThis()) + { + BOOL interior = pMD->GetMethodTable()->IsValueType() && !pMD->IsUnboxingStub(); + + FakePromote((Object **)(pFrame + argit.GetThisOffset()), &sc, interior ? GC_CALL_INTERIOR : 0); + } + + if (argit.IsVarArg()) + { + *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + argit.GetVASigCookieOffset()) = GCREFMAP_VASIG_COOKIE; + + // We are done for varargs - the remaining arguments are reported via vasig cookie + return; + } + + // Also if the method has a return buffer, then it is the first argument, and could be an interior ref, + // so always promote it. + if (argit.HasRetBuffArg()) + { + FakePromote((Object **)(pFrame + argit.GetRetBuffArgOffset()), &sc, GC_CALL_INTERIOR); + } + + // + // Now iterate the arguments + // + + // Cycle through the arguments, and call msig.GcScanRoots for each + int argOffset; + while ((argOffset = argit.GetNextOffset()) != TransitionBlock::InvalidOffset) + { + ArgDestination argDest(pFrame, argOffset, argit.GetArgLocDescForStructInRegs()); + msig.GcScanRoots(&argDest, &FakePromote, &sc, &FakePromoteCarefully); + } +} + +bool CheckGCRefMapEqual(PTR_BYTE pGCRefMap, MethodDesc* pMD, bool isDispatchCell) +{ +#ifdef _DEBUG + GCRefMapBuilder gcRefMapNew; + ComputeCallRefMap(pMD, &gcRefMapNew, isDispatchCell); + + DWORD dwFinalLength; + PVOID pBlob = gcRefMapNew.GetBlob(&dwFinalLength); + + UINT nTokensDecoded = 0; + + GCRefMapDecoder decoderNew((BYTE *)pBlob); + GCRefMapDecoder decoderExisting(pGCRefMap); + +#ifdef TARGET_X86 + _ASSERTE(decoderNew.ReadStackPop() == decoderExisting.ReadStackPop()); +#endif + + _ASSERTE(decoderNew.AtEnd() == decoderExisting.AtEnd()); + while (!decoderNew.AtEnd()) + { + _ASSERTE(decoderNew.CurrentPos() == decoderExisting.CurrentPos()); + _ASSERTE(decoderNew.ReadToken() == decoderExisting.ReadToken()); + _ASSERTE(decoderNew.AtEnd() == decoderExisting.AtEnd()); + } +#endif + return true; +} + +void ComputeCallRefMap(MethodDesc* pMD, + GCRefMapBuilder * pBuilder, + bool isDispatchCell) +{ +#ifdef _DEBUG + DWORD dwInitialLength = pBuilder->GetBlobLength(); + UINT nTokensWritten = 0; +#endif + + SigTypeContext typeContext(pMD); + PCCOR_SIGNATURE pSig; + DWORD cbSigSize; + pMD->GetSig(&pSig, &cbSigSize); + MetaSig msig(pSig, cbSigSize, pMD->GetModule(), &typeContext); + + // + // Shared default interface methods (i.e. virtual interface methods with an implementation) require + // an instantiation argument. But if we're in a situation where we haven't resolved the method yet + // we need to pretent that unresolved default interface methods are like any other interface + // methods and don't have an instantiation argument. + // See code:CEEInfo::getMethodSigInternal + // + assert(!isDispatchCell || !pMD->RequiresInstArg() || pMD->GetMethodTable()->IsInterface()); + if (pMD->RequiresInstArg() && !isDispatchCell) + { + msig.SetHasParamTypeArg(); + } + + ArgIterator argit(&msig); + + UINT nStackBytes = argit.SizeOfFrameArgumentArray(); + + // Allocate a fake stack + CQuickBytes qbFakeStack; + qbFakeStack.AllocThrows(sizeof(TransitionBlock) + nStackBytes); + memset(qbFakeStack.Ptr(), 0, qbFakeStack.Size()); + + BYTE * pFrame = (BYTE *)qbFakeStack.Ptr(); + + // Fill it in + FakeGcScanRoots(msig, argit, pMD, pFrame); + + // + // Encode the ref map + // + + UINT nStackSlots; + +#ifdef TARGET_X86 + UINT cbStackPop = argit.CbStackPop(); + pBuilder->WriteStackPop(cbStackPop / sizeof(TADDR)); + + nStackSlots = nStackBytes / sizeof(TADDR) + NUM_ARGUMENT_REGISTERS; +#else + nStackSlots = (sizeof(TransitionBlock) + nStackBytes - TransitionBlock::GetOffsetOfFirstGCRefMapSlot()) / TARGET_POINTER_SIZE; +#endif + + for (UINT pos = 0; pos < nStackSlots; pos++) + { + int ofs; + +#ifdef TARGET_X86 + ofs = (pos < NUM_ARGUMENT_REGISTERS) ? + (TransitionBlock::GetOffsetOfArgumentRegisters() + ARGUMENTREGISTERS_SIZE - (pos + 1) * sizeof(TADDR)) : + (TransitionBlock::GetOffsetOfArgs() + (pos - NUM_ARGUMENT_REGISTERS) * sizeof(TADDR)); +#else + ofs = TransitionBlock::GetOffsetOfFirstGCRefMapSlot() + pos * TARGET_POINTER_SIZE; +#endif + + CORCOMPILE_GCREFMAP_TOKENS token = *(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + ofs); + + if (token != 0) + { + INDEBUG(nTokensWritten++;) + pBuilder->WriteToken(pos, token); + } + } + + // We are done + pBuilder->Flush(); + +#ifdef _DEBUG + // + // Verify that decoder produces what got encoded + // + + DWORD dwFinalLength; + PVOID pBlob = pBuilder->GetBlob(&dwFinalLength); + + UINT nTokensDecoded = 0; + + GCRefMapDecoder decoder((BYTE *)pBlob + dwInitialLength); + +#ifdef TARGET_X86 + _ASSERTE(decoder.ReadStackPop() * sizeof(TADDR) == cbStackPop); +#endif + + while (!decoder.AtEnd()) + { + int pos = decoder.CurrentPos(); + int token = decoder.ReadToken(); + + int ofs; + +#ifdef TARGET_X86 + ofs = (pos < NUM_ARGUMENT_REGISTERS) ? + (TransitionBlock::GetOffsetOfArgumentRegisters() + ARGUMENTREGISTERS_SIZE - (pos + 1) * sizeof(TADDR)) : + (TransitionBlock::GetOffsetOfArgs() + (pos - NUM_ARGUMENT_REGISTERS) * sizeof(TADDR)); +#else + ofs = TransitionBlock::GetOffsetOfFirstGCRefMapSlot() + pos * TARGET_POINTER_SIZE; +#endif + + if (token != 0) + { + _ASSERTE(*(CORCOMPILE_GCREFMAP_TOKENS *)(pFrame + ofs) == token); + nTokensDecoded++; + } + } + + // Verify that all tokens got decoded. + _ASSERTE(nTokensWritten == nTokensDecoded); +#endif // _DEBUG +} + +#endif // !DACCESS_COMPILE diff --git a/src/coreclr/src/vm/frames.h b/src/coreclr/src/vm/frames.h index d37ba38..d280e72 100644 --- a/src/coreclr/src/vm/frames.h +++ b/src/coreclr/src/vm/frames.h @@ -3463,6 +3463,12 @@ public: #define ASSUME_BYREF_FROM_JIT_STACK_END() #endif //defined (_DEBUG) && !defined (DACCESS_COMPILE) +void ComputeCallRefMap(MethodDesc* pMD, + GCRefMapBuilder * pBuilder, + bool isDispatchCell); + +bool CheckGCRefMapEqual(PTR_BYTE pGCRefMap, MethodDesc* pMD, bool isDispatchCell); + //------------------------------------------------------------------------ #if defined(FRAMES_TURNED_FPO_ON) diff --git a/src/coreclr/src/vm/prestub.cpp b/src/coreclr/src/vm/prestub.cpp index d44c3cc..187b803 100644 --- a/src/coreclr/src/vm/prestub.cpp +++ b/src/coreclr/src/vm/prestub.cpp @@ -2668,6 +2668,13 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl pCode = pMD->GetMethodEntryPoint(); +#if _DEBUG + if (pEMFrame->GetGCRefMap() != NULL) + { + _ASSERTE(CheckGCRefMapEqual(pEMFrame->GetGCRefMap(), pMD, false)); + } +#endif // _DEBUG + // // Note that we do not want to call code:MethodDesc::IsPointingToPrestub() here. It does not take remoting // interception into account and so it would cause otherwise intercepted methods to be JITed. It is a compat diff --git a/src/coreclr/src/vm/virtualcallstub.cpp b/src/coreclr/src/vm/virtualcallstub.cpp index 1f05aef..078b4ee 100644 --- a/src/coreclr/src/vm/virtualcallstub.cpp +++ b/src/coreclr/src/vm/virtualcallstub.cpp @@ -1335,6 +1335,14 @@ extern "C" PCODE STDCALL StubDispatchFixupWorker(TransitionBlock * pTransitionBl pTarget = pMgr->ResolveWorker(&callSite, protectedObj, token, VirtualCallStubManager::SK_LOOKUP); _ASSERTE(pTarget != NULL); +#if _DEBUG + if (pSDFrame->GetGCRefMap() != NULL) + { + GCX_PREEMP(); + _ASSERTE(CheckGCRefMapEqual(pSDFrame->GetGCRefMap(), pSDFrame->GetFunction(), true)); + } +#endif // _DEBUG + // Ready to return UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; @@ -1652,6 +1660,14 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock, target = pMgr->ResolveWorker(&callSite, protectedObj, representativeToken, stubKind); +#if _DEBUG + if (pSDFrame->GetGCRefMap() != NULL) + { + GCX_PREEMP(); + _ASSERTE(CheckGCRefMapEqual(pSDFrame->GetGCRefMap(), pSDFrame->GetFunction(), true)); + } +#endif // _DEBUG + GCPROTECT_END(); UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; -- 2.7.4