From f9d2ad35195da4f6df2514c629c5db6caebb85e0 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Sun, 27 Feb 2022 20:21:14 -0800 Subject: [PATCH 01/11] Update various comments and logging --- src/coreclr/debug/ee/amd64/amd64walker.cpp | 4 +- src/coreclr/debug/ee/controller.cpp | 53 ++++++++++------------ src/coreclr/debug/ee/debugger.cpp | 16 ++----- src/coreclr/debug/ee/functioninfo.cpp | 2 +- src/coreclr/debug/ee/rcthread.cpp | 6 +-- 5 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/coreclr/debug/ee/amd64/amd64walker.cpp b/src/coreclr/debug/ee/amd64/amd64walker.cpp index 16807eedc1b77d..98365382afeaa2 100644 --- a/src/coreclr/debug/ee/amd64/amd64walker.cpp +++ b/src/coreclr/debug/ee/amd64/amd64walker.cpp @@ -33,7 +33,7 @@ void NativeWalker::Decode() BYTE rex = NULL; - LOG((LF_CORDB, LL_INFO100000, "NW:Decode: m_ip 0x%x\n", m_ip)); + LOG((LF_CORDB, LL_INFO100000, "NW:Decode: m_ip 0x%p\n", m_ip)); BYTE prefix = *ip; if (prefix == 0xcc) @@ -103,7 +103,7 @@ void NativeWalker::Decode() // Read the opcode m_opcode = *ip++; - LOG((LF_CORDB, LL_INFO100000, "NW:Decode: ip 0x%x, m_opcode:%0.2x\n", ip, m_opcode)); + LOG((LF_CORDB, LL_INFO100000, "NW:Decode: ip 0x%p, m_opcode:%0.2x\n", ip, m_opcode)); // Don't remove this, when we did the check above for the prefix we didn't modify the codestream // and since m_opcode was just taken directly from the code stream it will be patched if we diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index f9304d16ab0702..af087a7f819416 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -592,8 +592,8 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll _ASSERTE(kind == PATCH_KIND_NATIVE_MANAGED || kind == PATCH_KIND_NATIVE_UNMANAGED); LOG((LF_CORDB,LL_INFO10000,"DCP:AddPatchForAddress bound " - "absolute to 0x%x with dji 0x%x (mdDef:0x%x) " - "controller:0x%x AD:0x%x\n", + "absolute to 0x%p with dji 0x%p (mdDef:0x%x) " + "controller:0x%xp AD:0x%p\n", address, dji, (fd!=NULL?fd->GetMemberDef():0), controller, pAppDomain)); @@ -2094,7 +2094,7 @@ BOOL DebuggerController::AddBindAndActivatePatchForMethodDesc(MethodDesc *fd, ControllerLockHolder ch; LOG((LF_CORDB|LF_ENC,LL_INFO10000,"DC::AP: Add to %s::%s, at offs 0x%x " - "fp:0x%x AD:0x%x\n", fd->m_pszDebugClassName, + "fp:0x%p AD:0x%p\n", fd->m_pszDebugClassName, fd->m_pszDebugMethodName, nativeOffset, fp.GetSPValue(), pAppDomain)); @@ -3222,7 +3222,7 @@ void DebuggerController::DisableSingleStep() // void DebuggerController::ApplyTraceFlag(Thread *thread) { - LOG((LF_CORDB,LL_INFO1000, "DC::ApplyTraceFlag thread:0x%x [0x%0x]\n", thread, Debugger::GetThreadIdHelper(thread))); + LOG((LF_CORDB,LL_INFO1000, "DC::ApplyTraceFlag thread:0x%p [0x%0x]\n", thread, Debugger::GetThreadIdHelper(thread))); CONTEXT *context; if(thread->GetInteropDebuggingHijacked()) @@ -3241,7 +3241,6 @@ void DebuggerController::ApplyTraceFlag(Thread *thread) LOG((LF_CORDB,LL_INFO1000, "DC::ApplyTraceFlag marked thread for debug stepping\n")); SetSSFlag(reinterpret_cast(context) ARM_ARG(thread) ARM64_ARG(thread)); - LOG((LF_CORDB,LL_INFO1000, "DC::ApplyTraceFlag Leaving, baby!\n")); } // @@ -3251,7 +3250,7 @@ void DebuggerController::ApplyTraceFlag(Thread *thread) void DebuggerController::UnapplyTraceFlag(Thread *thread) { - LOG((LF_CORDB,LL_INFO1000, "DC::UnapplyTraceFlag thread:0x%x\n", thread)); + LOG((LF_CORDB,LL_INFO1000, "DC::UnapplyTraceFlag thread:0x%p\n", thread)); // Either this is the helper thread, or we're manipulating our own context. @@ -5107,7 +5106,7 @@ bool DebuggerStepper::ShouldContinueStep( ControllerStackInfo *info, bool DebuggerStepper::IsRangeAppropriate(ControllerStackInfo *info) { - LOG((LF_CORDB,LL_INFO10000, "DS::IRA: info:0x%x \n", info)); + LOG((LF_CORDB,LL_INFO10000, "DS::IRA: info:0x%p \n", info)); if (m_range == NULL) { LOG((LF_CORDB,LL_INFO10000, "DS::IRA: m_range == NULL, returning FALSE\n")); @@ -5129,8 +5128,8 @@ bool DebuggerStepper::IsRangeAppropriate(ControllerStackInfo *info) realFrame = &(info->m_activeFrame); } - LOG((LF_CORDB,LL_INFO10000, "DS::IRA: info->m_activeFrame.fp:0x%x m_fp:0x%x\n", info->m_activeFrame.fp, m_fp)); - LOG((LF_CORDB,LL_INFO10000, "DS::IRA: m_fdException:0x%x realFrame->md:0x%x realFrame->fp:0x%x m_fpException:0x%x\n", + LOG((LF_CORDB,LL_INFO10000, "DS::IRA: info->m_activeFrame.fp:0x%p m_fp:0x%p\n", info->m_activeFrame.fp, m_fp)); + LOG((LF_CORDB,LL_INFO10000, "DS::IRA: m_fdException:0x%p realFrame->md:0x%p realFrame->fp:0x%p m_fpException:0x%p\n", m_fdException, realFrame->md, realFrame->fp, m_fpException)); if ( (info->m_activeFrame.fp == m_fp) || ( (m_fdException != NULL) && (realFrame->md == m_fdException) && @@ -5179,7 +5178,7 @@ bool DebuggerStepper::IsRangeAppropriate(ControllerStackInfo *info) bool DebuggerStepper::IsInRange(SIZE_T ip, COR_DEBUG_STEP_RANGE *range, SIZE_T rangeCount, ControllerStackInfo *pInfo) { - LOG((LF_CORDB,LL_INFO10000,"DS::IIR: off=0x%x\n", ip)); + LOG((LF_CORDB,LL_INFO10000,"DS::IIR: off=0x%p\n", ip)); if (range == NULL) { @@ -5204,7 +5203,7 @@ bool DebuggerStepper::IsInRange(SIZE_T ip, COR_DEBUG_STEP_RANGE *range, SIZE_T r if (ip >= r->startOffset && ip < endOffset) { - LOG((LF_CORDB,LL_INFO1000,"DS::IIR:this:0x%x Found native offset " + LOG((LF_CORDB,LL_INFO1000,"DS::IIR:this:0x%p Found native offset " "0x%x to be in the range" "[0x%x, 0x%x), index 0x%x\n\n", this, ip, r->startOffset, endOffset, ((r-range)/sizeof(COR_DEBUG_STEP_RANGE *)) )); @@ -5227,7 +5226,7 @@ bool DebuggerStepper::IsInRange(SIZE_T ip, COR_DEBUG_STEP_RANGE *range, SIZE_T r bool DebuggerStepper::DetectHandleInterceptors(ControllerStackInfo *info) { LOG((LF_CORDB,LL_INFO10000,"DS::DHI: Start DetectHandleInterceptors\n")); - LOG((LF_CORDB,LL_INFO10000,"DS::DHI: active frame=0x%08x, has return frame=%d, return frame=0x%08x m_reason:%d\n", + LOG((LF_CORDB,LL_INFO10000,"DS::DHI: active frame=0x%p, has return frame=%d, return frame=0x%p m_reason:%d\n", info->m_activeFrame.frame, info->HasReturnFrame(), info->GetReturnFrame().frame, m_reason)); // If this is a normal step, then we want to continue stepping, even if we @@ -5255,7 +5254,7 @@ bool DebuggerStepper::DetectHandleInterceptors(ControllerStackInfo *info) } else { - LOG((LF_CORDB,LL_INFO10000,"DS::DHI: 0x%x set to STEP_INTERCEPT\n", this)); + LOG((LF_CORDB,LL_INFO10000,"DS::DHI: 0x%p set to STEP_INTERCEPT\n", this)); m_reason = STEP_INTERCEPT; //remember why we've stopped } @@ -5547,7 +5546,7 @@ bool DebuggerStepper::TrapStepInto(ControllerStackInfo *info, if (IsCloserToRoot(info->m_activeFrame.fp, m_fpStepInto)) m_fpStepInto = info->m_activeFrame.fp; - LOG((LF_CORDB, LL_INFO1000, "Ds::TSI this:0x%x m_fpStepInto:0x%x\n", + LOG((LF_CORDB, LL_INFO1000, "DS::TSI this:0x%p m_fpStepInto:0x%p\n", this, m_fpStepInto.GetSPValue())); TraceDestination trace; @@ -5867,8 +5866,6 @@ bool DebuggerStepper::TrapStep(ControllerStackInfo *info, bool in) // assume our context is bogus. if (fIsActiveFrameLive) { - LOG((LF_CORDB,LL_INFO10000, "DC::TS: immediate?\n")); - // Note that by definition our walker must always be able to step // through a single instruction, so any return // of NULL IP's from those cases on the first step @@ -5980,7 +5977,7 @@ bool DebuggerStepper::TrapStep(ControllerStackInfo *info, bool in) } if (walker.GetSkipIP() == NULL) { - LOG((LF_CORDB,LL_INFO10000,"DS::TS 0x%x m_reason = STEP_CALL (skip)\n", + LOG((LF_CORDB,LL_INFO10000,"DS::TS 0x%p m_reason = STEP_CALL (skip)\n", this)); m_reason = STEP_CALL; @@ -5994,8 +5991,8 @@ bool DebuggerStepper::TrapStep(ControllerStackInfo *info, bool in) case WALK_UNKNOWN: LWALK_UNKNOWN: - LOG((LF_CORDB,LL_INFO10000,"DS::TS:WALK_UNKNOWN - curIP:0x%x " - "nextIP:0x%x skipIP:0x%x 1st byte of opcode:0x%x\n", (BYTE*)GetControlPC(&(info->m_activeFrame. + LOG((LF_CORDB,LL_INFO10000,"DS::TS:WALK_UNKNOWN - curIP:0x%p " + "nextIP:0x%p skipIP:0x%p 1st byte of opcode:0x%x\n", (BYTE*)GetControlPC(&(info->m_activeFrame. registers)), walker.GetNextIP(),walker.GetSkipIP(), *(BYTE*)GetControlPC(&(info->m_activeFrame.registers)))); @@ -6145,7 +6142,7 @@ bool DebuggerStepper::TrapStep(ControllerStackInfo *info, bool in) info->GetReturnFrame().fp, NULL); - LOG((LF_CORDB,LL_INFO10000,"DS 0x%x m_reason=STEP_CALL4\n",this)); + LOG((LF_CORDB,LL_INFO10000,"DS 0x%p m_reason=STEP_CALL4\n",this)); m_reason = STEP_CALL; return true; @@ -6716,7 +6713,7 @@ bool DebuggerStepper::SetRangesFromIL(DebuggerJitInfo *dji, COR_DEBUG_STEP_RANGE if (dji != NULL) { - LOG((LF_CORDB,LL_INFO10000,"DeSt::St: For code md=0x%x, got DJI 0x%x, from 0x%x to 0x%x\n", + LOG((LF_CORDB,LL_INFO10000,"DeSt::St: For code md=0x%p, got DJI 0x%p, from 0x%p to 0x%p\n", fd, dji, dji->m_addrOfCode, (ULONG)dji->m_addrOfCode + (ULONG)dji->m_sizeOfCode)); @@ -6981,7 +6978,7 @@ bool DebuggerStepper::Step(FramePointer fp, bool in, } m_eMode = m_stepIn ? cStepIn : cStepOver; - LOG((LF_CORDB,LL_INFO10000,"DS 0x%x STep: STEP_NORMAL\n",this)); + LOG((LF_CORDB,LL_INFO10000,"DS 0x%p Step: STEP_NORMAL\n",this)); m_reason = STEP_NORMAL; //assume it'll be a normal step & set it to //something else if we walk over it if (fIsILStub) @@ -7035,7 +7032,7 @@ TP_RESULT DebuggerStepper::TriggerPatch(DebuggerControllerPatch *patch, Thread *thread, TRIGGER_WHY tyWhy) { - LOG((LF_CORDB, LL_INFO10000, "DeSt::TP\n")); + LOG((LF_CORDB, LL_INFO10000, "DS::TP\n")); // If we're frozen, we may hit a patch but we just ignore it if (IsFrozen()) @@ -7221,7 +7218,7 @@ TP_RESULT DebuggerStepper::TriggerPatch(DebuggerControllerPatch *patch, m_reason = STEP_NORMAL; //we tried to do a STEP_CALL, but since it didn't //work, we're doing what amounts to a normal step. - LOG((LF_CORDB,LL_INFO10000,"DS 0x%x m_reason = STEP_NORMAL" + LOG((LF_CORDB,LL_INFO10000,"DS 0x%p m_reason = STEP_NORMAL" "(attempted call thru stub manager, SM didn't know where" " we're going, so did a step out to original call\n",this)); } @@ -7337,13 +7334,13 @@ void DebuggerStepper::TriggerMethodEnter(Thread * thread, _ASSERTE(!IsFrozen()); MethodDesc * pDesc = dji->m_nativeCodeVersion.GetMethodDesc(); - LOG((LF_CORDB, LL_INFO10000, "DJMCStepper::TME, desc=%p, addr=%p\n", + LOG((LF_CORDB, LL_INFO10000, "DebuggerStepper::TME, desc=%p, addr=%p\n", pDesc, ip)); - // JMC steppers won't stop in Lightweight delegates. Just return & keep executing. + // JMC steppers won't stop in Lightweight codegen (LCG). Just return & keep executing. if (pDesc->IsNoMetadata()) { - LOG((LF_CORDB, LL_INFO100000, "DJMCStepper::TME, skipping b/c it's lw-codegen\n")); + LOG((LF_CORDB, LL_INFO100000, "DebuggerStepper::TME, skipping b/c it's dynamic code (LCG)\n")); return; } @@ -7450,7 +7447,7 @@ bool DebuggerStepper::TriggerSingleStep(Thread *thread, const BYTE *ip) } // If we EnC the method, we'll blast the function address, - // and so have to get it from teh DJI that we'll have. If + // and so have to get it from the DJI that we'll have. If // we haven't gotten debugger info about a regular function, then // we'll have to get the info from the EE, which will be valid // since we're standing in the function at this point, and diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index bd96c50fc59d31..eac99b70fca06b 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -2730,16 +2730,8 @@ DebuggerJitInfo *Debugger::GetJitInfoWorker(MethodDesc *fd, const BYTE *pbAddr, break; } } - LOG((LF_CORDB, LL_INFO1000, "D::GJI: for md:0x%x (%s::%s), got dmi:0x%x.\n", - fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, - dmi)); - - - - // Log stuff - fd may be null; so we don't want to AV in the log. - - LOG((LF_CORDB, LL_INFO1000, "D::GJI: for md:0x%x (%s::%s), got dmi:0x%x, dji:0x%x, latest dji:0x%x, latest fd:0x%x, prev dji:0x%x\n", + LOG((LF_CORDB, LL_INFO1000, "D::GJI: for md:0x%p (%s::%s), got dmi:0x%p, dji:0x%p, latest dji:0x%p, latest fd:0x%p, prev dji:0x%p\n", fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, dmi, dji, (dmi ? dmi->GetLatestJitInfo_NoCreate() : 0), ((dmi && dmi->GetLatestJitInfo_NoCreate()) ? dmi->GetLatestJitInfo_NoCreate()->m_nativeCodeVersion.GetMethodDesc():0), @@ -5467,7 +5459,7 @@ void Debugger::ReleaseAllRuntimeThreads(AppDomain *pAppDomain) //@todo APPD if we want true isolation, remove this & finish the work pAppDomain = NULL; - STRESS_LOG1(LF_CORDB, LL_INFO10000, "D::RART: Releasing all Runtime threads" + STRESS_LOG1(LF_CORDB, LL_INFO10000, "D::RART: Releasing all Runtime threads " "for AppD 0x%x.\n", pAppDomain); // Mark that we're on our way now... @@ -5679,8 +5671,6 @@ void Debugger::TraceCall(const BYTE *code) * Invoked from a probe in managed code when we enter a user method and * the flag (set by GetJMCFlagAddr) for that method is != 0. * pIP - the ip within the method, right after the prolog. - * sp - stack pointer (frame pointer on x86) for the managed method we're entering. - * bsp - backing store pointer for the managed method we're entering ******************************************************************************/ void Debugger::OnMethodEnter(void * pIP) { @@ -5695,7 +5685,7 @@ void Debugger::OnMethodEnter(void * pIP) if (!CORDebuggerAttached()) { - LOG((LF_CORDB, LL_INFO1000000, "D::OnMethodEnter returning since debugger attached.\n")); + LOG((LF_CORDB, LL_INFO1000000, "D::OnMethodEnter returning since debugger not attached.\n")); return; } FramePointer fp = LEAF_MOST_FRAME; diff --git a/src/coreclr/debug/ee/functioninfo.cpp b/src/coreclr/debug/ee/functioninfo.cpp index bba30942271855..55845f934cae7b 100644 --- a/src/coreclr/debug/ee/functioninfo.cpp +++ b/src/coreclr/debug/ee/functioninfo.cpp @@ -2407,7 +2407,7 @@ DebuggerMethodInfo *DebuggerMethodInfoTable::GetMethodInfo(Module *pModule, mdMe } else { - LOG((LF_CORDB, LL_INFO1000, "DMI::GMI: for methodDef 0x%x, got 0x%x prev:0x%x\n", + LOG((LF_CORDB, LL_INFO1000, "DMI::GMI: for methodDef 0x%x, got 0x%p prev:0x%p\n", token, entry->mi, (entry->mi?entry->mi->m_prevMethodInfo:0))); return entry->mi; } diff --git a/src/coreclr/debug/ee/rcthread.cpp b/src/coreclr/debug/ee/rcthread.cpp index 961b06c879f952..6b4cc79a1944e3 100644 --- a/src/coreclr/debug/ee/rcthread.cpp +++ b/src/coreclr/debug/ee/rcthread.cpp @@ -592,7 +592,7 @@ static LONG _debugFilter(LPEXCEPTION_POINTERS ep, PVOID pv) EX_END_CATCH(RethrowTerminalExceptions); CONSISTENCY_CHECK_MSGF(false, - ("Unhandled exception on the helper thread.\nEvent=%s(0x%x)\nCode=0x%0x, Ip=0x%p, .cxr=%p, .exr=%p.\n pid=0x%x (%d), tid=0x%x (%d).\n-----\nStack of exception:\n%s\n----\n", + ("Unhandled exception on the helper thread.\nEvent=%s(0x%p)\nCode=0x%0x, Ip=0x%p, .cxr=%p, .exr=%p.\n pid=0x%x (%d), tid=0x%x (%d).\n-----\nStack of exception:\n%s\n----\n", IPCENames::GetName(type), type, ep->ExceptionRecord->ExceptionCode, GetIP(ep->ContextRecord), ep->ContextRecord, ep->ExceptionRecord, pid, pid, tid, tid, @@ -1573,7 +1573,7 @@ HRESULT DebuggerRCThread::SendIPCEvent() DebuggerIPCEvent* pManagedEvent = GetIPCEventSendBuffer(); - STRESS_LOG2(LF_CORDB, LL_INFO1000, "D::SendIPCEvent %s to outofproc appD 0x%x,\n", + STRESS_LOG2(LF_CORDB, LL_INFO1000, "D::SendIPCEvent %s to outofproc appD 0x%p,\n", IPCENames::GetName(pManagedEvent->type), VmPtrToCookie(pManagedEvent->vmAppDomain)); @@ -1611,7 +1611,7 @@ bool DebuggerRCThread::IsRCThreadReady() // leaving the threadid still non-0. So check the actual thread object // and make sure it's still around. int ret = WaitForSingleObject(m_thread, 0); - LOG((LF_CORDB, LL_EVERYTHING, "DRCT::IsReady - wait(0x%x)=%d, GetLastError() = %d\n", m_thread, ret, GetLastError())); + LOG((LF_CORDB, LL_EVERYTHING, "DRCT::IsReady - wait(0x%p)=%d, GetLastError() = %d\n", m_thread, ret, GetLastError())); if (ret != WAIT_TIMEOUT) { From 83d8110536aeaf21e372c3d4e360107da9a79cc3 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Sun, 27 Feb 2022 20:24:37 -0800 Subject: [PATCH 02/11] The StubManager is now aware of InstantiatingMethods in IL Stub and StubLink scenarios Created the concept of a "StepThroughStub". This handles cases where the stub should be treated like as an implementation details and simply forward to the appropriate target. Updates to various logging mechanisms. --- src/coreclr/debug/daccess/fntableaccess.h | 4 +- src/coreclr/vm/common.h | 1 + src/coreclr/vm/ilstubcache.cpp | 4 +- src/coreclr/vm/method.hpp | 35 ++++- src/coreclr/vm/prestub.cpp | 6 +- src/coreclr/vm/stublink.cpp | 38 +++-- src/coreclr/vm/stublink.h | 97 ++++++++----- src/coreclr/vm/stubmgr.cpp | 167 ++++++++++++++-------- src/coreclr/vm/stubmgr.h | 4 - 9 files changed, 234 insertions(+), 122 deletions(-) diff --git a/src/coreclr/debug/daccess/fntableaccess.h b/src/coreclr/debug/daccess/fntableaccess.h index 690056d005a1e9..b89880caf93c3c 100644 --- a/src/coreclr/debug/daccess/fntableaccess.h +++ b/src/coreclr/debug/daccess/fntableaccess.h @@ -110,7 +110,7 @@ struct FakeStubUnwindInfoHeapSegment struct FakeStub { ULONG m_refcount; - ULONG m_patchOffset; + ULONG m_Offset; UINT m_numCodeBytes; #ifdef _DEBUG @@ -168,7 +168,7 @@ class CheckDuplicatedStructLayouts CHECK_OFFSET(Stub, m_refcount); - CHECK_OFFSET(Stub, m_patchOffset); + CHECK_OFFSET(Stub, m_Offset); CHECK_OFFSET(Stub, m_numCodeBytes); #ifdef _DEBUG CHECK_OFFSET(Stub, m_signature); diff --git a/src/coreclr/vm/common.h b/src/coreclr/vm/common.h index eb37c0a0cac94b..b47119e51d83f1 100644 --- a/src/coreclr/vm/common.h +++ b/src/coreclr/vm/common.h @@ -138,6 +138,7 @@ typedef VPTR(struct IUnknown) PTR_IUnknown; typedef DPTR(class InstMethodHashTable) PTR_InstMethodHashTable; typedef DPTR(class MetaSig) PTR_MetaSig; typedef DPTR(class MethodDesc) PTR_MethodDesc; +typedef DPTR(PTR_MethodDesc) PTR_PTR_MethodDesc; typedef DPTR(class MethodDescChunk) PTR_MethodDescChunk; typedef DPTR(class MethodImpl) PTR_MethodImpl; typedef DPTR(class MethodTable) PTR_MethodTable; diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index df989b8d9ef972..3f689bbaf47e36 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -202,6 +202,7 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa #ifdef FEATURE_ARRAYSTUB_AS_IL if (SF_IsArrayOpStub(dwStubFlags)) { + pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdStepThroughStub; pMD->GetILStubResolver()->SetStubType(ILStubResolver::ArrayOpStub); } else @@ -223,12 +224,13 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa #ifdef FEATURE_INSTANTIATINGSTUB_AS_IL if (SF_IsUnboxingILStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdUnboxingILStub; + pMD->m_dwExtendedFlags |= (DynamicMethodDesc::nomdUnboxingILStub | DynamicMethodDesc::nomdStepThroughStub); pMD->GetILStubResolver()->SetStubType(ILStubResolver::UnboxingILStub); } else if (SF_IsInstantiatingStub(dwStubFlags)) { + pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdStepThroughStub; pMD->GetILStubResolver()->SetStubType(ILStubResolver::InstantiatingStub); } else diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 79d6fd612ff766..c5d82f32c773ce 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2495,7 +2495,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc nomdDelegateStub = 0x0040, nomdStructMarshalStub = 0x0080, nomdUnbreakable = 0x0100, - //unused = 0x0200, + nomdStepThroughStub = 0x0200, // Stub has no user steppable code and should be stepped through. //unused = 0x0400, nomdStubNeedsCOMStarted = 0x0800, // EnsureComStarted must be called before executing the method nomdMulticastStub = 0x1000, @@ -2573,12 +2573,39 @@ class DynamicMethodDesc : public StoredSigMethodDesc bool IsUnmanagedCallersOnlyStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdUnmanagedCallersOnlyStub)); } bool IsCALLIStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdCALLIStub)); } bool IsDelegateStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdDelegateStub)); } - bool IsCLRToCOMStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return ((0 == (m_dwExtendedFlags & mdStatic)) && !IsReverseStub() && !IsDelegateStub() && !IsStructMarshalStub()); } - bool IsCOMToCLRStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return ((0 == (m_dwExtendedFlags & mdStatic)) && IsReverseStub()); } - bool IsPInvokeStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return ((0 != (m_dwExtendedFlags & mdStatic)) && !IsReverseStub() && !IsCALLIStub() && !IsStructMarshalStub()); } bool IsUnbreakable() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdUnbreakable)); } bool IsStubNeedsCOMStarted() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStubNeedsCOMStarted)); } bool IsStructMarshalStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStructMarshalStub)); } + bool IsStepThroughStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStepThroughStub)); } + + // The following checks don't have explicit indicates of type, but rather use a heuristic. + // Therefore, we break them out so they are clearer. + bool IsCLRToCOMStub() + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsILStub()); + return ((0 == (m_dwExtendedFlags & mdStatic)) + && !IsReverseStub() + && !IsDelegateStub() + && !IsStructMarshalStub()); + } + bool IsCOMToCLRStub() + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsILStub()); + return ((0 == (m_dwExtendedFlags & mdStatic)) + && IsReverseStub()); + } + bool IsPInvokeStub() + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsILStub()); + return ((0 != (m_dwExtendedFlags & mdStatic)) + && !IsReverseStub() + && !IsCALLIStub() + && !IsStructMarshalStub()); + } + #ifdef FEATURE_MULTICASTSTUB_AS_IL bool IsMulticastStub() { LIMITED_METHOD_DAC_CONTRACT; diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 4d8d954891ce7e..32f081b2eaa891 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -1682,7 +1682,8 @@ Stub * MakeUnboxingStubWorker(MethodDesc *pMD) sl.EmitComputedInstantiatingMethodStub(pUnboxedMD, &portableShuffle[0], NULL); - pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap()); + pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_INSTANTIATING_METHOD); + pstub->SetMethodDesc(pUnboxedMD); } else #endif @@ -1755,7 +1756,8 @@ Stub * MakeInstantiatingStubWorker(MethodDesc *pMD) _ASSERTE(pSharedMD != NULL && pSharedMD != pMD); sl.EmitComputedInstantiatingMethodStub(pSharedMD, &portableShuffle[0], extraArg); - pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap()); + pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_INSTANTIATING_METHOD); + pstub->SetMethodDesc(pSharedMD); } else #endif diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index a2070a558403f0..75c63c27b7cd78 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -1087,10 +1087,10 @@ bool StubLinker::EmitStub(Stub* pStub, int globalsize, int totalSize, LoaderHeap { UINT32 uLabelOffset = GetLabelOffset(m_pPatchLabel); _ASSERTE(FitsIn(uLabelOffset)); - pStubRW->SetPatchOffset(static_cast(uLabelOffset)); + pStubRW->SetOffset(static_cast(uLabelOffset)); LOG((LF_CORDB, LL_INFO100, "SL::ES: patch offset:0x%x\n", - pStub->GetPatchOffset())); + pStub->GetOffset())); } #ifdef STUBLINKER_GENERATES_UNWIND_INFO @@ -2004,7 +2004,7 @@ VOID Stub::DeleteStub() // a size of 0 is a signal to Nirvana to flush the entire cache //FlushInstructionCache(GetCurrentProcess(),0,0); - if ((m_patchOffset & LOADER_HEAP_BIT) == 0) + if ((m_Offset & LOADER_HEAP_BIT) == 0) { #ifdef _DEBUG m_signature = kFreedStub; @@ -2110,6 +2110,10 @@ Stub* Stub::NewStub(PTR_VOID pCode, DWORD flags) size += numCodeBytes; } + // Add pointer to store the target MethodDesc. + if (flags & NEWSTUB_FL_INSTANTIATING_METHOD) + size += sizeof(PTR_MethodDesc); + if (size.IsOverflow()) COMPlusThrowArithmetic(); @@ -2128,7 +2132,9 @@ Stub* Stub::NewStub(PTR_VOID pCode, DWORD flags) } size_t stubPayloadOffset = totalSize - - (sizeof(Stub) + ((flags & NEWSTUB_FL_EXTERNAL) ? sizeof(PTR_PCODE) : numCodeBytes)); + (sizeof(Stub) + + ((flags & NEWSTUB_FL_EXTERNAL) ? sizeof(PTR_PCODE) : numCodeBytes) + + ((flags & NEWSTUB_FL_INSTANTIATING_METHOD) ? sizeof(PTR_MethodDesc) : 0)); // Make sure that the payload of the stub is aligned Stub* pStubRX = (Stub*)(pBlock + stubPayloadOffset); @@ -2181,19 +2187,27 @@ void Stub::SetupStub(int numCodeBytes, DWORD flags m_numCodeBytes = numCodeBytes; m_refcount = 1; - m_patchOffset = 0; + m_Offset = 0; - if((flags & NEWSTUB_FL_LOADERHEAP) != 0) - m_patchOffset |= LOADER_HEAP_BIT; - if((flags & NEWSTUB_FL_MULTICAST) != 0) - m_patchOffset |= MULTICAST_DELEGATE_BIT; - if ((flags & NEWSTUB_FL_EXTERNAL) != 0) - m_patchOffset |= EXTERNAL_ENTRY_BIT; + if (flags != NEWSTUB_FL_NONE) + { + if((flags & NEWSTUB_FL_LOADERHEAP) != 0) + m_Offset |= LOADER_HEAP_BIT; + if((flags & NEWSTUB_FL_MULTICAST) != 0) + m_Offset |= MULTICAST_DELEGATE_BIT; + if ((flags & NEWSTUB_FL_EXTERNAL) != 0) + m_Offset |= EXTERNAL_ENTRY_BIT; + if ((flags & NEWSTUB_FL_INSTANTIATING_METHOD) != 0) + { + m_Offset |= INSTANTIATING_METHOD_BIT; + SetOffset(m_numCodeBytes); + } + } #ifdef STUBLINKER_GENERATES_UNWIND_INFO if (nUnwindInfoSize) { - m_patchOffset |= UNWIND_INFO_BIT; + m_Offset |= UNWIND_INFO_BIT; StubUnwindInfoHeaderSuffix * pSuffix = GetUnwindInfoHeaderSuffix(); pSuffix->nUnwindInfoSize = (BYTE)nUnwindInfoSize; diff --git a/src/coreclr/vm/stublink.h b/src/coreclr/vm/stublink.h index 2b739de302561f..208acb3868ad70 100644 --- a/src/coreclr/vm/stublink.h +++ b/src/coreclr/vm/stublink.h @@ -443,9 +443,11 @@ struct CodeLabel enum NewStubFlags { - NEWSTUB_FL_MULTICAST = 0x00000002, - NEWSTUB_FL_EXTERNAL = 0x00000004, - NEWSTUB_FL_LOADERHEAP = 0x00000008 + NEWSTUB_FL_NONE = 0x00000000, + NEWSTUB_FL_INSTANTIATING_METHOD = 0x00000001, + NEWSTUB_FL_MULTICAST = 0x00000002, + NEWSTUB_FL_EXTERNAL = 0x00000004, + NEWSTUB_FL_LOADERHEAP = 0x00000008 }; @@ -463,18 +465,21 @@ class Stub friend class CheckAsmOffsets; protected: + + // These values are shared with the debugger - see FakeStub. enum { - MULTICAST_DELEGATE_BIT = 0x80000000, - EXTERNAL_ENTRY_BIT = 0x40000000, - LOADER_HEAP_BIT = 0x20000000, - UNWIND_INFO_BIT = 0x08000000, - - PATCH_OFFSET_MASK = UNWIND_INFO_BIT - 1, - MAX_PATCH_OFFSET = PATCH_OFFSET_MASK + 1, + MULTICAST_DELEGATE_BIT = 0x80000000, + EXTERNAL_ENTRY_BIT = 0x40000000, + LOADER_HEAP_BIT = 0x20000000, + INSTANTIATING_METHOD_BIT= 0x10000000, + UNWIND_INFO_BIT = 0x08000000, + + OFFSET_MASK = UNWIND_INFO_BIT - 1, + MAX_OFFSET = OFFSET_MASK + 1, }; - static_assert_no_msg(PATCH_OFFSET_MASK < UNWIND_INFO_BIT); + static_assert_no_msg(OFFSET_MASK < UNWIND_INFO_BIT); public: //------------------------------------------------------------------- @@ -482,15 +487,12 @@ class Stub //------------------------------------------------------------------- VOID IncRef(); - //------------------------------------------------------------------- // Dec the refcount. // Returns true if the count went to zero and the stub was deleted //------------------------------------------------------------------- BOOL DecRef(); - - //------------------------------------------------------------------- // Used for throwing out unused stubs from stub caches. This // method cannot be 100% accurate due to race conditions. This @@ -510,34 +512,61 @@ class Stub BOOL IsMulticastDelegate() { LIMITED_METHOD_CONTRACT; - return (m_patchOffset & MULTICAST_DELEGATE_BIT) != 0; + return (m_Offset & MULTICAST_DELEGATE_BIT) != 0; } //------------------------------------------------------------------- - // For stubs which execute user code, a patch offset needs to be set - // to tell the debugger how far into the stub code the debugger has - // to step until the frame is set up. + // Used by the debugger to help step through stubs //------------------------------------------------------------------- - USHORT GetPatchOffset() + BOOL IsInstantiatingMethodStub() { LIMITED_METHOD_CONTRACT; + return (m_Offset & INSTANTIATING_METHOD_BIT) != 0; + } - return (USHORT)(m_patchOffset & PATCH_OFFSET_MASK); + USHORT GetOffset() + { + LIMITED_METHOD_CONTRACT; + return (USHORT)(m_Offset & OFFSET_MASK); } - void SetPatchOffset(USHORT offset) + void SetOffset(USHORT offset) { LIMITED_METHOD_CONTRACT; - _ASSERTE(GetPatchOffset() == 0); - m_patchOffset |= offset; - _ASSERTE(GetPatchOffset() == offset); + _ASSERTE(GetOffset() == 0); + m_Offset |= offset; + _ASSERTE(GetOffset() == offset); } + void SetMethodDesc(PTR_MethodDesc pMD) + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsInstantiatingMethodStub()); + *GetMethodDescLocation() = pMD; + } + + //------------------------------------------------------------------- + // For stubs which execute user code, a patch offset needs to be set + // to tell the debugger how far into the stub code the debugger has + // to step until the frame is set up. + //------------------------------------------------------------------- TADDR GetPatchAddress() { WRAPPER_NO_CONTRACT; + _ASSERTE(!IsInstantiatingMethodStub()); + return dac_cast(GetEntryPointInternal()) + GetOffset(); + } - return dac_cast(GetEntryPointInternal()) + GetPatchOffset(); + //------------------------------------------------------------------- + // For instantiating methods, the target MethodDesc needs to be set + // to tell the debugger where to step through the instantiating method + // stub. + //------------------------------------------------------------------- + PTR_PTR_MethodDesc GetMethodDescLocation() + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsInstantiatingMethodStub()); + return dac_cast(GetEntryPointInternal() + GetOffset()); } //------------------------------------------------------------------- @@ -549,7 +578,7 @@ class Stub BOOL HasUnwindInfo() { LIMITED_METHOD_CONTRACT; - return (m_patchOffset & UNWIND_INFO_BIT) != 0; + return (m_Offset & UNWIND_INFO_BIT) != 0; } StubUnwindInfoHeaderSuffix *GetUnwindInfoHeaderSuffix() @@ -717,21 +746,21 @@ class Stub { LIMITED_METHOD_CONTRACT; - return (m_patchOffset & EXTERNAL_ENTRY_BIT) != 0; + return (m_Offset & EXTERNAL_ENTRY_BIT) != 0; } //------------------------------------------------------------------- // This creates stubs. //------------------------------------------------------------------- static Stub* NewStub(LoaderHeap *pLoaderHeap, UINT numCodeBytes, - DWORD flags = 0 + DWORD flags = NEWSTUB_FL_NONE #ifdef STUBLINKER_GENERATES_UNWIND_INFO , UINT nUnwindInfoSize = 0 #endif ); - static Stub* NewStub(PTR_VOID pCode, DWORD flags = 0); - static Stub* NewStub(PCODE pCode, DWORD flags = 0) + static Stub* NewStub(PTR_VOID pCode, DWORD flags = NEWSTUB_FL_NONE); + static Stub* NewStub(PCODE pCode, DWORD flags = NEWSTUB_FL_NONE) { return NewStub((PTR_VOID)pCode, flags); } @@ -773,7 +802,7 @@ class Stub } ULONG m_refcount; - ULONG m_patchOffset; + ULONG m_Offset; UINT m_numCodeBytes; @@ -792,11 +821,7 @@ class Stub #endif // HOST_64BIT #endif // _DEBUG -#ifdef _DEBUG - Stub() // Stubs are created by NewStub(), not "new". Hide the - { LIMITED_METHOD_CONTRACT; } // constructor to enforce this. -#endif - + Stub() = delete; // Stubs are created by NewStub(), not "new". }; diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index d08ded97c933b8..25a69dab5b5f1a 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -181,7 +181,7 @@ void TraceDestination::InitForUnjittedMethod(MethodDesc * pDesc) { pDesc = pNewDesc; - LOG((LF_CORDB, LL_INFO10000, "TD::UnjittedMethod: wrapper md: %p --> %p", pDesc, pNewDesc)); + LOG((LF_CORDB, LL_INFO10000, "TD::UnjittedMethod: wrapper md: %p --> %p\n", pDesc, pNewDesc)); } } @@ -635,7 +635,7 @@ void StubManager::AddStubManager(StubManager *mgr) g_pFirstManager = mgr; } - LOG((LF_CORDB, LL_EVERYTHING, "StubManager::AddStubManager - 0x%p (vptr %x%p)\n", mgr, (*(PVOID*)mgr))); + LOG((LF_CORDB, LL_EVERYTHING, "StubManager::AddStubManager - 0x%p (vptr %p)\n", mgr, (*(PVOID*)mgr))); } //----------------------------------------------------------- @@ -1169,13 +1169,13 @@ BOOL StubLinkStubManager::DoTraceStub(PCODE stubStartAddress, CONTRACTL_END LOG((LF_CORDB, LL_INFO10000, - "StubLinkStubManager::DoTraceStub: stubStartAddress=0x%08x\n", + "StubLinkStubManager::DoTraceStub: stubStartAddress=0x%p\n", stubStartAddress)); Stub *stub = Stub::RecoverStub(stubStartAddress); LOG((LF_CORDB, LL_INFO10000, - "StubLinkStubManager::DoTraceStub: stub=0x%08x\n", stub)); + "StubLinkStubManager::DoTraceStub: stub=0x%p\n", stub)); // // If this is an intercept stub, we may be able to step @@ -1188,39 +1188,58 @@ BOOL StubLinkStubManager::DoTraceStub(PCODE stubStartAddress, TADDR pRealAddr = 0; if (stub->IsMulticastDelegate()) { - LOG((LF_CORDB, LL_INFO10000, - "StubLinkStubManager(MCDel)::DoTraceStub: stubStartAddress=0x%08x\n", - stubStartAddress)); - - LOG((LF_CORDB, LL_INFO10000, - "StubLinkStubManager(MCDel)::DoTraceStub: stub=0x%08x MGR_PUSH to entrypoint:0x%x\n", stub, - stub->GetEntryPoint())); - // If it's a MC delegate, then we want to set a BP & do a context-ful // manager push, so that we can figure out if this call will be to a // single multicast delegate or a multi multicast delegate trace->InitForManagerPush(stubStartAddress, this); - + LOG_TRACE_DESTINATION(trace, stubStartAddress, "StubLinkStubManager(MCDel)::DoTraceStub"); return TRUE; } - else if (stub->GetPatchOffset() == 0) + else if (stub->IsInstantiatingMethodStub()) { - LOG((LF_CORDB, LL_INFO10000, "StubLinkStubManager::DoTraceStub: patch offset is 0!\n")); - - return FALSE; + trace->InitForManagerPush(stubStartAddress, this); + LOG_TRACE_DESTINATION(trace, stubStartAddress, "StubLinkStubManager(InstantiatingMethod)::DoTraceStub"); + return TRUE; } - else + else if (stub->GetOffset() != 0) { trace->InitForFramePush((PCODE)stub->GetPatchAddress()); - LOG_TRACE_DESTINATION(trace, stubStartAddress, "StubLinkStubManager::DoTraceStub"); - return TRUE; } + + LOG((LF_CORDB, LL_INFO10000, "StubLinkStubManager::DoTraceStub: patch offset is 0!\n")); + return FALSE; } #ifndef DACCESS_COMPILE +static PCODE GetStubTarget(PTR_MethodDesc pTargetMD) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } + CONTRACTL_END; + + NativeCodeVersion targetCode; + +#ifdef FEATURE_CODE_VERSIONING + CodeVersionManager::LockHolder codeVersioningLockHolder; + ILCodeVersion ilcode = pTargetMD->GetCodeVersionManager()->GetActiveILCodeVersion(pTargetMD); + targetCode = ilcode.GetActiveNativeCodeVersion(pTargetMD); +#else + targetCode = NativeCodeVersion(pTargetMD); +#endif + + if (targetCode.IsNull() || targetCode.GetNativeCode() == NULL) + return NULL; + + return targetCode.GetNativeCode(); +} + BOOL StubLinkStubManager::TraceManager(Thread *thread, TraceDestination *trace, T_CONTEXT *pContext, @@ -1236,21 +1255,36 @@ BOOL StubLinkStubManager::TraceManager(Thread *thread, } CONTRACTL_END - // NOTE that we're assuming that this will be called if and ONLY if - // we're examing a multicast delegate stub. Otherwise, we'll have to figure out - // what we're looking iat - - BYTE *pbDel = 0; - LPVOID pc = (LPVOID)GetIP(pContext); - *pRetAddr = (BYTE *)StubManagerHelpers::GetReturnAddress(pContext); + LOG((LF_CORDB,LL_INFO10000, "SLSM:TM 0x%p, retAddr is 0x%p\n", pc, (*pRetAddr))); - pbDel = (BYTE *)StubManagerHelpers::GetThisPtr(pContext); + Stub *stub = Stub::RecoverStub((PCODE)pc); + if (stub->IsInstantiatingMethodStub()) + { + LOG((LF_CORDB,LL_INFO10000, "SLSM:TM Instantiating method stub\n")); + PTR_PTR_MethodDesc ppMD = stub->GetMethodDescLocation(); + _ASSERTE(*ppMD != NULL); - LOG((LF_CORDB,LL_INFO10000, "SLSM:TM at 0x%x, retAddr is 0x%x\n", pc, (*pRetAddr))); + PCODE target = GetStubTarget(*ppMD); + if (target == NULL) + { + LOG((LF_CORDB,LL_INFO10000, "SLSM:TM Unable to determine stub target, ppMD 0x%p\n", ppMD)); + trace->InitForUnjittedMethod(*ppMD); + return TRUE; + } - return DelegateInvokeStubManager::TraceDelegateObject(pbDel, trace); + trace->InitForManaged(target); + return TRUE; + } + else if (stub->IsMulticastDelegate()) + { + LOG((LF_CORDB,LL_INFO10000, "SLSM:TM MultiCastDelegate\n")); + BYTE *pbDel = (BYTE *)StubManagerHelpers::GetThisPtr(pContext); + return DelegateInvokeStubManager::TraceDelegateObject(pbDel, trace); + } + + return FALSE; } #endif // #ifndef DACCESS_COMPILE @@ -1595,7 +1629,7 @@ BOOL ILStubManager::DoTraceStub(PCODE stubStartAddress, #ifndef DACCESS_COMPILE #ifdef FEATURE_COMINTEROP -PCODE ILStubManager::GetCOMTarget(Object *pThis, ComPlusCallInfo *pComPlusCallInfo) +static PCODE GetCOMTarget(Object *pThis, ComPlusCallInfo *pComPlusCallInfo) { CONTRACTL { @@ -1639,27 +1673,24 @@ BOOL ILStubManager::TraceManager(Thread *thread, #endif DynamicMethodDesc *pStubMD = Entry2MethodDesc(stubIP, NULL)->AsDynamicMethodDesc(); - TADDR arg = StubManagerHelpers::GetHiddenArg(pContext); - Object * pThis = StubManagerHelpers::GetThisPtr(pContext); + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: Enter: StubMD 0x%p, HiddenArg 0x%p, ThisPtr 0x%p\n", + pStubMD, arg, pThis)); // See code:ILStubCache.CreateNewMethodDesc for the code that sets flags on stub MDs - PCODE target; + PCODE target = NULL; #ifdef FEATURE_MULTICASTSTUB_AS_IL - if(pStubMD->IsMulticastStub()) + if (pStubMD->IsMulticastStub()) { _ASSERTE(GetIP(pContext) == GetEEFuncEntryPoint(StubHelpers::MulticastDebuggerTraceHelper)); int delegateCount = (int)StubManagerHelpers::GetSecondArg(pContext); - int totalDelegateCount = (int)*(size_t*)((BYTE*)pThis + DelegateObject::GetOffsetOfInvocationCount()); - if (delegateCount == totalDelegateCount) { - LOG((LF_CORDB, LL_INFO1000, "MF::TF: Executed all stubs, should return\n")); - // We've executed all the stubs, so we should return + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: Fired all delegates\n")); return FALSE; } else @@ -1673,7 +1704,6 @@ BOOL ILStubManager::TraceManager(Thread *thread, _ASSERTE(pbDel); return DelegateInvokeStubManager::TraceDelegateObject(pbDel, trace); } - } else #endif // FEATURE_MULTICASTSTUB_AS_IL @@ -1684,14 +1714,13 @@ BOOL ILStubManager::TraceManager(Thread *thread, // This is reverse P/Invoke stub, the argument is UMEntryThunk UMEntryThunk *pEntryThunk = (UMEntryThunk *)arg; target = pEntryThunk->GetManagedTarget(); - - LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Reverse P/Invoke case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Reverse P/Invoke case 0x%p\n", target)); } else { // This is COM-to-CLR stub, the argument is the target target = (PCODE)arg; - LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: COM-to-CLR case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: COM-to-CLR case 0x%p\n", target)); } trace->InitForManaged(target); } @@ -1701,7 +1730,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, DelegateObject *pDel = (DelegateObject *)pThis; target = pDel->GetMethodPtrAux(); - LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Forward delegate P/Invoke case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Forward delegate P/Invoke case 0x%p\n", target)); trace->InitForUnmanaged(target); } else if (pStubMD->IsCALLIStub()) @@ -1714,29 +1743,41 @@ BOOL ILStubManager::TraceManager(Thread *thread, target = target >> 1; // call target is encoded as (addr << 1) | 1 #endif // TARGET_AMD64 - LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Unmanaged CALLI case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Unmanaged CALLI case 0x%p\n", target)); trace->InitForUnmanaged(target); } - else if (pStubMD->IsStructMarshalStub()) + else if (pStubMD->IsStepThroughStub()) { - // There's no "target" for struct marshalling stubs - // so we have nowhere to tell the debugger to move the breakpoint. - return FALSE; + MethodDesc* pTargetMD = pStubMD->GetILStubResolver()->GetStubTargetMethodDesc(); + if (pTargetMD == NULL) + { + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: Stub has no target to step through to\n")); + return FALSE; + } + + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: Step through to target - 0x%p\n", pTargetMD)); + target = GetStubTarget(pTargetMD); + if (target == NULL) + return FALSE; + + trace->InitForManaged(target); } - else + else if (pStubMD->HasMDContextArg()) { + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: Hidden argument is MethodDesc\n")); + // This is either direct forward P/Invoke or a CLR-to-COM call, the argument is MD MethodDesc *pMD = (MethodDesc *)arg; - if (pMD->IsNDirect()) { target = (PCODE)((NDirectMethodDesc *)pMD)->GetNativeNDirectTarget(); - LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Forward P/Invoke case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Forward P/Invoke case 0x%p\n", target)); trace->InitForUnmanaged(target); } #ifdef FEATURE_COMINTEROP else { + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: Stub is CLR-to-COM\n")); _ASSERTE(pMD->IsComPlusCall()); ComPlusCallMethodDesc *pCMD = (ComPlusCallMethodDesc *)pMD; _ASSERTE(!pCMD->IsStatic() && !pCMD->IsCtor() && "Static methods and constructors are not supported for built-in classic COM"); @@ -1744,13 +1785,17 @@ BOOL ILStubManager::TraceManager(Thread *thread, if (pThis != NULL) { target = GetCOMTarget(pThis, pCMD->m_pComPlusCallInfo); - - LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: CLR-to-COM case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: CLR-to-COM case 0x%p\n", target)); trace->InitForUnmanaged(target); } } #endif // FEATURE_COMINTEROP } + else + { + // There's no "target" so we have nowhere to tell the debugger to move the breakpoint. + return FALSE; + } return TRUE; } @@ -1871,13 +1916,13 @@ BOOL InteropDispatchStubManager::TraceManager(Thread *thread, _ASSERTE(pNMD->IsNDirect()); PCODE target = (PCODE)pNMD->GetNDirectTarget(); - LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Vararg P/Invoke case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Vararg P/Invoke case 0x%p\n", target)); trace->InitForUnmanaged(target); } else if (GetIP(pContext) == GetEEFuncEntryPoint(GenericPInvokeCalliHelper)) { PCODE target = (PCODE)arg; - LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Unmanaged CALLI case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Unmanaged CALLI case 0x%p\n", target)); trace->InitForUnmanaged(target); } #ifdef FEATURE_COMINTEROP @@ -1907,7 +1952,7 @@ BOOL InteropDispatchStubManager::TraceManager(Thread *thread, LPVOID *lpVtbl = *(LPVOID **)(IUnknown *)pUnk; PCODE target = (PCODE)lpVtbl[6]; // DISPATCH_INVOKE_SLOT; - LOG((LF_CORDB, LL_INFO10000, "CPSM::TraceManager: CLR-to-COM late-bound case 0x%x\n", target)); + LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: CLR-to-COM late-bound case 0x%p\n", target)); trace->InitForUnmanaged(target); GCPROTECT_END(); @@ -2063,7 +2108,7 @@ BOOL DelegateInvokeStubManager::TraceManager(Thread *thread, TraceDestination *t // We use the patch offset field to indicate whether the stub has a hidden return buffer argument. // This field is set in SetupShuffleThunk(). - if (pStub->GetPatchOffset() != 0) + if (pStub->GetOffset() != 0) { // This stub has a hidden return buffer argument. orDelegate = (DELEGATEREF)ObjectToOBJECTREF(StubManagerHelpers::GetSecondArg(pContext)); @@ -2128,7 +2173,7 @@ BOOL DelegateInvokeStubManager::TraceDelegateObject(BYTE* pbDel, TraceDestinatio BYTE *pbDelInvocationList = *(BYTE **)(pbDel + DelegateObject::GetOffsetOfInvocationList()); - LOG((LF_CORDB,LL_INFO10000, "DISM::TMI: invocationList: 0x%x\n", pbDelInvocationList)); + LOG((LF_CORDB,LL_INFO10000, "DISM::TMI: invocationList: 0x%p\n", pbDelInvocationList)); if (pbDelInvocationList == NULL) { @@ -2153,7 +2198,7 @@ BOOL DelegateInvokeStubManager::TraceDelegateObject(BYTE* pbDel, TraceDestinatio } - LOG((LF_CORDB,LL_INFO10000, "DISM(DelegateStub)::TM: ppbDest: 0x%x *ppbDest:0x%x\n", ppbDest, *ppbDest)); + LOG((LF_CORDB,LL_INFO10000, "DISM(DelegateStub)::TM: ppbDest: 0x%p *ppbDest:0x%p\n", ppbDest, *ppbDest)); BOOL res = StubManager::TraceStub((PCODE) (*ppbDest), trace); @@ -2180,7 +2225,7 @@ BOOL DelegateInvokeStubManager::TraceDelegateObject(BYTE* pbDel, TraceDestinatio return FALSE; } - LOG((LF_CORDB,LL_INFO10000, "DISM(DelegateStub)::TM: ppbDest: 0x%x *ppbDest:0x%x\n", ppbDest, *ppbDest)); + LOG((LF_CORDB,LL_INFO10000, "DISM(DelegateStub)::TM: ppbDest: 0x%p *ppbDest:0x%p\n", ppbDest, *ppbDest)); BOOL res = StubManager::TraceStub((PCODE) (*ppbDest), trace); diff --git a/src/coreclr/vm/stubmgr.h b/src/coreclr/vm/stubmgr.h index 2bb4b10eda150e..acb89f5af2a22f 100644 --- a/src/coreclr/vm/stubmgr.h +++ b/src/coreclr/vm/stubmgr.h @@ -662,10 +662,6 @@ class ILStubManager : public StubManager virtual BOOL DoTraceStub(PCODE stubStartAddress, TraceDestination *trace); #ifndef DACCESS_COMPILE -#ifdef FEATURE_COMINTEROP - static PCODE GetCOMTarget(Object *pThis, ComPlusCallInfo *pComPlusCallInfo); -#endif // FEATURE_COMINTEROP - virtual BOOL TraceManager(Thread *thread, TraceDestination *trace, T_CONTEXT *pContext, From e24a783a1cc6bb0e363b6ec187cb95a91101e603 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 28 Feb 2022 11:47:42 -0800 Subject: [PATCH 03/11] Updates to comments and logging. --- src/coreclr/debug/ee/debugger.cpp | 10 +++++----- src/coreclr/debug/ee/functioninfo.cpp | 10 +++++----- src/coreclr/vm/method.inl | 14 +++++++------- src/coreclr/vm/stubmgr.cpp | 1 + 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index eac99b70fca06b..7c06fee064f26b 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -2488,7 +2488,7 @@ void Debugger::JITComplete(NativeCodeVersion nativeCodeVersion, TADDR newAddress MethodDesc* fd = nativeCodeVersion.GetMethodDesc(); - LOG((LF_CORDB, LL_INFO100000, "D::JITComplete: md:0x%x (%s::%s), address:0x%x.\n", + LOG((LF_CORDB, LL_INFO100000, "D::JITComplete: md:0x%p (%s::%s), address:0x%p.\n", fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, newAddress)); @@ -2521,13 +2521,13 @@ void Debugger::JITComplete(NativeCodeVersion nativeCodeVersion, TADDR newAddress // method on two threads. When this occurs both threads will // return the same code pointer and this callback is invoked // multiple times. - LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%x (%s::%s), address:0x%x. Already created\n", + LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%p (%s::%s), address:0x%p. Already created\n", fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, newAddress)); goto Exit; } - LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%x (%s::%s), address:0x%x. Created ji:0x%x\n", + LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%p (%s::%s), address:0x%p. Created ji:0x%p\n", fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, newAddress, ji)); @@ -4808,7 +4808,7 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, mdMethodDef md = fd->GetMemberDef(); LOG((LF_CORDB,LL_INFO10000,"D::MABFP: All BPs will be mapped to " - "Ver:0x%04x (DJI:0x%08x)\n", djiNew?djiNew->m_methodInfo->GetCurrentEnCVersion():0, djiNew)); + "Ver:0x%04x (DJI:0x%p)\n", djiNew?djiNew->m_methodInfo->GetCurrentEnCVersion():0, djiNew)); // We need to traverse the patch list while under the controller lock (small lock). // But we can only send BreakpointSetErros while under the debugger lock (big lock). @@ -4847,7 +4847,7 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, // elsewhere. if(dcp->pMethodDescFilter != NULL && dcp->pMethodDescFilter != djiNew->m_nativeCodeVersion.GetMethodDesc()) { - LOG((LF_CORDB, LL_INFO10000, "Patch not in this generic instance\n")); + LOG((LF_CORDB, LL_INFO10000, "Patch not in this generic instance, filter 0x%p\n", dcp->pMethodDescFilter)); continue; } diff --git a/src/coreclr/debug/ee/functioninfo.cpp b/src/coreclr/debug/ee/functioninfo.cpp index 55845f934cae7b..ec945f40133276 100644 --- a/src/coreclr/debug/ee/functioninfo.cpp +++ b/src/coreclr/debug/ee/functioninfo.cpp @@ -1016,7 +1016,7 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping * } CONTRACTL_END; - LOG((LF_CORDB,LL_EVERYTHING, "DJI::SetBoundaries: this=0x%x cMap=0x%x pMap=0x%x\n", this, cMap, pMap)); + LOG((LF_CORDB,LL_EVERYTHING, "DJI::SetBoundaries: this=0x%p cMap=0x%x pMap=0x%p\n", this, cMap, pMap)); _ASSERTE((cMap == 0) == (pMap == NULL)); _ASSERTE(m_sequenceMap == NULL); @@ -1043,7 +1043,7 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping * // like the DebuggerJitInfo's. // m_sequenceMap = (DebuggerILToNativeMap *)new (interopsafe) DebuggerILToNativeMap[cMap]; - LOG((LF_CORDB,LL_EVERYTHING, "DJI::SetBoundaries: this=0x%x m_sequenceMap=0x%x\n", this, m_sequenceMap)); + LOG((LF_CORDB,LL_EVERYTHING, "DJI::SetBoundaries: this=0x%p m_sequenceMap=0x%x\n", this, m_sequenceMap)); _ASSERTE(m_sequenceMap != NULL); // we'll throw on null m_sequenceMapCount = cMap; @@ -1186,7 +1186,7 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping * m_callsiteMap = m_sequenceMap + m_sequenceMapCount; m_callsiteMapCount -= m_sequenceMapCount; - LOG((LF_CORDB, LL_INFO100000, "DJI::SetBoundaries: this=0x%x boundary count is %d (%d callsites)\n", + LOG((LF_CORDB, LL_INFO100000, "DJI::SetBoundaries: this=0x%p boundary count is %d (%d callsites)\n", this, m_sequenceMapCount, m_callsiteMapCount)); #ifdef LOGGING @@ -1691,7 +1691,7 @@ DebuggerJitInfo *DebuggerMethodInfo::CreateInitAndAddJitInfo(NativeCodeVersion n // We know it's not in the table. Go add it! DebuggerJitInfo *djiPrev = m_latestJitInfo; - LOG((LF_CORDB,LL_INFO10000,"DMI:CAAJI: current head of dji list:0x%08x\n", djiPrev)); + LOG((LF_CORDB,LL_INFO10000,"DMI:CAAJI: current head of dji list:0x%p\n", djiPrev)); if (djiPrev != NULL) { @@ -1714,7 +1714,7 @@ DebuggerJitInfo *DebuggerMethodInfo::CreateInitAndAddJitInfo(NativeCodeVersion n // We've now added a new DJI into the table and released the lock. Thus any other thread // can come and use our DJI. Good thing we inited the DJI _before_ adding it to the table. - LOG((LF_CORDB,LL_INFO10000,"DMI:CAAJI: new head of dji list:0x%08x\n", m_latestJitInfo)); + LOG((LF_CORDB,LL_INFO10000,"DMI:CAAJI: new head of dji list:0x%p\n", m_latestJitInfo)); return dji; } diff --git a/src/coreclr/vm/method.inl b/src/coreclr/vm/method.inl index 1296f2de36bf4e..fd32a95bee3827 100644 --- a/src/coreclr/vm/method.inl +++ b/src/coreclr/vm/method.inl @@ -15,13 +15,6 @@ inline InstantiatedMethodDesc* MethodDesc::AsInstantiatedMethodDesc() const return dac_cast(this); } -inline PTR_DynamicResolver DynamicMethodDesc::GetResolver() -{ - LIMITED_METHOD_CONTRACT; - - return m_pResolver; -} - inline SigParser MethodDesc::GetSigParser() { WRAPPER_NO_CONTRACT; @@ -44,6 +37,13 @@ inline SigPointer MethodDesc::GetSigPointer() return SigPointer(pSig, cSig); } +inline PTR_DynamicResolver DynamicMethodDesc::GetResolver() +{ + LIMITED_METHOD_CONTRACT; + + return m_pResolver; +} + inline PTR_LCGMethodResolver DynamicMethodDesc::GetLCGMethodResolver() { CONTRACTL diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 25a69dab5b5f1a..256a3e838b5ac5 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -1793,6 +1793,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, } else { + LOG((LF_CORDB, LL_INFO1000, "ILSM::TraceManager: No known target, IL Stub is a leaf\n")); // There's no "target" so we have nowhere to tell the debugger to move the breakpoint. return FALSE; } From 319a40c6d8a07587db77996f8826924ef86f4876 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 28 Feb 2022 11:49:10 -0800 Subject: [PATCH 04/11] ArrayOp IL Stubs no longer marked as nomdStepThroughStub. --- src/coreclr/vm/dynamicmethod.cpp | 40 +++++++++++++++++++++++++++++++- src/coreclr/vm/ilstubcache.cpp | 1 - src/coreclr/vm/method.hpp | 6 +---- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/dynamicmethod.cpp b/src/coreclr/vm/dynamicmethod.cpp index a5a9ee44ba8ebd..0618bec7f8c3d8 100644 --- a/src/coreclr/vm/dynamicmethod.cpp +++ b/src/coreclr/vm/dynamicmethod.cpp @@ -530,7 +530,7 @@ HostCodeHeap::TrackAllocation* HostCodeHeap::AllocFromFreeList(size_t header, si { // create a new TrackAllocation after the memory we just allocated and insert it into the free list TrackAllocation *pNewCurrent = (TrackAllocation*)((BYTE*)pCurrent + realSize); - + ExecutableWriterHolder newCurrentWriterHolder(pNewCurrent, sizeof(TrackAllocation)); newCurrentWriterHolder.GetRW()->pNext = pCurrent->pNext; newCurrentWriterHolder.GetRW()->size = pCurrent->size - realSize; @@ -1475,6 +1475,44 @@ void LCGMethodResolver::GetEHInfo(unsigned EHnumber, CORINFO_EH_CLAUSE* clause) #endif // !DACCESS_COMPILE +bool DynamicMethodDesc::HasMDContextArg() +{ + CONTRACTL + { + MODE_ANY; + GC_NOTRIGGER; + NOTHROW; + PRECONDITION(IsILStub()); + } + CONTRACTL_END; + + // Perform a minimal check. This is historically sufficient, but has not been updated + // with flag usage. + // If the check indicates false, it is for sure false. However, if it is true + // we need to check additional cases. + bool minCheck = IsCLRToCOMStub() || (IsPInvokeStub() && !IsDelegateStub()); + if (!minCheck) + return false; + +#ifdef DACCESS_COMPILE + // The DAC's usage of this API is narrow enough that the precise nature needed by + // the runtime is not needed. Therefore, we can return the historically sufficient answer. + return true; +#else + ILStubResolver::ILStubType type = GetILStubResolver()->GetStubType(); + switch (type) + { +#ifdef FEATURE_ARRAYSTUB_AS_IL + case ILStubResolver::ArrayOpStub: +#endif + case ILStubResolver::TailCallCallTargetStub: + case ILStubResolver::TailCallStoreArgsStub: + return false; + default: + return true; + } +#endif // !DACCESS_COMPILE +} // Get the associated managed resolver. This method will be called during a GC so it should not throw, trigger a GC or cause the // object in question to be validated. diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index 3f689bbaf47e36..2e621c51fb9d3d 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -202,7 +202,6 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa #ifdef FEATURE_ARRAYSTUB_AS_IL if (SF_IsArrayOpStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdStepThroughStub; pMD->GetILStubResolver()->SetStubType(ILStubResolver::ArrayOpStub); } else diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index c5d82f32c773ce..e2a75ee22023df 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2627,11 +2627,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc #endif // Whether the stub takes a context argument that is an interop MethodDesc. - bool HasMDContextArg() - { - LIMITED_METHOD_CONTRACT; - return IsCLRToCOMStub() || (IsPInvokeStub() && !IsDelegateStub()); - } + bool HasMDContextArg(); // // following implementations defined in DynamicMethod.cpp From d41e0ddac9db4b5775f772db21cf2ccadb463867 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 1 Mar 2022 08:51:58 -0800 Subject: [PATCH 05/11] Reorganize the ILStubType logic between DynamicMethodDesc and ILStubResolver. --- src/coreclr/debug/ee/controller.cpp | 2 +- src/coreclr/debug/ee/frameinfo.cpp | 2 +- src/coreclr/vm/dllimport.cpp | 13 +- src/coreclr/vm/dynamicmethod.cpp | 48 +------ src/coreclr/vm/ilstubcache.cpp | 88 ++++++++---- src/coreclr/vm/ilstubresolver.cpp | 54 +------ src/coreclr/vm/ilstubresolver.h | 30 +--- src/coreclr/vm/method.cpp | 8 +- src/coreclr/vm/method.hpp | 211 +++++++++++++++++----------- src/coreclr/vm/prestub.cpp | 16 +-- src/coreclr/vm/stubmgr.cpp | 2 +- src/coreclr/vm/threadsuspend.cpp | 8 +- 12 files changed, 220 insertions(+), 262 deletions(-) diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index af087a7f819416..d1856928bb77a0 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -6384,7 +6384,7 @@ void DebuggerStepper::TrapStepOut(ControllerStackInfo *info, bool fForceTraditio else #endif // FEATURE_MULTICASTSTUB_AS_IL if (info->m_activeFrame.md != nullptr && info->m_activeFrame.md->IsILStub() && - info->m_activeFrame.md->AsDynamicMethodDesc()->GetILStubResolver()->GetStubType() == ILStubResolver::TailCallCallTargetStub) + info->m_activeFrame.md->AsDynamicMethodDesc()->GetILStubType() == DynamicMethodDesc::StubTailCallCallTarget) { // Normally the stack trace would not include IL stubs, but we // include this specific IL stub so that we can check if a call into diff --git a/src/coreclr/debug/ee/frameinfo.cpp b/src/coreclr/debug/ee/frameinfo.cpp index 9bd0d1315bd0b0..f765d84bba9f19 100644 --- a/src/coreclr/debug/ee/frameinfo.cpp +++ b/src/coreclr/debug/ee/frameinfo.cpp @@ -1568,7 +1568,7 @@ StackWalkAction DebuggerWalkStackProc(CrawlFrame *pCF, void *data) #ifdef FEATURE_MULTICASTSTUB_AS_IL use |= dMD->IsMulticastStub(); #endif - use |= dMD->GetILStubResolver()->GetStubType() == ILStubResolver::TailCallCallTargetStub; + use |= dMD->GetILStubType() == DynamicMethodDesc::StubTailCallCallTarget; if (use) { diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 35446a502ae958..7be3b32af83f57 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -436,12 +436,12 @@ class ILStubState : public StubState if (callConvInfo & CORINFO_CALLCONV_HASTHIS) { - ((PTR_DynamicMethodDesc)pStubMD)->m_dwExtendedFlags &= ~mdStatic; + ((PTR_DynamicMethodDesc)pStubMD)->ClearFlags(DynamicMethodDesc::FlagStatic); pStubMD->ClearStatic(); } else { - ((PTR_DynamicMethodDesc)pStubMD)->m_dwExtendedFlags |= mdStatic; + ((PTR_DynamicMethodDesc)pStubMD)->SetFlags(DynamicMethodDesc::FlagStatic); pStubMD->SetStatic(); } @@ -3834,7 +3834,7 @@ static void CreateNDirectStubWorker(StubState* pss, marshalType == MarshalInfo::MARSHAL_TYPE_CRITICALHANDLE) { if (pMD->IsDynamicMethod()) - pMD->AsDynamicMethodDesc()->SetUnbreakable(true); + pMD->AsDynamicMethodDesc()->SetFlags(DynamicMethodDesc::FlagUnbreakable); } } @@ -3868,7 +3868,8 @@ static void CreateNDirectStubWorker(StubState* pss, DynamicMethodDesc *pDMD = pMD->AsDynamicMethodDesc(); pDMD->SetNativeStackArgSize(static_cast(nativeStackSize)); - pDMD->SetStubNeedsCOMStarted(fStubNeedsCOM); + if (fStubNeedsCOM) + pDMD->SetFlags(DynamicMethodDesc::FlagRequiresCOM); } // FinishEmit needs to know the native stack arg size so we call it after the number @@ -5703,7 +5704,9 @@ PCODE GetStubForInteropMethod(MethodDesc* pMD, DWORD dwStubFlags) UNREACHABLE_MSG("unexpected MethodDesc type"); } - if (pStubMD != NULL && pStubMD->IsILStub() && pStubMD->AsDynamicMethodDesc()->IsStubNeedsCOMStarted()) + if (pStubMD != NULL + && pStubMD->IsILStub() + && pStubMD->AsDynamicMethodDesc()->AreFlagSets(DynamicMethodDesc::FlagRequiresCOM)) { // the stub uses COM so make sure that it is started EnsureComStarted(); diff --git a/src/coreclr/vm/dynamicmethod.cpp b/src/coreclr/vm/dynamicmethod.cpp index 0618bec7f8c3d8..8ff5b8822ba8bc 100644 --- a/src/coreclr/vm/dynamicmethod.cpp +++ b/src/coreclr/vm/dynamicmethod.cpp @@ -180,8 +180,9 @@ void DynamicMethodTable::AddMethodsToList() pNewMD->SetMemberDef(0); pNewMD->SetSlot(MethodTable::NO_SLOT); // we can't ever use the slot for dynamic methods pNewMD->SetStatic(); - - pNewMD->m_dwExtendedFlags = mdPublic | mdStatic | DynamicMethodDesc::nomdLCGMethod; + pNewMD->InitializeFlags(DynamicMethodDesc::FlagPublic + | DynamicMethodDesc::FlagStatic + | DynamicMethodDesc::FlagIsLCGMethod); LCGMethodResolver* pResolver = new (pResolvers) LCGMethodResolver(); pResolver->m_pDynamicMethod = pNewMD; @@ -272,8 +273,9 @@ DynamicMethodDesc* DynamicMethodTable::GetDynamicMethod(BYTE *psig, DWORD sigSiz pNewMD->SetStoredMethodSig((PCCOR_SIGNATURE)psig, sigSize); // the dynamic part of the method desc pNewMD->m_pszMethodName = name; - - pNewMD->m_dwExtendedFlags = mdPublic | mdStatic | DynamicMethodDesc::nomdLCGMethod; + pNewMD->InitializeFlags(DynamicMethodDesc::FlagPublic + | DynamicMethodDesc::FlagStatic + | DynamicMethodDesc::FlagIsLCGMethod); #ifdef _DEBUG pNewMD->m_pszDebugMethodName = name; @@ -1475,44 +1477,6 @@ void LCGMethodResolver::GetEHInfo(unsigned EHnumber, CORINFO_EH_CLAUSE* clause) #endif // !DACCESS_COMPILE -bool DynamicMethodDesc::HasMDContextArg() -{ - CONTRACTL - { - MODE_ANY; - GC_NOTRIGGER; - NOTHROW; - PRECONDITION(IsILStub()); - } - CONTRACTL_END; - - // Perform a minimal check. This is historically sufficient, but has not been updated - // with flag usage. - // If the check indicates false, it is for sure false. However, if it is true - // we need to check additional cases. - bool minCheck = IsCLRToCOMStub() || (IsPInvokeStub() && !IsDelegateStub()); - if (!minCheck) - return false; - -#ifdef DACCESS_COMPILE - // The DAC's usage of this API is narrow enough that the precise nature needed by - // the runtime is not needed. Therefore, we can return the historically sufficient answer. - return true; -#else - ILStubResolver::ILStubType type = GetILStubResolver()->GetStubType(); - switch (type) - { -#ifdef FEATURE_ARRAYSTUB_AS_IL - case ILStubResolver::ArrayOpStub: -#endif - case ILStubResolver::TailCallCallTargetStub: - case ILStubResolver::TailCallStoreArgsStub: - return false; - default: - return true; - } -#endif // !DACCESS_COMPILE -} // Get the associated managed resolver. This method will be called during a GC so it should not throw, trigger a GC or cause the // object in question to be validated. diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index 2e621c51fb9d3d..44ece6bf30ba43 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -126,6 +126,45 @@ MethodDesc* ILStubCache::CreateAndLinkNewILStubMethodDesc(LoaderAllocator* pAllo } + +namespace +{ + LPCUTF8 GetStubMethodName(DynamicMethodDesc::ILStubType type) + { + CONTRACTL + { + MODE_ANY; + GC_NOTRIGGER; + NOTHROW; + } + CONTRACTL_END; + + switch (type) + { + case DynamicMethodDesc::StubCLRToNativeInterop: return "IL_STUB_PInvoke"; + case DynamicMethodDesc::StubCLRToCOMInterop: return "IL_STUB_CLRtoCOM"; + case DynamicMethodDesc::StubNativeToCLRInterop: return "IL_STUB_ReversePInvoke"; + case DynamicMethodDesc::StubCOMToCLRInterop: return "IL_STUB_COMtoCLR"; + case DynamicMethodDesc::StubStructMarshalInterop: return "IL_STUB_StructMarshal"; +#ifdef FEATURE_ARRAYSTUB_AS_IL + case DynamicMethodDesc::StubArrayOp: return "IL_STUB_Array"; +#endif +#ifdef FEATURE_MULTICASTSTUB_AS_IL + case DynamicMethodDesc::StubMulticastDelegate: return "IL_STUB_MulticastDelegate_Invoke"; +#endif +#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL + case DynamicMethodDesc::StubUnboxingIL: return "IL_STUB_UnboxingStub"; + case DynamicMethodDesc::StubInstantiating: return "IL_STUB_InstantiatingStub"; +#endif + case DynamicMethodDesc::StubWrapperDelegate: return "IL_STUB_WrapperDelegate_Invoke"; + case DynamicMethodDesc::StubTailCallStoreArgs: return "IL_STUB_StoreTailCallArgs"; + case DynamicMethodDesc::StubTailCallCallTarget: return "IL_STUB_CallTailCallTarget"; + default: + UNREACHABLE_MSG("Unknown stub type"); + } + } +} + // static MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTable* pMT, DWORD dwStubFlags, Module* pSigModule, PCCOR_SIGNATURE pSig, DWORD cbSig, SigTypeContext *pTypeContext, @@ -157,8 +196,7 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa pMD->SetSlot(MethodTable::NO_SLOT); // we can't ever use the slot for dynamic methods // the no metadata part of the method desc pMD->m_pszMethodName = (PTR_CUTF8)"IL_STUB"; - pMD->m_dwExtendedFlags = mdPublic | DynamicMethodDesc::nomdILStub; - + pMD->InitializeFlags(DynamicMethodDesc::FlagPublic | DynamicMethodDesc::FlagIsILStub); pMD->SetTemporaryEntryPoint(pMT->GetLoaderAllocator(), pamTracker); // @@ -187,7 +225,7 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa if (!(callConvInfo & CORINFO_CALLCONV_HASTHIS)) { - pMD->m_dwExtendedFlags |= mdStatic; + pMD->SetFlags(DynamicMethodDesc::FlagStatic); pMD->SetStatic(); } @@ -197,51 +235,46 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa memset((void*)pMD->m_pResolver, 0xCC, sizeof(ILStubResolver)); #endif // _DEBUG pMD->m_pResolver = new (pMD->m_pResolver) ILStubResolver(); - pMD->GetILStubResolver()->SetLoaderHeap(pCreationHeap); #ifdef FEATURE_ARRAYSTUB_AS_IL if (SF_IsArrayOpStub(dwStubFlags)) { - pMD->GetILStubResolver()->SetStubType(ILStubResolver::ArrayOpStub); + pMD->SetILStubType(DynamicMethodDesc::StubArrayOp); } else #endif #ifdef FEATURE_MULTICASTSTUB_AS_IL if (SF_IsMulticastDelegateStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdMulticastStub; - pMD->GetILStubResolver()->SetStubType(ILStubResolver::MulticastDelegateStub); + pMD->SetILStubType(DynamicMethodDesc::StubMulticastDelegate); } else #endif if (SF_IsWrapperDelegateStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdWrapperDelegateStub; - pMD->GetILStubResolver()->SetStubType(ILStubResolver::WrapperDelegateStub); + pMD->SetILStubType(DynamicMethodDesc::StubWrapperDelegate); } else #ifdef FEATURE_INSTANTIATINGSTUB_AS_IL if (SF_IsUnboxingILStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= (DynamicMethodDesc::nomdUnboxingILStub | DynamicMethodDesc::nomdStepThroughStub); - pMD->GetILStubResolver()->SetStubType(ILStubResolver::UnboxingILStub); + pMD->SetILStubType(DynamicMethodDesc::StubUnboxingIL); } else if (SF_IsInstantiatingStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdStepThroughStub; - pMD->GetILStubResolver()->SetStubType(ILStubResolver::InstantiatingStub); + pMD->SetILStubType(DynamicMethodDesc::StubInstantiating); } else #endif if (SF_IsTailCallStoreArgsStub(dwStubFlags)) { - pMD->GetILStubResolver()->SetStubType(ILStubResolver::TailCallStoreArgsStub); + pMD->SetILStubType(DynamicMethodDesc::StubTailCallStoreArgs); } else if (SF_IsTailCallCallTargetStub(dwStubFlags)) { - pMD->GetILStubResolver()->SetStubType(ILStubResolver::TailCallCallTargetStub); + pMD->SetILStubType(DynamicMethodDesc::StubTailCallCallTarget); } else #ifdef FEATURE_COMINTEROP @@ -250,43 +283,40 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa // mark certain types of stub MDs with random flags so ILStubManager recognizes them if (SF_IsReverseStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdReverseStub; - - ILStubResolver::ILStubType type = ILStubResolver::COMToCLRInteropStub; - pMD->GetILStubResolver()->SetStubType(type); + pMD->SetILStubType(DynamicMethodDesc::StubCOMToCLRInterop); } else { - ILStubResolver::ILStubType type = ILStubResolver::CLRToCOMInteropStub; - pMD->GetILStubResolver()->SetStubType(type); + pMD->SetILStubType(DynamicMethodDesc::StubCLRToCOMInterop); } } else #endif if (SF_IsStructMarshalStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdStructMarshalStub; - pMD->GetILStubResolver()->SetStubType(ILStubResolver::StructMarshalInteropStub); + // Struct marshal stub MethodDescs might be directly called from `call` IL instructions + // so we want to keep their compile time data alive as long as the LoaderAllocator in case they're used again. + pMD->GetILStubResolver()->SetLoaderHeap(pCreationHeap); + pMD->SetILStubType(DynamicMethodDesc::StubStructMarshalInterop); } else { // mark certain types of stub MDs with random flags so ILStubManager recognizes them if (SF_IsReverseStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdReverseStub | DynamicMethodDesc::nomdUnmanagedCallersOnlyStub; - pMD->GetILStubResolver()->SetStubType(ILStubResolver::NativeToCLRInteropStub); + pMD->SetILStubType(DynamicMethodDesc::StubNativeToCLRInterop); } else { if (SF_IsDelegateStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdDelegateStub; + pMD->SetFlags(DynamicMethodDesc::FlagIsDelegate); } else if (SF_IsCALLIStub(dwStubFlags)) { - pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdCALLIStub; + pMD->SetFlags(DynamicMethodDesc::FlagIsCALLI); } - pMD->GetILStubResolver()->SetStubType(ILStubResolver::CLRToNativeInteropStub); + pMD->SetILStubType(DynamicMethodDesc::StubCLRToNativeInterop); } } @@ -308,7 +338,7 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa else #endif { - pMD->m_pszMethodName = pMD->GetILStubResolver()->GetStubMethodName(); + pMD->m_pszMethodName = GetStubMethodName(pMD->GetILStubType()); } diff --git a/src/coreclr/vm/ilstubresolver.cpp b/src/coreclr/vm/ilstubresolver.cpp index 74cd62a70153a8..76f6df6a4fbedc 100644 --- a/src/coreclr/vm/ilstubresolver.cpp +++ b/src/coreclr/vm/ilstubresolver.cpp @@ -58,41 +58,6 @@ LPCUTF8 ILStubResolver::GetStubClassName(MethodDesc* pMD) return "ILStubClass"; } -LPCUTF8 ILStubResolver::GetStubMethodName() -{ - CONTRACTL - { - MODE_ANY; - GC_NOTRIGGER; - NOTHROW; - } - CONTRACTL_END; - - switch (m_type) - { - case CLRToNativeInteropStub: return "IL_STUB_PInvoke"; - case CLRToCOMInteropStub: return "IL_STUB_CLRtoCOM"; - case NativeToCLRInteropStub: return "IL_STUB_ReversePInvoke"; - case COMToCLRInteropStub: return "IL_STUB_COMtoCLR"; - case StructMarshalInteropStub: return "IL_STUB_StructMarshal"; -#ifdef FEATURE_ARRAYSTUB_AS_IL - case ArrayOpStub: return "IL_STUB_Array"; -#endif -#ifdef FEATURE_MULTICASTSTUB_AS_IL - case MulticastDelegateStub: return "IL_STUB_MulticastDelegate_Invoke"; -#endif -#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL - case UnboxingILStub: return "IL_STUB_UnboxingStub"; - case InstantiatingStub: return "IL_STUB_InstantiatingStub"; -#endif - case WrapperDelegateStub: return "IL_STUB_WrapperDelegate_Invoke"; - case TailCallStoreArgsStub: return "IL_STUB_StoreTailCallArgs"; - case TailCallCallTargetStub: return "IL_STUB_CallTailCallTarget"; - default: - UNREACHABLE_MSG("Unknown stub type"); - } -} - void ILStubResolver::GetJitContext(SecurityControlFlags* pSecurityControlFlags, TypeHandle* pTypeOwner) { @@ -237,18 +202,6 @@ void ILStubResolver::GetEHInfo(unsigned EHnumber, CORINFO_EH_CLAUSE* clause) clause->FilterOffset = ehInfo->GetFilterOffset(); } -ILStubResolver::ILStubType ILStubResolver::GetStubType() -{ - LIMITED_METHOD_CONTRACT; - return m_type; -} - -void ILStubResolver::SetStubType(ILStubType stubType) -{ - LIMITED_METHOD_CONTRACT; - m_type = stubType; -} - void ILStubResolver::SetStubMethodDesc(MethodDesc* pStubMD) { LIMITED_METHOD_CONTRACT; @@ -303,7 +256,6 @@ ILStubResolver::ILStubResolver() : m_pCompileTimeState(dac_cast(ILNotYetGenerated)), m_pStubMD(dac_cast(nullptr)), m_pStubTargetMD(dac_cast(nullptr)), - m_type(Unassigned), m_jitFlags(), m_loaderHeap(dac_cast(nullptr)) { @@ -369,8 +321,6 @@ ILStubResolver::AllocGeneratedIL( } else { - CONSISTENCY_CHECK(m_loaderHeap != dac_cast(nullptr)); - AllocMemHolder pNewILCodeBuffer(m_loaderHeap->AllocMem(S_SIZE_T(cbCode))); AllocMemHolder pNewCompileTimeState(m_loaderHeap->AllocMem(S_SIZE_T(sizeof(CompileTimeState)))); memset(pNewCompileTimeState, 0, sizeof(CompileTimeState)); @@ -442,9 +392,7 @@ COR_ILMETHOD_SECT_EH* ILStubResolver::AllocEHSect(size_t nClauses) bool ILStubResolver::UseLoaderHeap() { - // Struct marshal stub MethodDescs might be directly called from `call` IL instructions - // so we want to keep their compile time data alive as long as the LoaderAllocator in case they're used again. - return m_type == StructMarshalInteropStub; + return m_loaderHeap != dac_cast(nullptr); } void ILStubResolver::FreeCompileTimeState() diff --git a/src/coreclr/vm/ilstubresolver.h b/src/coreclr/vm/ilstubresolver.h index b60721d2541b11..dd20255c8d09ca 100644 --- a/src/coreclr/vm/ilstubresolver.h +++ b/src/coreclr/vm/ilstubresolver.h @@ -39,7 +39,6 @@ class ILStubResolver : DynamicResolver void GetEHInfo(unsigned EHnumber, CORINFO_EH_CLAUSE* clause); static LPCUTF8 GetStubClassName(MethodDesc* pMD); - LPCUTF8 GetStubMethodName(); MethodDesc* GetDynamicMethod() { LIMITED_METHOD_CONTRACT; return m_pStubMD; } @@ -68,35 +67,12 @@ class ILStubResolver : DynamicResolver void SetJitFlags(CORJIT_FLAGS jitFlags); CORJIT_FLAGS GetJitFlags(); + // This is only set for StructMarshal interop stubs. + // See callsites for more details. void SetLoaderHeap(PTR_LoaderHeap pLoaderHeap); static void StubGenFailed(ILStubResolver* pResolver); - enum ILStubType - { - Unassigned = 0, - CLRToNativeInteropStub, - CLRToCOMInteropStub, - NativeToCLRInteropStub, - COMToCLRInteropStub, - StructMarshalInteropStub, -#ifdef FEATURE_ARRAYSTUB_AS_IL - ArrayOpStub, -#endif -#ifdef FEATURE_MULTICASTSTUB_AS_IL - MulticastDelegateStub, -#endif - WrapperDelegateStub, -#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL - UnboxingILStub, - InstantiatingStub, -#endif - TailCallStoreArgsStub, - TailCallCallTargetStub, - }; - - ILStubType GetStubType(); - protected: enum CompileTimeStatePtrSpecialValues @@ -106,7 +82,6 @@ class ILStubResolver : DynamicResolver }; void ClearCompileTimeState(CompileTimeStatePtrSpecialValues newState); - void SetStubType(ILStubType stubType); bool UseLoaderHeap(); // @@ -125,7 +100,6 @@ class ILStubResolver : DynamicResolver PTR_MethodDesc m_pStubMD; PTR_MethodDesc m_pStubTargetMD; - ILStubType m_type; CORJIT_FLAGS m_jitFlags; PTR_LoaderHeap m_loaderHeap; }; diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 4309978b132346..8969e312c0f161 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1318,7 +1318,7 @@ DWORD MethodDesc::GetAttrs() const if (IsArray()) return dac_cast(this)->GetAttrs(); else if (IsNoMetadata()) - return dac_cast(this)->GetAttrs();; + return dac_cast(this)->GetFlagsAsMetadata(); DWORD dwAttributes; if (FAILED(GetMDImport()->GetMethodDefProps(GetMemberDef(), &dwAttributes))) @@ -1662,7 +1662,7 @@ UINT MethodDesc::CbStackPop() { msig.ClearHasThis(); } - + return argit.CbStackPop(); } #endif // TARGET_X86 @@ -3606,7 +3606,9 @@ BOOL MethodDesc::HasUnmanagedCallersOnlyAttribute() if (IsILStub()) { - return AsDynamicMethodDesc()->IsUnmanagedCallersOnlyStub(); + // Stubs generated for being called from native code are equivalent to + // managed methods marked with UnmanagedCallersOnly. + return AsDynamicMethodDesc()->GetILStubType() == DynamicMethodDesc::StubNativeToCLRInterop; } HRESULT hr = GetCustomAttribute( diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index e2a75ee22023df..10dbd1f3df0224 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2378,19 +2378,20 @@ MethodDesc* Entry2MethodDesc(PCODE entryPoint, MethodTable *pMT); typedef DPTR(class StoredSigMethodDesc) PTR_StoredSigMethodDesc; class StoredSigMethodDesc : public MethodDesc { - public: +public: // Put the sig RVA in here - this allows us to avoid // touching the method desc table when CoreLib is prejitted. TADDR m_pSig; DWORD m_cSig; -#ifdef TARGET_64BIT + +protected: // m_dwExtendedFlags is not used by StoredSigMethodDesc itself. // It is used by child classes. We allocate the space here to get // optimal layout. DWORD m_dwExtendedFlags; -#endif +public: TADDR GetSigRVA() { LIMITED_METHOD_DAC_CONTRACT; @@ -2477,40 +2478,88 @@ class DynamicMethodDesc : public StoredSigMethodDesc PTR_CUTF8 m_pszMethodName; PTR_DynamicResolver m_pResolver; -#ifndef TARGET_64BIT - // We use m_dwExtendedFlags from StoredSigMethodDesc on WIN64 - DWORD m_dwExtendedFlags; // see DynamicMethodDesc::ExtendedFlags enum +public: + enum ILStubType : DWORD + { + StubNotSet = 0, + StubCLRToNativeInterop, + StubCLRToCOMInterop, + StubNativeToCLRInterop, + StubCOMToCLRInterop, + StubStructMarshalInterop, +#ifdef FEATURE_ARRAYSTUB_AS_IL + StubArrayOp, +#endif +#ifdef FEATURE_MULTICASTSTUB_AS_IL + StubMulticastDelegate, #endif + StubWrapperDelegate, +#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL + StubUnboxingIL, + StubInstantiating, +#endif + StubTailCallStoreArgs, + StubTailCallCallTarget, + + StubLast + }; + + enum Flag : DWORD + { + // Flags for DynamicMethodDesc + // Define new flags in descending order. This allows the IL type enumeration to increase naturally. + FlagNone = 0x00000000, + FlagPublic = 0x00000400, + FlagStatic = 0x00000800, + FlagUnbreakable = 0x00001000, + FlagRequiresCOM = 0x00002000, + FlagIsLCGMethod = 0x00004000, + FlagIsILStub = 0x00008000, + FlagIsDelegate = 0x00010000, + FlagIsCALLI = 0x00020000, + FlagMask = 0x0003fc00, + StackArgSizeMask = 0xfffc0000, // native stack arg size for IL stubs + ILStubTypeMask = ~(FlagMask | StackArgSizeMask) + }; + static_assert_no_msg((FlagMask & StubLast) == 0); + static_assert_no_msg((StackArgSizeMask & FlagMask) == 0); + + // MethodDesc memory is acquired in an uninitialized state. + // The first step should be to explicitly set the entire + // flag state and then modify it. + void InitializeFlags(DWORD flags) + { + m_dwExtendedFlags = flags; + } + bool AreFlagSets(DWORD flags) const + { + return !!(m_dwExtendedFlags & flags); + } + void SetFlags(DWORD flags) + { + m_dwExtendedFlags |= flags; + } + void ClearFlags(DWORD flags) + { + m_dwExtendedFlags = (m_dwExtendedFlags & ~flags); + } + + ILStubType GetILStubType() const + { + ILStubType type = (ILStubType)(m_dwExtendedFlags & ILStubTypeMask); + _ASSERTE(type == StubNotSet || AreFlagSets(FlagIsILStub)); + return type; + } - typedef enum ExtendedFlags - { - nomdAttrs = 0x0000FFFF, // method attributes (LCG) - nomdILStubAttrs = mdMemberAccessMask | mdStatic, // method attributes (IL stubs) - - // attributes (except mdStatic and mdMemberAccessMask) have different meaning for IL stubs - // mdMemberAccessMask = 0x0007, - nomdReverseStub = 0x0008, - // mdStatic = 0x0010, - nomdCALLIStub = 0x0020, - nomdDelegateStub = 0x0040, - nomdStructMarshalStub = 0x0080, - nomdUnbreakable = 0x0100, - nomdStepThroughStub = 0x0200, // Stub has no user steppable code and should be stepped through. - //unused = 0x0400, - nomdStubNeedsCOMStarted = 0x0800, // EnsureComStarted must be called before executing the method - nomdMulticastStub = 0x1000, - nomdUnboxingILStub = 0x2000, - nomdWrapperDelegateStub = 0x4000, - nomdUnmanagedCallersOnlyStub = 0x8000, - - nomdILStub = 0x00010000, - nomdLCGMethod = 0x00020000, - nomdStackArgSize = 0xFFFC0000, // native stack arg size for IL stubs - } ExtendedFlags; + void SetILStubType(ILStubType type) + { + _ASSERTE(AreFlagSets(FlagIsILStub)); + m_dwExtendedFlags |= type; + } public: - bool IsILStub() { LIMITED_METHOD_DAC_CONTRACT; return !!(m_dwExtendedFlags & nomdILStub); } - bool IsLCGMethod() { LIMITED_METHOD_DAC_CONTRACT; return !!(m_dwExtendedFlags & nomdLCGMethod); } + bool IsILStub() const { LIMITED_METHOD_DAC_CONTRACT; return AreFlagSets(FlagIsILStub); } + bool IsLCGMethod() const { LIMITED_METHOD_DAC_CONTRACT; return AreFlagSets(FlagIsLCGMethod); } inline PTR_DynamicResolver GetResolver(); inline PTR_LCGMethodResolver GetLCGMethodResolver(); @@ -2522,23 +2571,21 @@ class DynamicMethodDesc : public StoredSigMethodDesc return m_pszMethodName; } - WORD GetAttrs() - { - LIMITED_METHOD_CONTRACT; - return (IsILStub() ? (m_dwExtendedFlags & nomdILStubAttrs) : (m_dwExtendedFlags & nomdAttrs)); - } - - DWORD GetExtendedFlags() + // Baseed on the current flags, compute the equivalent as COR metaadata. + WORD GetFlagsAsMetadata() const { LIMITED_METHOD_CONTRACT; - return m_dwExtendedFlags; + WORD asMetadata = 0; + asMetadata |= AreFlagSets(FlagPublic) ? mdPublic : 0; + asMetadata |= AreFlagSets(FlagStatic) ? mdStatic : 0; + return asMetadata; } WORD GetNativeStackArgSize() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); - return (WORD)((m_dwExtendedFlags & nomdStackArgSize) >> 16); + return (WORD)((m_dwExtendedFlags & StackArgSizeMask) >> 16); } void SetNativeStackArgSize(WORD cbArgSize) @@ -2548,86 +2595,80 @@ class DynamicMethodDesc : public StoredSigMethodDesc #if !defined(OSX_ARM64_ABI) _ASSERTE((cbArgSize % TARGET_POINTER_SIZE) == 0); #endif - m_dwExtendedFlags = (m_dwExtendedFlags & ~nomdStackArgSize) | ((DWORD)cbArgSize << 16); + m_dwExtendedFlags = (m_dwExtendedFlags & ~StackArgSizeMask) | ((DWORD)cbArgSize << 16); } - void SetUnbreakable(bool value) + bool IsReverseStub() const { - LIMITED_METHOD_CONTRACT; - if (value) - { - m_dwExtendedFlags |= nomdUnbreakable; - } + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(IsILStub()); + ILStubType type = GetILStubType(); + return type == StubCOMToCLRInterop || type == StubNativeToCLRInterop; } - void SetStubNeedsCOMStarted(bool value) + bool IsDelegateStub() const { - LIMITED_METHOD_CONTRACT; - if (value) - { - m_dwExtendedFlags |= nomdStubNeedsCOMStarted; - } + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(IsILStub()); + return AreFlagSets(FlagIsDelegate); } - bool IsReverseStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdReverseStub)); } - bool IsUnmanagedCallersOnlyStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdUnmanagedCallersOnlyStub)); } - bool IsCALLIStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdCALLIStub)); } - bool IsDelegateStub() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdDelegateStub)); } - bool IsUnbreakable() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdUnbreakable)); } - bool IsStubNeedsCOMStarted() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStubNeedsCOMStarted)); } - bool IsStructMarshalStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStructMarshalStub)); } - bool IsStepThroughStub() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStepThroughStub)); } + bool IsStepThroughStub() const + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsILStub()); + ILStubType type = GetILStubType(); + return type == StubUnboxingIL || type == StubInstantiating; + } - // The following checks don't have explicit indicates of type, but rather use a heuristic. - // Therefore, we break them out so they are clearer. - bool IsCLRToCOMStub() + bool IsCLRToCOMStub() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - return ((0 == (m_dwExtendedFlags & mdStatic)) - && !IsReverseStub() - && !IsDelegateStub() - && !IsStructMarshalStub()); + return !AreFlagSets(FlagStatic) && GetILStubType() == StubCLRToCOMInterop; } - bool IsCOMToCLRStub() + bool IsCOMToCLRStub() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - return ((0 == (m_dwExtendedFlags & mdStatic)) - && IsReverseStub()); + return !AreFlagSets(FlagStatic) && GetILStubType() == StubCOMToCLRInterop; } - bool IsPInvokeStub() + bool IsPInvokeStub() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - return ((0 != (m_dwExtendedFlags & mdStatic)) - && !IsReverseStub() - && !IsCALLIStub() - && !IsStructMarshalStub()); + return AreFlagSets(FlagStatic) && GetILStubType() == StubCLRToNativeInterop; } #ifdef FEATURE_MULTICASTSTUB_AS_IL - bool IsMulticastStub() { + bool IsMulticastStub() const + { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); - return !!(m_dwExtendedFlags & nomdMulticastStub); + return GetILStubType() == DynamicMethodDesc::StubMulticastDelegate; } #endif - bool IsWrapperDelegateStub() { + bool IsWrapperDelegateStub() const + { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); - return !!(m_dwExtendedFlags & nomdWrapperDelegateStub); + return GetILStubType() == DynamicMethodDesc::StubWrapperDelegate; } #ifdef FEATURE_INSTANTIATINGSTUB_AS_IL - bool IsUnboxingILStub() { + bool IsUnboxingILStub() const + { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); - return !!(m_dwExtendedFlags & nomdUnboxingILStub); + return GetILStubType() == DynamicMethodDesc::StubUnboxingIL; } #endif // Whether the stub takes a context argument that is an interop MethodDesc. - bool HasMDContextArg(); + bool HasMDContextArg() const + { + LIMITED_METHOD_CONTRACT; + return IsCLRToCOMStub() || (IsPInvokeStub() && !IsDelegateStub()); + } // // following implementations defined in DynamicMethod.cpp diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 32f081b2eaa891..3a712e26f06316 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -388,18 +388,14 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) DynamicMethodDesc* stubMethodDesc = this->AsDynamicMethodDesc(); if (stubMethodDesc->IsILStub() && stubMethodDesc->IsPInvokeStub()) { - ILStubResolver* pStubResolver = stubMethodDesc->GetILStubResolver(); - if (pStubResolver->GetStubType() == ILStubResolver::CLRToNativeInteropStub) + MethodDesc* pTargetMD = stubMethodDesc->GetILStubResolver()->GetStubTargetMethodDesc(); + if (pTargetMD != NULL) { - MethodDesc* pTargetMD = stubMethodDesc->GetILStubResolver()->GetStubTargetMethodDesc(); - if (pTargetMD != NULL) + pCode = pTargetMD->GetPrecompiledR2RCode(pConfig); + if (pCode != NULL) { - pCode = pTargetMD->GetPrecompiledR2RCode(pConfig); - if (pCode != NULL) - { - LOG_USING_R2R_CODE(this); - pConfig->SetNativeCode(pCode, &pCode); - } + LOG_USING_R2R_CODE(this); + pConfig->SetNativeCode(pCode, &pCode); } } } diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 256a3e838b5ac5..4f3028d77e1e43 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -1733,7 +1733,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Forward delegate P/Invoke case 0x%p\n", target)); trace->InitForUnmanaged(target); } - else if (pStubMD->IsCALLIStub()) + else if (pStubMD->AreFlagSets(DynamicMethodDesc::FlagIsCALLI)) { // This is unmanaged CALLI stub, the argument is the target target = (PCODE)arg; diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 61dbae0ca95603..5dfa0b061b2ba5 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -773,7 +773,7 @@ StackWalkAction TAStackCrawlCallBack(CrawlFrame* pCf, void* data) // Does the current and latched frame represent the same call? if (pCf->pFrame == pData->LatchedCF.pFrame) { - if (pData->LatchedCF.GetFunction()->AsDynamicMethodDesc()->IsUnbreakable()) + if (pData->LatchedCF.GetFunction()->AsDynamicMethodDesc()->AreFlagSets(DynamicMethodDesc::FlagUnbreakable)) { // Report only the latched IL stub frame which is a CER root. frameAction = DiscardCurrentFrame; @@ -808,7 +808,7 @@ StackWalkAction TAStackCrawlCallBack(CrawlFrame* pCf, void* data) MethodDesc *pMD = pCf->GetFunction(); if (pMD != NULL && pMD->IsILStub() && pData->LatchedCF.GetFrame()->GetReturnAddress() == GetControlPC(pCf->GetRegisterSet()) && - pMD->AsDynamicMethodDesc()->IsUnbreakable()) + pMD->AsDynamicMethodDesc()->AreFlagSets(DynamicMethodDesc::FlagUnbreakable)) { // The current and latched frame represent the same call and the IL stub is marked as unbreakable. // We will discard the interop method and report only the IL stub which is a CER root. @@ -2908,9 +2908,9 @@ BOOL Thread::RedirectThreadAtHandledJITCase(PFN_REDIRECTTARGET pTgt) #if defined(TARGET_X86) || defined(TARGET_AMD64) // Scenarios like GC stress may indirectly disable XState features in the pCtx // depending on the state at the time of GC stress interrupt. - // + // // Make sure that AVX feature mask is set, if supported. - // + // // This should not normally fail. // The system silently ignores any feature specified in the FeatureMask // which is not enabled on the processor. From 32ab9eab4e16005070a9a47e353aa155b73fe66f Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 1 Mar 2022 17:18:29 -0800 Subject: [PATCH 06/11] Review feedback. --- src/coreclr/debug/daccess/fntableaccess.h | 47 ---------- src/coreclr/debug/ee/controller.cpp | 2 +- src/coreclr/vm/codeman.cpp | 2 +- src/coreclr/vm/common.h | 1 - src/coreclr/vm/prestub.cpp | 4 +- src/coreclr/vm/stublink.cpp | 79 ++++++++-------- src/coreclr/vm/stublink.h | 109 +++++++++++++--------- src/coreclr/vm/stublink.inl | 2 +- src/coreclr/vm/stubmgr.cpp | 23 +++-- 9 files changed, 120 insertions(+), 149 deletions(-) diff --git a/src/coreclr/debug/daccess/fntableaccess.h b/src/coreclr/debug/daccess/fntableaccess.h index b89880caf93c3c..a9ac83a0ae88c2 100644 --- a/src/coreclr/debug/daccess/fntableaccess.h +++ b/src/coreclr/debug/daccess/fntableaccess.h @@ -100,30 +100,6 @@ struct FakeStubUnwindInfoHeapSegment FakeStubUnwindInfoHeapSegment *pNext; }; -#define FAKE_STUB_EXTERNAL_ENTRY_BIT 0x40000000 -#define FAKE_STUB_UNWIND_INFO_BIT 0x08000000 - -#ifdef _DEBUG -#define FAKE_STUB_SIGNATURE 0x42555453 -#endif - -struct FakeStub -{ - ULONG m_refcount; - ULONG m_Offset; - - UINT m_numCodeBytes; -#ifdef _DEBUG - UINT32 m_signature; -#else -#ifdef HOST_64BIT - //README ALIGNMENT: in retail mode UINT m_numCodeBytes does not align to 16byte for the code - // after the Stub struct. This is to pad properly - UINT m_pad_code_bytes; -#endif // HOST_64BIT -#endif // _DEBUG -}; - #endif // DEBUGSUPPORT_STUBS_HAVE_UNWIND_INFO @@ -166,32 +142,9 @@ class CheckDuplicatedStructLayouts CHECK_OFFSET(StubUnwindInfoHeapSegment, pUnwindHeaderList); CHECK_OFFSET(StubUnwindInfoHeapSegment, pNext); - - CHECK_OFFSET(Stub, m_refcount); - CHECK_OFFSET(Stub, m_Offset); - CHECK_OFFSET(Stub, m_numCodeBytes); -#ifdef _DEBUG - CHECK_OFFSET(Stub, m_signature); -#endif // _DEBUG - #endif // DEBUGSUPPORT_STUBS_HAVE_UNWIND_INFO #undef CHECK_OFFSET - -#ifdef DEBUGSUPPORT_STUBS_HAVE_UNWIND_INFO - - static_assert_no_msg( Stub::EXTERNAL_ENTRY_BIT - == FAKE_STUB_EXTERNAL_ENTRY_BIT); - - static_assert_no_msg( Stub::UNWIND_INFO_BIT - == FAKE_STUB_UNWIND_INFO_BIT); - -#ifdef _DEBUG - static_assert_no_msg( FAKE_STUB_SIGNATURE - == Stub::kUsedStub); -#endif - -#endif // DEBUGSUPPORT_STUBS_HAVE_UNWIND_INFO }; #ifdef DEBUGSUPPORT_STUBS_HAVE_UNWIND_INFO diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index d1856928bb77a0..acc35ec3ba0339 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -593,7 +593,7 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll _ASSERTE(kind == PATCH_KIND_NATIVE_MANAGED || kind == PATCH_KIND_NATIVE_UNMANAGED); LOG((LF_CORDB,LL_INFO10000,"DCP:AddPatchForAddress bound " "absolute to 0x%p with dji 0x%p (mdDef:0x%x) " - "controller:0x%xp AD:0x%p\n", + "controller:0x%p AD:0x%p\n", address, dji, (fd!=NULL?fd->GetMemberDef():0), controller, pAppDomain)); diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 633c135b60a95e..d039d00a7c2c97 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -286,7 +286,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R // Add to the function table pRtlGrowFunctionTable(unwindInfo->hHandle, unwindInfo->cTableCurCount); - STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] ADDING 0x%xp TO END, now 0x%x entries\n", + STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] ADDING 0x%p TO END, now 0x%x entries\n", unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, data->BeginAddress, unwindInfo->cTableCurCount); return; diff --git a/src/coreclr/vm/common.h b/src/coreclr/vm/common.h index b47119e51d83f1..eb37c0a0cac94b 100644 --- a/src/coreclr/vm/common.h +++ b/src/coreclr/vm/common.h @@ -138,7 +138,6 @@ typedef VPTR(struct IUnknown) PTR_IUnknown; typedef DPTR(class InstMethodHashTable) PTR_InstMethodHashTable; typedef DPTR(class MetaSig) PTR_MetaSig; typedef DPTR(class MethodDesc) PTR_MethodDesc; -typedef DPTR(PTR_MethodDesc) PTR_PTR_MethodDesc; typedef DPTR(class MethodDescChunk) PTR_MethodDescChunk; typedef DPTR(class MethodImpl) PTR_MethodImpl; typedef DPTR(class MethodTable) PTR_MethodTable; diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 3a712e26f06316..8731342137ec1f 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -1679,7 +1679,7 @@ Stub * MakeUnboxingStubWorker(MethodDesc *pMD) sl.EmitComputedInstantiatingMethodStub(pUnboxedMD, &portableShuffle[0], NULL); pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_INSTANTIATING_METHOD); - pstub->SetMethodDesc(pUnboxedMD); + pstub->SetInstantiatedMethodDesc(pUnboxedMD); } else #endif @@ -1753,7 +1753,7 @@ Stub * MakeInstantiatingStubWorker(MethodDesc *pMD) sl.EmitComputedInstantiatingMethodStub(pSharedMD, &portableShuffle[0], extraArg); pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_INSTANTIATING_METHOD); - pstub->SetMethodDesc(pSharedMD); + pstub->SetInstantiatedMethodDesc(pSharedMD); } else #endif diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index 75c63c27b7cd78..6060d3c8572d26 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -1087,10 +1087,10 @@ bool StubLinker::EmitStub(Stub* pStub, int globalsize, int totalSize, LoaderHeap { UINT32 uLabelOffset = GetLabelOffset(m_pPatchLabel); _ASSERTE(FitsIn(uLabelOffset)); - pStubRW->SetOffset(static_cast(uLabelOffset)); + pStubRW->SetPatchOffset(static_cast(uLabelOffset)); LOG((LF_CORDB, LL_INFO100, "SL::ES: patch offset:0x%x\n", - pStub->GetOffset())); + pStub->GetPatchOffset())); } #ifdef STUBLINKER_GENERATES_UNWIND_INFO @@ -2001,14 +2001,11 @@ VOID Stub::DeleteStub() } #endif - // a size of 0 is a signal to Nirvana to flush the entire cache - //FlushInstructionCache(GetCurrentProcess(),0,0); - - if ((m_Offset & LOADER_HEAP_BIT) == 0) + if ((m_numCodeBytesAndFlags & LOADER_HEAP_BIT) == 0) { #ifdef _DEBUG m_signature = kFreedStub; - FillMemory(this+1, m_numCodeBytes, 0xcc); + FillMemory(this+1, GetNumCodeBytes(), 0xcc); #endif delete [] (BYTE*)GetAllocationBase(); @@ -2035,7 +2032,7 @@ TADDR Stub::GetAllocationBase() PTR_StubUnwindInfoHeaderSuffix(info - cbPrefix - sizeof(*pSuffix)); - cbPrefix += StubUnwindInfoHeader::ComputeSize(pSuffix->nUnwindInfoSize); + cbPrefix += StubUnwindInfoHeader::ComputeAlignedSize(pSuffix->nUnwindInfoSize); } #endif // STUBLINKER_GENERATES_UNWIND_INFO @@ -2057,6 +2054,10 @@ Stub* Stub::NewStub(PTR_VOID pCode, DWORD flags) CONTRACTL_END; Stub* pStub = NewStub(NULL, 0, flags | NEWSTUB_FL_EXTERNAL); + + // Passing NEWSTUB_FL_EXTERNAL requests the stub struct be + // expanded in size by a single pointer. Insert the code point at this + // location. *(PTR_VOID *)(pStub + 1) = pCode; return pStub; @@ -2081,26 +2082,30 @@ Stub* Stub::NewStub(PTR_VOID pCode, DWORD flags) } CONTRACTL_END; - if (flags & NEWSTUB_FL_EXTERNAL) - { - _ASSERTE(pHeap == NULL); - } + // The memory layout of the allocated memory for the Stub instance is as follows: + // Offset: + // - 0 + // optional: unwind info - see nUnwindInfoSize usage. + // - stubPayloadOffset + // Stub instance + // optional: external pointer | padding + code + size_t stubPayloadOffset = 0; + S_SIZE_T size = S_SIZE_T(sizeof(Stub)); #ifdef STUBLINKER_GENERATES_UNWIND_INFO _ASSERTE(!nUnwindInfoSize || !pHeap || pHeap->m_fPermitStubsWithUnwindInfo); -#endif // STUBLINKER_GENERATES_UNWIND_INFO - S_SIZE_T size = S_SIZE_T(sizeof(Stub)); - -#ifdef STUBLINKER_GENERATES_UNWIND_INFO if (nUnwindInfoSize != 0) { - size += StubUnwindInfoHeader::ComputeSize(nUnwindInfoSize); + // The Unwind info precedes the Stub itself. + stubPayloadOffset = StubUnwindInfoHeader::ComputeAlignedSize(nUnwindInfoSize); + size += stubPayloadOffset; } -#endif +#endif // STUBLINKER_GENERATES_UNWIND_INFO if (flags & NEWSTUB_FL_EXTERNAL) { + _ASSERTE(pHeap == NULL); _ASSERTE(numCodeBytes == 0); size += sizeof(PTR_PCODE); } @@ -2110,10 +2115,6 @@ Stub* Stub::NewStub(PTR_VOID pCode, DWORD flags) size += numCodeBytes; } - // Add pointer to store the target MethodDesc. - if (flags & NEWSTUB_FL_INSTANTIATING_METHOD) - size += sizeof(PTR_MethodDesc); - if (size.IsOverflow()) COMPlusThrowArithmetic(); @@ -2131,12 +2132,7 @@ Stub* Stub::NewStub(PTR_VOID pCode, DWORD flags) flags |= NEWSTUB_FL_LOADERHEAP; } - size_t stubPayloadOffset = totalSize - - (sizeof(Stub) - + ((flags & NEWSTUB_FL_EXTERNAL) ? sizeof(PTR_PCODE) : numCodeBytes) - + ((flags & NEWSTUB_FL_INSTANTIATING_METHOD) ? sizeof(PTR_MethodDesc) : 0)); - - // Make sure that the payload of the stub is aligned + _ASSERTE((stubPayloadOffset % CODE_SIZE_ALIGN) == 0); Stub* pStubRX = (Stub*)(pBlock + stubPayloadOffset); Stub* pStubRW; ExecutableWriterHolder stubWriterHolder; @@ -2171,43 +2167,44 @@ void Stub::SetupStub(int numCodeBytes, DWORD flags { CONTRACTL { - NOTHROW; + THROWS; GC_NOTRIGGER; } CONTRACTL_END; #ifdef _DEBUG m_signature = kUsedStub; -#else #ifdef HOST_64BIT - m_pad_code_bytes = 0; + m_pad_code_bytes1 = 0; + m_pad_code_bytes2 = 0; + m_pad_code_bytes3 = 0; #endif #endif - m_numCodeBytes = numCodeBytes; + if (numCodeBytes >= MAX_CODEBYTES) + COMPlusThrowHR(COR_E_OVERFLOW); + + m_numCodeBytesAndFlags = numCodeBytes; m_refcount = 1; - m_Offset = 0; + m_data = {}; if (flags != NEWSTUB_FL_NONE) { if((flags & NEWSTUB_FL_LOADERHEAP) != 0) - m_Offset |= LOADER_HEAP_BIT; + m_numCodeBytesAndFlags |= LOADER_HEAP_BIT; if((flags & NEWSTUB_FL_MULTICAST) != 0) - m_Offset |= MULTICAST_DELEGATE_BIT; + m_numCodeBytesAndFlags |= MULTICAST_DELEGATE_BIT; if ((flags & NEWSTUB_FL_EXTERNAL) != 0) - m_Offset |= EXTERNAL_ENTRY_BIT; + m_numCodeBytesAndFlags |= EXTERNAL_ENTRY_BIT; if ((flags & NEWSTUB_FL_INSTANTIATING_METHOD) != 0) - { - m_Offset |= INSTANTIATING_METHOD_BIT; - SetOffset(m_numCodeBytes); - } + m_numCodeBytesAndFlags |= INSTANTIATING_STUB_BIT; } #ifdef STUBLINKER_GENERATES_UNWIND_INFO if (nUnwindInfoSize) { - m_Offset |= UNWIND_INFO_BIT; + m_numCodeBytesAndFlags |= UNWIND_INFO_BIT; StubUnwindInfoHeaderSuffix * pSuffix = GetUnwindInfoHeaderSuffix(); pSuffix->nUnwindInfoSize = (BYTE)nUnwindInfoSize; diff --git a/src/coreclr/vm/stublink.h b/src/coreclr/vm/stublink.h index 208acb3868ad70..1f1e363c2a2dac 100644 --- a/src/coreclr/vm/stublink.h +++ b/src/coreclr/vm/stublink.h @@ -91,7 +91,7 @@ struct StubUnwindInfoHeader UNWIND_INFO UnwindInfo; // variable length // Computes the size needed for this variable-sized struct. - static SIZE_T ComputeSize(UINT nUnwindInfoSize); + static SIZE_T ComputeAlignedSize(UINT nUnwindInfoSize); void Init (); @@ -465,21 +465,18 @@ class Stub friend class CheckAsmOffsets; protected: - - // These values are shared with the debugger - see FakeStub. enum { MULTICAST_DELEGATE_BIT = 0x80000000, EXTERNAL_ENTRY_BIT = 0x40000000, LOADER_HEAP_BIT = 0x20000000, - INSTANTIATING_METHOD_BIT= 0x10000000, + INSTANTIATING_STUB_BIT= 0x10000000, UNWIND_INFO_BIT = 0x08000000, - OFFSET_MASK = UNWIND_INFO_BIT - 1, - MAX_OFFSET = OFFSET_MASK + 1, + CODEBYTES_MASK = UNWIND_INFO_BIT - 1, + MAX_CODEBYTES = CODEBYTES_MASK + 1, }; - - static_assert_no_msg(OFFSET_MASK < UNWIND_INFO_BIT); + static_assert_no_msg(CODEBYTES_MASK < UNWIND_INFO_BIT); public: //------------------------------------------------------------------- @@ -512,37 +509,40 @@ class Stub BOOL IsMulticastDelegate() { LIMITED_METHOD_CONTRACT; - return (m_Offset & MULTICAST_DELEGATE_BIT) != 0; + return (m_numCodeBytesAndFlags & MULTICAST_DELEGATE_BIT) != 0; } //------------------------------------------------------------------- // Used by the debugger to help step through stubs //------------------------------------------------------------------- - BOOL IsInstantiatingMethodStub() - { - LIMITED_METHOD_CONTRACT; - return (m_Offset & INSTANTIATING_METHOD_BIT) != 0; - } - - USHORT GetOffset() + BOOL IsInstantiatingStub() { LIMITED_METHOD_CONTRACT; - return (USHORT)(m_Offset & OFFSET_MASK); + return (m_numCodeBytesAndFlags & INSTANTIATING_STUB_BIT) != 0; } - void SetOffset(USHORT offset) + //------------------------------------------------------------------- + // For stubs which execute user code, a patch offset needs to be set + // to tell the debugger how far into the stub code the debugger has + // to step until the frame is set up. + //------------------------------------------------------------------- + void SetPatchOffset(USHORT offset) { LIMITED_METHOD_CONTRACT; - _ASSERTE(GetOffset() == 0); - m_Offset |= offset; - _ASSERTE(GetOffset() == offset); + _ASSERTE(!IsInstantiatingStub()); + m_data.PatchOffset = offset; } - void SetMethodDesc(PTR_MethodDesc pMD) + //------------------------------------------------------------------- + // For stubs which execute user code, a patch offset needs to be set + // to tell the debugger how far into the stub code the debugger has + // to step until the frame is set up. + //------------------------------------------------------------------- + USHORT GetPatchOffset() { LIMITED_METHOD_CONTRACT; - _ASSERTE(IsInstantiatingMethodStub()); - *GetMethodDescLocation() = pMD; + _ASSERTE(!IsInstantiatingStub()); + return m_data.PatchOffset; } //------------------------------------------------------------------- @@ -552,9 +552,21 @@ class Stub //------------------------------------------------------------------- TADDR GetPatchAddress() { - WRAPPER_NO_CONTRACT; - _ASSERTE(!IsInstantiatingMethodStub()); - return dac_cast(GetEntryPointInternal()) + GetOffset(); + LIMITED_METHOD_CONTRACT; + _ASSERTE(!IsInstantiatingStub()); + return dac_cast(GetEntryPointInternal()) + GetPatchOffset(); + } + + //------------------------------------------------------------------- + // For instantiating methods, the target MethodDesc needs to be set + // to tell the debugger where to step through the instantiating method + // stub. + //------------------------------------------------------------------- + void SetInstantiatedMethodDesc(PTR_MethodDesc pMD) + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsInstantiatingStub()); + m_data.InstantiatedMethod = pMD; } //------------------------------------------------------------------- @@ -562,11 +574,11 @@ class Stub // to tell the debugger where to step through the instantiating method // stub. //------------------------------------------------------------------- - PTR_PTR_MethodDesc GetMethodDescLocation() + PTR_MethodDesc GetInstantiatedMethodDesc() { LIMITED_METHOD_CONTRACT; - _ASSERTE(IsInstantiatingMethodStub()); - return dac_cast(GetEntryPointInternal() + GetOffset()); + _ASSERTE(IsInstantiatingStub()); + return m_data.InstantiatedMethod; } //------------------------------------------------------------------- @@ -578,7 +590,7 @@ class Stub BOOL HasUnwindInfo() { LIMITED_METHOD_CONTRACT; - return (m_Offset & UNWIND_INFO_BIT) != 0; + return (m_numCodeBytesAndFlags & UNWIND_INFO_BIT) != 0; } StubUnwindInfoHeaderSuffix *GetUnwindInfoHeaderSuffix() @@ -609,12 +621,14 @@ class Stub } CONTRACTL_END + _ASSERTE(HasUnwindInfo()); + StubUnwindInfoHeaderSuffix *pSuffix = GetUnwindInfoHeaderSuffix(); TADDR suffixEnd = dac_cast(pSuffix) + sizeof(*pSuffix); return PTR_StubUnwindInfoHeader(suffixEnd - - StubUnwindInfoHeader::ComputeSize(pSuffix->nUnwindInfoSize)); + StubUnwindInfoHeader::ComputeAlignedSize(pSuffix->nUnwindInfoSize)); } #endif // STUBLINKER_GENERATES_UNWIND_INFO @@ -654,7 +668,7 @@ class Stub WRAPPER_NO_CONTRACT; SUPPORTS_DAC; - return m_numCodeBytes; + return (m_numCodeBytesAndFlags & CODEBYTES_MASK); } //------------------------------------------------------------------- @@ -690,7 +704,7 @@ class Stub CONTRACT_END; Stub *pStub = Stub::RecoverStub(pEntryPoint); - *pSize = sizeof(Stub) + pStub->m_numCodeBytes; + *pSize = sizeof(Stub) + pStub->GetNumCodeBytes(); RETURN pStub; } @@ -698,12 +712,12 @@ class Stub { LIMITED_METHOD_CONTRACT; if ((pBuffer == NULL) || - (dwBufferSize < (sizeof(*this) + m_numCodeBytes))) + (dwBufferSize < (sizeof(*this) + GetNumCodeBytes()))) { return E_INVALIDARG; } - memcpyNoGCRefs(pBuffer, this, sizeof(*this) + m_numCodeBytes); + memcpyNoGCRefs(pBuffer, this, sizeof(*this) + GetNumCodeBytes()); reinterpret_cast(pBuffer)->m_refcount = 1; return S_OK; @@ -746,7 +760,7 @@ class Stub { LIMITED_METHOD_CONTRACT; - return (m_Offset & EXTERNAL_ENTRY_BIT) != 0; + return (m_numCodeBytesAndFlags & EXTERNAL_ENTRY_BIT) != 0; } //------------------------------------------------------------------- @@ -801,10 +815,13 @@ class Stub } } - ULONG m_refcount; - ULONG m_Offset; - - UINT m_numCodeBytes; + UINT32 m_refcount; + UINT32 m_numCodeBytesAndFlags; + union + { + USHORT PatchOffset; + PTR_MethodDesc InstantiatedMethod; + } m_data; #ifdef _DEBUG enum { @@ -813,17 +830,17 @@ class Stub }; UINT32 m_signature; -#else #ifdef HOST_64BIT - //README ALIGNMENT: in retail mode UINT m_numCodeBytes does not align to 16byte for the code - // after the Stub struct. This is to pad properly - UINT m_pad_code_bytes; + //README ALIGNMENT: Enusure code after the Stub struct align to 16-bytes. + UINT32 m_pad_code_bytes1; + UINT32 m_pad_code_bytes2; + UINT32 m_pad_code_bytes3; #endif // HOST_64BIT #endif // _DEBUG Stub() = delete; // Stubs are created by NewStub(), not "new". }; - +static_assert_no_msg(sizeof(Stub) % CODE_SIZE_ALIGN == 0); //------------------------------------------------------------------------- // Each platform encodes the "branch" instruction in a different diff --git a/src/coreclr/vm/stublink.inl b/src/coreclr/vm/stublink.inl index 213aa594ad5953..aaaab7b82eeada 100644 --- a/src/coreclr/vm/stublink.inl +++ b/src/coreclr/vm/stublink.inl @@ -18,7 +18,7 @@ #ifdef STUBLINKER_GENERATES_UNWIND_INFO inline //static -SIZE_T StubUnwindInfoHeader::ComputeSize(UINT nUnwindInfoSize) +SIZE_T StubUnwindInfoHeader::ComputeAlignedSize(UINT nUnwindInfoSize) { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 4f3028d77e1e43..30f41bdc6215ea 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -1195,14 +1195,15 @@ BOOL StubLinkStubManager::DoTraceStub(PCODE stubStartAddress, LOG_TRACE_DESTINATION(trace, stubStartAddress, "StubLinkStubManager(MCDel)::DoTraceStub"); return TRUE; } - else if (stub->IsInstantiatingMethodStub()) + else if (stub->IsInstantiatingStub()) { trace->InitForManagerPush(stubStartAddress, this); LOG_TRACE_DESTINATION(trace, stubStartAddress, "StubLinkStubManager(InstantiatingMethod)::DoTraceStub"); return TRUE; } - else if (stub->GetOffset() != 0) + else if (stub->GetPatchOffset() != 0) { + // The patch offset is currently only non-zero in x86 non-IL delegate scenarios. trace->InitForFramePush((PCODE)stub->GetPatchAddress()); LOG_TRACE_DESTINATION(trace, stubStartAddress, "StubLinkStubManager::DoTraceStub"); return TRUE; @@ -1221,6 +1222,7 @@ static PCODE GetStubTarget(PTR_MethodDesc pTargetMD) THROWS; GC_TRIGGERS; MODE_COOPERATIVE; + PRECONDITION(pTargetMD != NULL); } CONTRACTL_END; @@ -1260,17 +1262,17 @@ BOOL StubLinkStubManager::TraceManager(Thread *thread, LOG((LF_CORDB,LL_INFO10000, "SLSM:TM 0x%p, retAddr is 0x%p\n", pc, (*pRetAddr))); Stub *stub = Stub::RecoverStub((PCODE)pc); - if (stub->IsInstantiatingMethodStub()) + if (stub->IsInstantiatingStub()) { LOG((LF_CORDB,LL_INFO10000, "SLSM:TM Instantiating method stub\n")); - PTR_PTR_MethodDesc ppMD = stub->GetMethodDescLocation(); - _ASSERTE(*ppMD != NULL); + PTR_MethodDesc pMD = stub->GetInstantiatedMethodDesc(); + _ASSERTE(pMD != NULL); - PCODE target = GetStubTarget(*ppMD); + PCODE target = GetStubTarget(pMD); if (target == NULL) { - LOG((LF_CORDB,LL_INFO10000, "SLSM:TM Unable to determine stub target, ppMD 0x%p\n", ppMD)); - trace->InitForUnjittedMethod(*ppMD); + LOG((LF_CORDB,LL_INFO10000, "SLSM:TM Unable to determine stub target, fd 0x%p\n", pMD)); + trace->InitForUnjittedMethod(pMD); return TRUE; } @@ -1284,6 +1286,9 @@ BOOL StubLinkStubManager::TraceManager(Thread *thread, return DelegateInvokeStubManager::TraceDelegateObject(pbDel, trace); } + // Runtime bug if we get here. Did we make a change in StubLinkStubManager::DoTraceStub() that + // dispatched new stubs to TraceManager without writing the code to handle them? + _ASSERTE(!"SLSM:TM wasn't expected to handle any other stub types"); return FALSE; } @@ -2109,7 +2114,7 @@ BOOL DelegateInvokeStubManager::TraceManager(Thread *thread, TraceDestination *t // We use the patch offset field to indicate whether the stub has a hidden return buffer argument. // This field is set in SetupShuffleThunk(). - if (pStub->GetOffset() != 0) + if (pStub->GetPatchOffset() != 0) { // This stub has a hidden return buffer argument. orDelegate = (DELEGATEREF)ObjectToOBJECTREF(StubManagerHelpers::GetSecondArg(pContext)); From 3beeb051e57738c3e5ed6e6e676b813981413c1f Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 1 Mar 2022 17:26:38 -0800 Subject: [PATCH 07/11] Apply suggestions from code review --- src/coreclr/vm/method.hpp | 2 +- src/coreclr/vm/stublink.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 10dbd1f3df0224..aecf51b1eb6cf9 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2571,7 +2571,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc return m_pszMethodName; } - // Baseed on the current flags, compute the equivalent as COR metaadata. + // Based on the current flags, compute the equivalent as COR metadata. WORD GetFlagsAsMetadata() const { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/stublink.h b/src/coreclr/vm/stublink.h index 1f1e363c2a2dac..ebc8cec0476383 100644 --- a/src/coreclr/vm/stublink.h +++ b/src/coreclr/vm/stublink.h @@ -470,7 +470,7 @@ class Stub MULTICAST_DELEGATE_BIT = 0x80000000, EXTERNAL_ENTRY_BIT = 0x40000000, LOADER_HEAP_BIT = 0x20000000, - INSTANTIATING_STUB_BIT= 0x10000000, + INSTANTIATING_STUB_BIT = 0x10000000, UNWIND_INFO_BIT = 0x08000000, CODEBYTES_MASK = UNWIND_INFO_BIT - 1, From 76cc6ab3f3dfa8cf8a23ad7f2c3d5ab42f710702 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 1 Mar 2022 18:54:43 -0800 Subject: [PATCH 08/11] Remove FlagUnbreakable flag. Move CODE_SIZE_ALIGN to compilation unit. --- src/coreclr/vm/dllimport.cpp | 13 ------------- src/coreclr/vm/method.hpp | 16 +++++++++++----- src/coreclr/vm/stublink.cpp | 7 +++++++ src/coreclr/vm/stublink.h | 1 - src/coreclr/vm/threadsuspend.cpp | 27 +++------------------------ 5 files changed, 21 insertions(+), 43 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 7be3b32af83f57..f77303160a34b1 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -3825,19 +3825,6 @@ static void CreateNDirectStubWorker(StubState* pss, DEBUG_ARG(pSigDesc->m_pDebugClassName) ); - // If the return value is a SafeHandle or CriticalHandle, mark the stub method. - // Interop methods that use this stub will have an implicit reliability contract - // (see code:TAStackCrawlCallBack). - if (!SF_IsHRESULTSwapping(dwStubFlags)) - { - if (marshalType == MarshalInfo::MARSHAL_TYPE_SAFEHANDLE || - marshalType == MarshalInfo::MARSHAL_TYPE_CRITICALHANDLE) - { - if (pMD->IsDynamicMethod()) - pMD->AsDynamicMethodDesc()->SetFlags(DynamicMethodDesc::FlagUnbreakable); - } - } - if (SF_IsHRESULTSwapping(dwStubFlags)) { if (msig.GetReturnType() != ELEMENT_TYPE_VOID) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index aecf51b1eb6cf9..749528c7cc7848 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2509,15 +2509,14 @@ class DynamicMethodDesc : public StoredSigMethodDesc // Flags for DynamicMethodDesc // Define new flags in descending order. This allows the IL type enumeration to increase naturally. FlagNone = 0x00000000, - FlagPublic = 0x00000400, - FlagStatic = 0x00000800, - FlagUnbreakable = 0x00001000, + FlagPublic = 0x00000800, + FlagStatic = 0x00001000, FlagRequiresCOM = 0x00002000, FlagIsLCGMethod = 0x00004000, FlagIsILStub = 0x00008000, FlagIsDelegate = 0x00010000, FlagIsCALLI = 0x00020000, - FlagMask = 0x0003fc00, + FlagMask = 0x0003f800, StackArgSizeMask = 0xfffc0000, // native stack arg size for IL stubs ILStubTypeMask = ~(FlagMask | StackArgSizeMask) }; @@ -2617,8 +2616,15 @@ class DynamicMethodDesc : public StoredSigMethodDesc { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); + + bool isStepThrough = false; + +#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL ILStubType type = GetILStubType(); - return type == StubUnboxingIL || type == StubInstantiating; + isStepThrough = type == StubUnboxingIL || type == StubInstantiating; +#endif // FEATURE_INSTANTIATINGSTUB_AS_IL + + return isStepThrough; } bool IsCLRToCOMStub() const diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index 6060d3c8572d26..b9752b40018b6a 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -1872,6 +1872,13 @@ UINT StubLinker::GetStackFrameSize() #ifndef DACCESS_COMPILE +// Redeclaring the Stub type here and assert its size. +// The size assertion is done here because of where CODE_SIZE_ALIGN +// is defined - it is not included in all places where stublink.h +// is consumed. +class Stub; +static_assert_no_msg((sizeof(Stub) % CODE_SIZE_ALIGN) == 0); + //------------------------------------------------------------------- // Inc the refcount. //------------------------------------------------------------------- diff --git a/src/coreclr/vm/stublink.h b/src/coreclr/vm/stublink.h index ebc8cec0476383..7f7875da4c2672 100644 --- a/src/coreclr/vm/stublink.h +++ b/src/coreclr/vm/stublink.h @@ -840,7 +840,6 @@ class Stub Stub() = delete; // Stubs are created by NewStub(), not "new". }; -static_assert_no_msg(sizeof(Stub) % CODE_SIZE_ALIGN == 0); //------------------------------------------------------------------------- // Each platform encodes the "branch" instruction in a different diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 5dfa0b061b2ba5..0c5abd873f41a3 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -773,16 +773,8 @@ StackWalkAction TAStackCrawlCallBack(CrawlFrame* pCf, void* data) // Does the current and latched frame represent the same call? if (pCf->pFrame == pData->LatchedCF.pFrame) { - if (pData->LatchedCF.GetFunction()->AsDynamicMethodDesc()->AreFlagSets(DynamicMethodDesc::FlagUnbreakable)) - { - // Report only the latched IL stub frame which is a CER root. - frameAction = DiscardCurrentFrame; - } - else - { - // Report the interop method (current frame) which may be annotated, then the IL stub. - frameAction = ProcessLatchedReversed; - } + // Report the interop method (current frame) which may be annotated, then the IL stub. + frameAction = ProcessLatchedReversed; } else { @@ -805,20 +797,7 @@ StackWalkAction TAStackCrawlCallBack(CrawlFrame* pCf, void* data) // However, we still want to discard the interop method frame if the call is unbreakable by convention. if (pData->fHaveLatchedCF) { - MethodDesc *pMD = pCf->GetFunction(); - if (pMD != NULL && pMD->IsILStub() && - pData->LatchedCF.GetFrame()->GetReturnAddress() == GetControlPC(pCf->GetRegisterSet()) && - pMD->AsDynamicMethodDesc()->AreFlagSets(DynamicMethodDesc::FlagUnbreakable)) - { - // The current and latched frame represent the same call and the IL stub is marked as unbreakable. - // We will discard the interop method and report only the IL stub which is a CER root. - frameAction = DiscardLatchedFrame; - } - else - { - // Otherwise process the two frames in order. - frameAction = ProcessLatchedInOrder; - } + frameAction = ProcessLatchedInOrder; pData->fHaveLatchedCF = FALSE; } else From 466bbc08df59709cd4d38656039b5d30f4c5b464 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 1 Mar 2022 20:17:16 -0800 Subject: [PATCH 09/11] Fix GCC failure. --- src/coreclr/vm/stublink.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index b9752b40018b6a..20a6b1f253f748 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -2188,7 +2188,7 @@ void Stub::SetupStub(int numCodeBytes, DWORD flags #endif #endif - if (numCodeBytes >= MAX_CODEBYTES) + if (((DWORD)numCodeBytes) >= MAX_CODEBYTES) COMPlusThrowHR(COR_E_OVERFLOW); m_numCodeBytesAndFlags = numCodeBytes; From 6eb3cf33e510ff016f63477af249737219670ba9 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 2 Mar 2022 09:51:21 -0800 Subject: [PATCH 10/11] Follow the W^X logic for stub generation. --- src/coreclr/vm/arm/stubs.cpp | 1 + src/coreclr/vm/arm64/stubs.cpp | 1 + src/coreclr/vm/i386/stublinkerx86.cpp | 1 + src/coreclr/vm/prestub.cpp | 2 -- src/coreclr/vm/stublink.cpp | 38 ++++++++++++++++++++++----- src/coreclr/vm/stublink.h | 6 +++++ 6 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/arm/stubs.cpp b/src/coreclr/vm/arm/stubs.cpp index 57e478633668f6..61663aaa8697fe 100644 --- a/src/coreclr/vm/arm/stubs.cpp +++ b/src/coreclr/vm/arm/stubs.cpp @@ -1634,6 +1634,7 @@ VOID StubLinkerCPU::EmitComputedInstantiatingMethodStub(MethodDesc* pSharedMD, s } ThumbEmitTailCallManagedMethod(pSharedMD); + SetTargetMethod(pSharedMD); } diff --git a/src/coreclr/vm/arm64/stubs.cpp b/src/coreclr/vm/arm64/stubs.cpp index fbd222a9b2aba5..5e44995868483d 100644 --- a/src/coreclr/vm/arm64/stubs.cpp +++ b/src/coreclr/vm/arm64/stubs.cpp @@ -1732,6 +1732,7 @@ VOID StubLinkerCPU::EmitComputedInstantiatingMethodStub(MethodDesc* pSharedMD, s // Tail call the real target. EmitCallManagedMethod(pSharedMD, TRUE /* tail call */); + SetTargetMethod(pSharedMD); } void StubLinkerCPU::EmitCallLabel(CodeLabel *target, BOOL fTailCall, BOOL fIndirect) diff --git a/src/coreclr/vm/i386/stublinkerx86.cpp b/src/coreclr/vm/i386/stublinkerx86.cpp index 74d55fd4544db3..33d6a4acd531b8 100644 --- a/src/coreclr/vm/i386/stublinkerx86.cpp +++ b/src/coreclr/vm/i386/stublinkerx86.cpp @@ -3060,6 +3060,7 @@ VOID StubLinkerCPU::EmitComputedInstantiatingMethodStub(MethodDesc* pSharedMD, s } EmitTailJumpToMethod(pSharedMD); + SetTargetMethod(pSharedMD); } #endif // defined(FEATURE_SHARE_GENERIC_CODE) && defined(TARGET_AMD64) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 8731342137ec1f..9d75ccc075c2d6 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -1679,7 +1679,6 @@ Stub * MakeUnboxingStubWorker(MethodDesc *pMD) sl.EmitComputedInstantiatingMethodStub(pUnboxedMD, &portableShuffle[0], NULL); pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_INSTANTIATING_METHOD); - pstub->SetInstantiatedMethodDesc(pUnboxedMD); } else #endif @@ -1753,7 +1752,6 @@ Stub * MakeInstantiatingStubWorker(MethodDesc *pMD) sl.EmitComputedInstantiatingMethodStub(pSharedMD, &portableShuffle[0], extraArg); pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_INSTANTIATING_METHOD); - pstub->SetInstantiatedMethodDesc(pSharedMD); } else #endif diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index 20a6b1f253f748..29c046eb4a0129 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -351,6 +351,7 @@ StubLinker::StubLinker() m_pFirstCodeLabel = NULL; m_pFirstLabelRef = NULL; m_pPatchLabel = NULL; + m_pTargetMethod = NULL; m_stackSize = 0; m_fDataOnly = FALSE; #ifdef TARGET_ARM @@ -678,7 +679,20 @@ CodeLabel* StubLinker::NewExternalCodeLabel(LPVOID pExternalAddress) return pCodeLabel; } - +//--------------------------------------------------------------- +// Set the target method for Instantiating stubs. +//--------------------------------------------------------------- +void StubLinker::SetTargetMethod(PTR_MethodDesc pMD) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + PRECONDITION(pMD != NULL); + } + CONTRACTL_END; + m_pTargetMethod = pMD; +} //--------------------------------------------------------------- @@ -1079,11 +1093,23 @@ bool StubLinker::EmitStub(Stub* pStub, int globalsize, int totalSize, LoaderHeap ZeroMemory(pCodeRW + lastCodeOffset, globalsize - lastCodeOffset); } - // Fill in patch offset, if we have one - // Note that these offsets are relative to the start of the stub, - // not the code, so you'll have to add sizeof(Stub) to get to the - // right spot. - if (m_pPatchLabel != NULL) + // Set additional stub data. + // - Fill in the target method for the Instantiating stub. + // + // - Fill in patch offset, if we have one + // Note that these offsets are relative to the start of the stub, + // not the code, so you'll have to add sizeof(Stub) to get to the + // right spot. + if (pStubRW->IsInstantiatingStub()) + { + _ASSERTE(m_pTargetMethod != NULL); + _ASSERTE(m_pPatchLabel == NULL); + pStubRW->SetInstantiatedMethodDesc(m_pTargetMethod); + + LOG((LF_CORDB, LL_INFO100, "SL::ES: InstantiatedMethod fd:0x%x\n", + pStub->GetInstantiatedMethodDesc())); + } + else if (m_pPatchLabel != NULL) { UINT32 uLabelOffset = GetLabelOffset(m_pPatchLabel); _ASSERTE(FitsIn(uLabelOffset)); diff --git a/src/coreclr/vm/stublink.h b/src/coreclr/vm/stublink.h index 7f7875da4c2672..7e20812a57a47f 100644 --- a/src/coreclr/vm/stublink.h +++ b/src/coreclr/vm/stublink.h @@ -214,6 +214,11 @@ class StubLinker return NewExternalCodeLabel((LPVOID)pExternalAddress); } + //--------------------------------------------------------------- + // Set the target method for Instantiating stubs. + //--------------------------------------------------------------- + void SetTargetMethod(PTR_MethodDesc pMD); + //--------------------------------------------------------------- // Push and Pop can be used to keep track of stack growth. // These should be adjusted by opcodes written to the stream. @@ -288,6 +293,7 @@ class StubLinker CodeLabel *m_pPatchLabel; // label of stub patch offset // currently just for multicast // frames. + PTR_MethodDesc m_pTargetMethod; // Used for instantiating stubs. SHORT m_stackSize; // count of pushes/pops CQuickHeap m_quickHeap; // throwaway heap for // labels, and From 4a80151dce5e451f05a8bddafd76b4302a844e7d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 3 Mar 2022 11:04:48 -0800 Subject: [PATCH 11/11] Use canonical method names for MethodDesc types. Missed flag for CALLI not being a PInvoke. --- src/coreclr/vm/dllimport.cpp | 2 +- src/coreclr/vm/method.cpp | 2 +- src/coreclr/vm/method.hpp | 33 ++++++++++++++------------------- src/coreclr/vm/stubmgr.cpp | 4 ++-- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index f77303160a34b1..26fda0fabb002f 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5693,7 +5693,7 @@ PCODE GetStubForInteropMethod(MethodDesc* pMD, DWORD dwStubFlags) if (pStubMD != NULL && pStubMD->IsILStub() - && pStubMD->AsDynamicMethodDesc()->AreFlagSets(DynamicMethodDesc::FlagRequiresCOM)) + && pStubMD->AsDynamicMethodDesc()->HasFlags(DynamicMethodDesc::FlagRequiresCOM)) { // the stub uses COM so make sure that it is started EnsureComStarted(); diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 8969e312c0f161..1c64e33588be3f 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1318,7 +1318,7 @@ DWORD MethodDesc::GetAttrs() const if (IsArray()) return dac_cast(this)->GetAttrs(); else if (IsNoMetadata()) - return dac_cast(this)->GetFlagsAsMetadata(); + return dac_cast(this)->GetAttrs(); DWORD dwAttributes; if (FAILED(GetMDImport()->GetMethodDefProps(GetMemberDef(), &dwAttributes))) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 749528c7cc7848..41c81f3d3e4c14 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2530,7 +2530,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc { m_dwExtendedFlags = flags; } - bool AreFlagSets(DWORD flags) const + bool HasFlags(DWORD flags) const { return !!(m_dwExtendedFlags & flags); } @@ -2546,19 +2546,19 @@ class DynamicMethodDesc : public StoredSigMethodDesc ILStubType GetILStubType() const { ILStubType type = (ILStubType)(m_dwExtendedFlags & ILStubTypeMask); - _ASSERTE(type == StubNotSet || AreFlagSets(FlagIsILStub)); + _ASSERTE(type == StubNotSet || HasFlags(FlagIsILStub)); return type; } void SetILStubType(ILStubType type) { - _ASSERTE(AreFlagSets(FlagIsILStub)); + _ASSERTE(HasFlags(FlagIsILStub)); m_dwExtendedFlags |= type; } public: - bool IsILStub() const { LIMITED_METHOD_DAC_CONTRACT; return AreFlagSets(FlagIsILStub); } - bool IsLCGMethod() const { LIMITED_METHOD_DAC_CONTRACT; return AreFlagSets(FlagIsLCGMethod); } + bool IsILStub() const { LIMITED_METHOD_DAC_CONTRACT; return HasFlags(FlagIsILStub); } + bool IsLCGMethod() const { LIMITED_METHOD_DAC_CONTRACT; return HasFlags(FlagIsLCGMethod); } inline PTR_DynamicResolver GetResolver(); inline PTR_LCGMethodResolver GetLCGMethodResolver(); @@ -2571,12 +2571,12 @@ class DynamicMethodDesc : public StoredSigMethodDesc } // Based on the current flags, compute the equivalent as COR metadata. - WORD GetFlagsAsMetadata() const + WORD GetAttrs() const { LIMITED_METHOD_CONTRACT; WORD asMetadata = 0; - asMetadata |= AreFlagSets(FlagPublic) ? mdPublic : 0; - asMetadata |= AreFlagSets(FlagStatic) ? mdStatic : 0; + asMetadata |= HasFlags(FlagPublic) ? mdPublic : 0; + asMetadata |= HasFlags(FlagStatic) ? mdStatic : 0; return asMetadata; } @@ -2605,13 +2605,6 @@ class DynamicMethodDesc : public StoredSigMethodDesc return type == StubCOMToCLRInterop || type == StubNativeToCLRInterop; } - bool IsDelegateStub() const - { - LIMITED_METHOD_DAC_CONTRACT; - _ASSERTE(IsILStub()); - return AreFlagSets(FlagIsDelegate); - } - bool IsStepThroughStub() const { LIMITED_METHOD_CONTRACT; @@ -2631,19 +2624,21 @@ class DynamicMethodDesc : public StoredSigMethodDesc { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - return !AreFlagSets(FlagStatic) && GetILStubType() == StubCLRToCOMInterop; + return !HasFlags(FlagStatic) && GetILStubType() == StubCLRToCOMInterop; } bool IsCOMToCLRStub() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - return !AreFlagSets(FlagStatic) && GetILStubType() == StubCOMToCLRInterop; + return !HasFlags(FlagStatic) && GetILStubType() == StubCOMToCLRInterop; } bool IsPInvokeStub() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - return AreFlagSets(FlagStatic) && GetILStubType() == StubCLRToNativeInterop; + return HasFlags(FlagStatic) + && !HasFlags(FlagIsCALLI) + && GetILStubType() == StubCLRToNativeInterop; } #ifdef FEATURE_MULTICASTSTUB_AS_IL @@ -2673,7 +2668,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc bool HasMDContextArg() const { LIMITED_METHOD_CONTRACT; - return IsCLRToCOMStub() || (IsPInvokeStub() && !IsDelegateStub()); + return IsCLRToCOMStub() || (IsPInvokeStub() && !HasFlags(FlagIsDelegate)); } // diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 30f41bdc6215ea..7a1823f804ead6 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -1729,7 +1729,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, } trace->InitForManaged(target); } - else if (pStubMD->IsDelegateStub()) + else if (pStubMD->HasFlags(DynamicMethodDesc::FlagIsDelegate)) { // This is forward delegate P/Invoke stub, the argument is undefined DelegateObject *pDel = (DelegateObject *)pThis; @@ -1738,7 +1738,7 @@ BOOL ILStubManager::TraceManager(Thread *thread, LOG((LF_CORDB, LL_INFO10000, "ILSM::TraceManager: Forward delegate P/Invoke case 0x%p\n", target)); trace->InitForUnmanaged(target); } - else if (pStubMD->AreFlagSets(DynamicMethodDesc::FlagIsCALLI)) + else if (pStubMD->HasFlags(DynamicMethodDesc::FlagIsCALLI)) { // This is unmanaged CALLI stub, the argument is the target target = (PCODE)arg;