From 3d689d00843618105e735c5647e1cb64e721a333 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 8 Feb 2018 14:35:01 -0800 Subject: Debugger api to set a breakpoint on offset 0 of all methods (#15819) * Debugger api to set a breakpoint on offset 0 of all methods * code review feedback * more code review feedback * respect IsIl in CorDbFunctionBreakoint() * change SIMPLIFYING_ASSERT to SIMPLIFYING_ASSUMPTION_SUCCEEDED for hrs, it outputs a much better diagnostic message --- src/debug/di/breakpoint.cpp | 12 +++++++----- src/debug/di/module.cpp | 39 +++++++++++++++++++++++++++++++++++++-- src/debug/di/rsfunction.cpp | 36 ++++++++++++++++++++++++++++++++++++ src/debug/di/rspriv.h | 22 ++++++++++++++++++---- src/debug/di/shimprocess.cpp | 8 ++++---- src/debug/ee/controller.cpp | 31 ++++--------------------------- src/debug/ee/controller.h | 1 + src/debug/ee/debugger.cpp | 1 + 8 files changed, 108 insertions(+), 42 deletions(-) (limited to 'src/debug') diff --git a/src/debug/di/breakpoint.cpp b/src/debug/di/breakpoint.cpp index e3e4fb04a5..cc55060fa8 100644 --- a/src/debug/di/breakpoint.cpp +++ b/src/debug/di/breakpoint.cpp @@ -57,9 +57,11 @@ HRESULT CordbBreakpoint::BaseIsActive(BOOL *pbActive) * ------------------------------------------------------------------------- */ CordbFunctionBreakpoint::CordbFunctionBreakpoint(CordbCode *code, - SIZE_T offset) + SIZE_T offset, + BOOL offsetIsIl) : CordbBreakpoint(code->GetProcess(), CBT_FUNCTION), - m_code(code), m_offset(offset) + m_code(code), m_offset(offset), + m_offsetIsIl(offsetIsIl) { // Remember the app domain we came from so that breakpoints can be // deactivated from within the ExitAppdomain callback. @@ -203,11 +205,11 @@ HRESULT CordbFunctionBreakpoint::Activate(BOOL fActivate) pEvent->BreakpointData.vmDomainFile = m_code->GetModule()->GetRuntimeDomainFile(); pEvent->BreakpointData.encVersion = m_code->GetVersion(); - BOOL fIsIL = m_code->IsIL(); + BOOL codeIsIL = m_code->IsIL(); - pEvent->BreakpointData.isIL = fIsIL ? true : false; + pEvent->BreakpointData.isIL = m_offsetIsIl; pEvent->BreakpointData.offset = m_offset; - if (fIsIL) + if (codeIsIL) { pEvent->BreakpointData.nativeCodeMethodDescToken = pEvent->BreakpointData.nativeCodeMethodDescToken.NullPtr(); } diff --git a/src/debug/di/module.cpp b/src/debug/di/module.cpp index 5d1d3da427..b0ba054a87 100644 --- a/src/debug/di/module.cpp +++ b/src/debug/di/module.cpp @@ -2975,8 +2975,9 @@ HRESULT CordbCode::CreateBreakpoint(ULONG32 offset, HRESULT hr; ULONG32 size = GetSize(); + BOOL offsetIsIl = IsIL(); LOG((LF_CORDB, LL_INFO10000, "CCode::CreateBreakpoint, offset=%d, size=%d, IsIl=%d, this=0x%p\n", - offset, size, m_fIsIL, this)); + offset, size, offsetIsIl, this)); // Make sure the offset is within range of the method. // If we're native code, then both offset & total code size are bytes of native code, @@ -2986,7 +2987,7 @@ HRESULT CordbCode::CreateBreakpoint(ULONG32 offset, return CORDBG_E_UNABLE_TO_SET_BREAKPOINT; } - CordbFunctionBreakpoint *bp = new (nothrow) CordbFunctionBreakpoint(this, offset); + CordbFunctionBreakpoint *bp = new (nothrow) CordbFunctionBreakpoint(this, offset, offsetIsIl); if (bp == NULL) return E_OUTOFMEMORY; @@ -3391,6 +3392,40 @@ mdSignature CordbILCode::GetLocalVarSigToken() return m_localVarSigToken; } +HRESULT CordbILCode::CreateNativeBreakpoint(ICorDebugFunctionBreakpoint **ppBreakpoint) +{ + FAIL_IF_NEUTERED(this); + VALIDATE_POINTER_TO_OBJECT(ppBreakpoint, ICorDebugFunctionBreakpoint **); + + HRESULT hr; + ULONG32 size = GetSize(); + LOG((LF_CORDB, LL_INFO10000, "CordbILCode::CreateNativeBreakpoint, size=%d, this=0x%p\n", + size, this)); + + ULONG32 offset = 0; + CordbFunctionBreakpoint *bp = new (nothrow) CordbFunctionBreakpoint(this, offset, FALSE); + + if (bp == NULL) + { + return E_OUTOFMEMORY; + } + + hr = bp->Activate(TRUE); + if (SUCCEEDED(hr)) + { + *ppBreakpoint = static_cast (bp); + bp->ExternalAddRef(); + return S_OK; + } + else + { + delete bp; + return hr; + } +} + + + CordbReJitILCode::CordbReJitILCode(CordbFunction *pFunction, SIZE_T encVersion, VMPTR_ILCodeVersionNode vmILCodeVersionNode) : CordbILCode(pFunction, TargetBuffer(), encVersion, mdSignatureNil, VmPtrToCookie(vmILCodeVersionNode)), m_cClauses(0), diff --git a/src/debug/di/rsfunction.cpp b/src/debug/di/rsfunction.cpp index bf3c49bb98..82ac87530d 100644 --- a/src/debug/di/rsfunction.cpp +++ b/src/debug/di/rsfunction.cpp @@ -132,6 +132,10 @@ HRESULT CordbFunction::QueryInterface(REFIID id, void **pInterface) { *pInterface = static_cast(this); } + else if (id == IID_ICorDebugFunction4) + { + *pInterface = static_cast(this); + } else if (id == IID_IUnknown) { *pInterface = static_cast(static_cast(this)); @@ -570,6 +574,38 @@ HRESULT CordbFunction::GetActiveReJitRequestILCode(ICorDebugILCode **ppReJitedIL return hr; } +//----------------------------------------------------------------------------- +// CordbFunction::CreateNativeBreakpoint +// Public method for ICorDebugFunction4::CreateNativeBreakpoint. +// Sets a breakpoint at native offset 0 for all native code versions of a method. +// +// Parameters +// pnVersion - out parameter to hold the version number. +// +// Returns: +// S_OK on success. +//----------------------------------------------------------------------------- +HRESULT CordbFunction::CreateNativeBreakpoint(ICorDebugFunctionBreakpoint **ppBreakpoint) +{ + PUBLIC_API_ENTRY(this); + FAIL_IF_NEUTERED(this); + VALIDATE_POINTER_TO_OBJECT(ppBreakpoint, ICorDebugFunctionBreakpoint **); + ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess()); + + HRESULT hr = S_OK; + + RSExtSmartPtr pCode; + + hr = GetILCode(&pCode); + + if (SUCCEEDED(hr)) + { + hr = pCode->CreateNativeBreakpoint(ppBreakpoint); + } + + return hr; +} + // determine whether we have a native-only implementation // Arguments: // Input: none (we use information in various data members of this instance of CordbFunction: m_isNativeImpl, diff --git a/src/debug/di/rspriv.h b/src/debug/di/rspriv.h index de821f26a6..41a04f284e 100644 --- a/src/debug/di/rspriv.h +++ b/src/debug/di/rspriv.h @@ -5338,7 +5338,11 @@ const BOOL bILCode = TRUE; // B/C of generics, a single IL function may get jitted multiple times and // be associated w/ multiple native code blobs (CordbNativeCode). // -class CordbFunction : public CordbBase, public ICorDebugFunction, public ICorDebugFunction2, public ICorDebugFunction3 +class CordbFunction : public CordbBase, + public ICorDebugFunction, + public ICorDebugFunction2, + public ICorDebugFunction3, + public ICorDebugFunction4 { public: //----------------------------------------------------------- @@ -5396,6 +5400,11 @@ public: //----------------------------------------------------------- COM_METHOD GetActiveReJitRequestILCode(ICorDebugILCode **ppReJitedILCode); + //----------------------------------------------------------- + // ICorDebugFunction4 + //----------------------------------------------------------- + COM_METHOD CreateNativeBreakpoint(ICorDebugFunctionBreakpoint **ppBreakpoint); + //----------------------------------------------------------- // Internal members //----------------------------------------------------------- @@ -5740,6 +5749,8 @@ public: HRESULT GetLocalVariableType(DWORD dwIndex, const Instantiation * pInst, CordbType ** ppResultType); mdSignature GetLocalVarSigToken(); + COM_METHOD CreateNativeBreakpoint(ICorDebugFunctionBreakpoint **ppBreakpoint); + private: // Read the actual bytes of IL code into the data member m_rgbCode. // Helper routine for GetCode @@ -5771,7 +5782,9 @@ protected: * rejitID. Thus it is 1:N with a given instantiation of CordbFunction. * ------------------------------------------------------------------------- */ -class CordbReJitILCode : public CordbILCode, public ICorDebugILCode, public ICorDebugILCode2 +class CordbReJitILCode : public CordbILCode, + public ICorDebugILCode, + public ICorDebugILCode2 { public: // Initialize a new CordbILCode instance @@ -5796,7 +5809,7 @@ public: //----------------------------------------------------------- COM_METHOD GetLocalVarSigToken(mdSignature *pmdSig); COM_METHOD GetInstrumentedILMap(ULONG32 cMap, ULONG32 *pcMap, COR_IL_MAP map[]); - + private: HRESULT Init(DacSharedReJitInfo* pSharedReJitInfo); @@ -7560,7 +7573,7 @@ class CordbFunctionBreakpoint : public CordbBreakpoint, public ICorDebugFunctionBreakpoint { public: - CordbFunctionBreakpoint(CordbCode *code, SIZE_T offset); + CordbFunctionBreakpoint(CordbCode *code, SIZE_T offset, BOOL offsetIsIl); ~CordbFunctionBreakpoint(); virtual void Neuter(); @@ -7621,6 +7634,7 @@ public: // leaked. RSExtSmartPtr m_code; SIZE_T m_offset; + BOOL m_offsetIsIl; }; /* ------------------------------------------------------------------------- * diff --git a/src/debug/di/shimprocess.cpp b/src/debug/di/shimprocess.cpp index 46c35fdc2b..99967a2af9 100644 --- a/src/debug/di/shimprocess.cpp +++ b/src/debug/di/shimprocess.cpp @@ -365,12 +365,12 @@ DWORD WINAPI CallStopGoThreadProc(LPVOID parameter) // Calling Stop + Continue will synchronize the process and force any queued events to be called. // Stop is synchronous and will block until debuggee is synchronized. hr = pProc->Stop(INFINITE); - SIMPLIFYING_ASSUMPTION(SUCCEEDED(hr)); + SIMPLIFYING_ASSUMPTION_SUCCEEDED(hr); // Continue will resume the debuggee. If there are queued events (which we expect in this case) // then continue will drain the event queue instead of actually resuming the process. hr = pProc->Continue(FALSE); - SIMPLIFYING_ASSUMPTION(SUCCEEDED(hr)); + SIMPLIFYING_ASSUMPTION_SUCCEEDED(hr); // This thread just needs to trigger an event dispatch. Now that it's done that, it can exit. return 0; @@ -833,7 +833,7 @@ HRESULT ShimProcess::HandleWin32DebugEvent(const DEBUG_EVENT * pEvent) EX_CATCH_HRESULT(hrIgnore); // Dont' expect errors here (but could probably return it up to become an // unrecoverable error if necessary). We still want to call Continue thought. - SIMPLIFYING_ASSUMPTION(SUCCEEDED(hrIgnore)); + SIMPLIFYING_ASSUMPTION_SUCCEEDED(hrIgnore); // // Continue the debuggee if needed. @@ -873,7 +873,7 @@ HRESULT ShimProcess::HandleWin32DebugEvent(const DEBUG_EVENT * pEvent) { ::Sleep(500); hrIgnore = GetNativePipeline()->EnsureThreadsRunning(); - SIMPLIFYING_ASSUMPTION(SUCCEEDED(hrIgnore)); + SIMPLIFYING_ASSUMPTION_SUCCEEDED(hrIgnore); } } } diff --git a/src/debug/ee/controller.cpp b/src/debug/ee/controller.cpp index 55e9936866..b6c20fd4d6 100644 --- a/src/debug/ee/controller.cpp +++ b/src/debug/ee/controller.cpp @@ -4946,40 +4946,17 @@ DebuggerBreakpoint::DebuggerBreakpoint(Module *module, SIZE_T ilEnCVersion, // must give the EnC version for non-native bps MethodDesc *nativeMethodDesc, // use only when m_native DebuggerJitInfo *nativeJITInfo, // optional when m_native, null otherwise + bool nativeCodeBindAllVersions, BOOL *pSucceed ) : DebuggerController(NULL, pAppDomain) { _ASSERTE(pSucceed != NULL); - _ASSERTE(native == (nativeMethodDesc != NULL)); + _ASSERTE((native == (nativeMethodDesc != NULL)) || nativeCodeBindAllVersions); _ASSERTE(native || nativeJITInfo == NULL); _ASSERTE(!nativeJITInfo || nativeJITInfo->m_jitComplete); // this is sent by the left-side, and it couldn't have got the code if the JIT wasn't complete - BOOL bindAcrossAllJittedInstances = !native; - MethodDesc* pGenericInstanceFilter = NULL; -#ifdef DEBUG - // Normally any breakpoint specified as a native offset only binds in one jitted instance of a method, however - // to better test the breakpoint binding logic in debug builds we allow the behavior to change. The test behavior - // binds native breakpoints in every code version of the same generic instance. Currently the only way to get more - // than one such version is to use tiered compilation, but even with only one version the code path is a little different. - // - // This covers the same code paths used to add a step-in breakpoint, because step-in needs to handle code version changes - // transparently but it is challenging to create a test case that ensures the code version will change exactly during the - // tiny window of time that the step-in breakpoint exists. - static ConfigDWORD config; - if(config.val(CLRConfig::INTERNAL_DbgNativeCodeBpBindsAcrossVersions)) - { - LOG((LF_CORDB, LL_INFO1000, "DB::DB Test hook COMPLUS_DbgNativeCodeBpBindsAcrossVersions is active\n")); - if (native && offset == 0 && nativeMethodDesc) - { - LOG((LF_CORDB, LL_INFO1000, "DB::DB Test hook modification: native breakpoint at offset 0 binding to all code versions\n")); - bindAcrossAllJittedInstances = TRUE; - pGenericInstanceFilter = nativeMethodDesc; - } - } -#endif - - if (!bindAcrossAllJittedInstances) + if (native && !nativeCodeBindAllVersions) { (*pSucceed) = AddBindAndActivateNativeManagedPatch(nativeMethodDesc, nativeJITInfo, offset, LEAF_MOST_FRAME, pAppDomain); return; @@ -4987,7 +4964,7 @@ DebuggerBreakpoint::DebuggerBreakpoint(Module *module, else { _ASSERTE(!native || offset == 0); - (*pSucceed) = AddILPatch(pAppDomain, module, md, pGenericInstanceFilter, ilEnCVersion, offset, !native); + (*pSucceed) = AddILPatch(pAppDomain, module, md, NULL, ilEnCVersion, offset, !native); } } diff --git a/src/debug/ee/controller.h b/src/debug/ee/controller.h index bac635e2f7..16dd1772fd 100644 --- a/src/debug/ee/controller.h +++ b/src/debug/ee/controller.h @@ -1450,6 +1450,7 @@ public: SIZE_T ilEnCVersion, // must give the EnC version for non-native bps MethodDesc *nativeMethodDesc, // must be non-null when m_native, null otherwise DebuggerJitInfo *nativeJITInfo, // optional when m_native, null otherwise + bool nativeCodeBindAllVersions, BOOL *pSucceed ); diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp index 23e5e54df4..94792dac2c 100644 --- a/src/debug/ee/debugger.cpp +++ b/src/debug/ee/debugger.cpp @@ -10777,6 +10777,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) pEvent->BreakpointData.encVersion, pMethodDesc, pDJI, + pEvent->BreakpointData.nativeCodeMethodDescToken == NULL, &fSuccess); TRACE_ALLOC(pDebuggerBP); -- cgit v1.2.3