diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index fffd6df484825f..f12b6166019a04 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -292,6 +292,48 @@ static inline DWORD CordbGetWaitTimeout() } } +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT +void UnmanagedThreadTracker::Suspend() +{ + _ASSERTE(m_hThread != INVALID_HANDLE_VALUE); + if (m_hThread != INVALID_HANDLE_VALUE) + { + m_dwSuspendCount++; + ::SuspendThread(m_hThread); + } +} + +void UnmanagedThreadTracker::Resume() +{ + _ASSERTE(m_dwSuspendCount > 0); + if (m_dwSuspendCount == 0) + return; + m_dwSuspendCount--; + _ASSERTE(m_hThread != INVALID_HANDLE_VALUE); + if (m_hThread != INVALID_HANDLE_VALUE) + { + ::ResumeThread(m_hThread); + } +} + +void UnmanagedThreadTracker::Close() +{ + HANDLE hThread = m_hThread; + DWORD dwSuspendCount = m_dwSuspendCount; + m_hThread = INVALID_HANDLE_VALUE; + m_dwSuspendCount = 0; + if (hThread == INVALID_HANDLE_VALUE) + { + return; + } + for (DWORD i = 0; i < dwSuspendCount; i++) + { + ::ResumeThread(hThread); + } + ::CloseHandle(hThread); +} +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT + //---------------------------------------------------------------------------- // Implementation of IDacDbiInterface::IMetaDataLookup. // lookup Internal Metadata Importer keyed by PEAssembly @@ -981,6 +1023,10 @@ CordbProcess::CordbProcess(ULONG64 clrInstanceId, m_fAssertOnTargetInconsistency(false), m_runtimeOffsetsInitialized(false), m_writableMetadataUpdateMode(LegacyCompatPolicy) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + m_dwOutOfProcessStepping(0) +#endif { _ASSERTE((m_id == 0) == (pShim == NULL)); @@ -1349,6 +1395,22 @@ void CordbProcess::Neuter() // Take the process lock. RSLockHolder lockHolder(GetProcessLock()); +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + CUnmanagedThreadHashTableIterator beginIter = m_unmanagedThreadHashTable.Begin(); + CUnmanagedThreadHashTableIterator endIter = m_unmanagedThreadHashTable.End(); + for (CUnmanagedThreadHashTableIterator it = beginIter; it != endIter; ++it) + { + UnmanagedThreadTracker * pUnmanagedThread = *it; + _ASSERTE(pUnmanagedThread != NULL); + if (pUnmanagedThread != NULL) + { + pUnmanagedThread->Close(); + delete pUnmanagedThread; + } + } + m_unmanagedThreadHashTable.RemoveAll(); + m_dwOutOfProcessStepping = 0; +#endif NeuterChildren(); @@ -11168,35 +11230,21 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) // However, there are situations where OpenThread can fail with an Access Denied error. // From https://github.com/dotnet/runtime/issues/107263, the control-c handler in // Windows causes the process to have higher privileges. - // We are now using the following approach to access the thread handle, which is the same - // approach used by CordbThread::RefreshHandle: - // 1. Get the thread handle from the DAC - // 2. Duplicate the handle to the current process - - // lookup the CordbThread by thread ID, so that we can access the left-side thread handle - CordbThread * pThread = TryLookupOrCreateThreadByVolatileOSId(dwThreadId); + // We are now caching the thread handle in the unmanaged thread hash table when the thread is created. - IDacDbiInterface* pDAC = GetDAC(); - if (pDAC == NULL) - { - LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - DAC not initialized\n")); - ThrowHR(E_UNEXPECTED); - } + UnmanagedThreadTracker * curThread = m_unmanagedThreadHashTable.Lookup(dwThreadId); - HANDLE hOutOfProcThread = pDAC->GetThreadHandle(pThread->m_vmThreadToken); - if (hOutOfProcThread == NULL) + if (curThread == NULL || curThread->GetThreadId() != dwThreadId) { - LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Failed to get thread handle\n")); - ThrowHR(E_UNEXPECTED); + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Thread not found\n")); + ThrowHR(CORDBG_E_BAD_THREAD_STATE); } - // Duplicate the thread handle to the current process - HandleHolder hThread; - BOOL fSuccess = DuplicateHandle(UnsafeGetProcessHandle(), hOutOfProcThread, ::GetCurrentProcess(), &hThread, 0, FALSE, DUPLICATE_SAME_ACCESS); - if (!fSuccess) + HANDLE hThread = curThread->GetThreadHandle(); + if (hThread == INVALID_HANDLE_VALUE) { - LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from DuplicateHandle\n")); - ThrowHR(HRESULT_FROM_GetLastError()); + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Thread handle not found\n")); + ThrowHR(CORDBG_E_BAD_THREAD_STATE); } // Suspend the thread and so that we can read the thread context. @@ -11224,10 +11272,8 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) TADDR lsContextAddr = (TADDR)context.Rcx; DWORD contextSize = (DWORD)context.Rdx; - // Read the expected Rip and Rsp from the thread context. This is used to - // validate the context read from the left-side. - TADDR expectedRip = (TADDR)context.R8; - TADDR expectedRsp = (TADDR)context.R9; + bool fIsInPlaceSingleStep = (bool)(context.R8&0x1); + PRD_TYPE opcode = (PRD_TYPE)context.R9; if (contextSize == 0 || contextSize > sizeof(CONTEXT) + 25000) { @@ -11259,15 +11305,6 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) ThrowHR(ERROR_PARTIAL_COPY); } - if (pContext->Rip != expectedRip || pContext->Rsp != expectedRsp) - { - _ASSERTE(!"ReadVirtual unexpectedly returned mismatched Rip and Rsp registers"); - - LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - ReadVirtual unexpectedly returned mismatched Rip and Rsp registers\n")); - - ThrowHR(E_UNEXPECTED); - } - // TODO: Ideally we would use ICorDebugMutableDataTarget::SetThreadContext however this API currently only handles the legacy context. // We should combine the following code with the shared implementation @@ -11346,10 +11383,133 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from SetThreadContext\n")); ThrowHR(HRESULT_FROM_WIN32(lastError)); } + + if (fIsInPlaceSingleStep) + { + CORDB_ADDRESS_TYPE *patchSkipAddr = (CORDB_ADDRESS_TYPE*)pFrameContext->Rip; + + HANDLE hProcess = UnsafeGetProcessHandle(); + LPVOID baseAddress = (LPVOID)(patchSkipAddr); + DWORD oldProt; + + if (!VirtualProtectEx(hProcess, + baseAddress, + CORDbg_BREAK_INSTRUCTION_SIZE, + PAGE_EXECUTE_READWRITE, &oldProt)) + { + // we may be seeing unwriteable directly mapped executable memory. + // let's try copy-on-write instead, + if (!VirtualProtectEx(hProcess, + baseAddress, + CORDbg_BREAK_INSTRUCTION_SIZE, + PAGE_EXECUTE_WRITECOPY, &oldProt)) + { + _ASSERTE(!"VirtualProtect of code page failed"); + ThrowHR(HRESULT_FROM_GetLastError()); + } + } + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - address=0x%p opcode=0x%x\n", patchSkipAddr, opcode)); + HRESULT hr = RemoveRemotePatch(this, (void*)patchSkipAddr, opcode); + IfFailThrow(hr); + + if (!VirtualProtectEx(hProcess, + baseAddress, + CORDbg_BREAK_INSTRUCTION_SIZE, + oldProt, &oldProt)) + { + _ASSERTE(!"VirtualProtect of code page failed"); + ThrowHR(HRESULT_FROM_GetLastError()); + } + + curThread->SetPatchSkipAddress(patchSkipAddr); + + // suspend all other threads + m_dwOutOfProcessStepping++; + CUnmanagedThreadHashTableIterator beginIter = m_unmanagedThreadHashTable.Begin(); + CUnmanagedThreadHashTableIterator endIter = m_unmanagedThreadHashTable.End(); + for (CUnmanagedThreadHashTableIterator curIter = beginIter; curIter != endIter; ++curIter) + { + UnmanagedThreadTracker * pUnmanagedThread = *curIter; + _ASSERTE(pUnmanagedThread != NULL); + if (pUnmanagedThread == NULL || pUnmanagedThread->GetThreadId() == dwThreadId) + { + continue; + } + pUnmanagedThread->Suspend(); + } + } #else #error Platform not supported #endif } + +bool CordbProcess::HandleInPlaceSingleStep(DWORD dwThreadId, PVOID pExceptionAddress) +{ + UnmanagedThreadTracker * curThread = m_unmanagedThreadHashTable.Lookup(dwThreadId); + _ASSERTE(curThread != NULL); + if (curThread != NULL && + curThread->GetThreadId() == dwThreadId && + curThread->IsInPlaceStepping()) + { + // this is an in-place step, so place the breakpoint instruction back to the patch location + + HANDLE hProcess = UnsafeGetProcessHandle(); + LPVOID baseAddress = (LPVOID)(curThread->GetPatchSkipAddress()); + DWORD oldProt; + + if (!VirtualProtectEx(hProcess, + baseAddress, + CORDbg_BREAK_INSTRUCTION_SIZE, + PAGE_EXECUTE_READWRITE, &oldProt)) + { + // we may be seeing unwriteable directly mapped executable memory. + // let's try copy-on-write instead, + if (!VirtualProtectEx(hProcess, + baseAddress, + CORDbg_BREAK_INSTRUCTION_SIZE, + PAGE_EXECUTE_WRITECOPY, &oldProt)) + { + _ASSERTE(!"VirtualProtect of code page failed"); + ThrowHR(HRESULT_FROM_GetLastError()); + } + } + + HRESULT hr = ApplyRemotePatch(this, curThread->GetPatchSkipAddress()); + IfFailThrow(hr); + + if (!VirtualProtectEx(hProcess, + baseAddress, + CORDbg_BREAK_INSTRUCTION_SIZE, + oldProt, &oldProt)) + { + _ASSERTE(!"VirtualProtect of code page failed"); + ThrowHR(HRESULT_FROM_GetLastError()); + } + + curThread->ClearPatchSkipAddress(); + + m_dwOutOfProcessStepping--; + + // resume all other threads + CUnmanagedThreadHashTableIterator beginIter = m_unmanagedThreadHashTable.Begin(); + CUnmanagedThreadHashTableIterator endIter = m_unmanagedThreadHashTable.End(); + for (CUnmanagedThreadHashTableIterator curIter = beginIter; curIter != endIter; ++curIter) + { + UnmanagedThreadTracker * pUnmanagedThread = *curIter; + _ASSERTE(pUnmanagedThread != NULL); + if (pUnmanagedThread == NULL || pUnmanagedThread->GetThreadId() == dwThreadId) + { + continue; + } + pUnmanagedThread->Resume(); + } + + return true; + } + + return false; +} #endif // OUT_OF_PROCESS_SETTHREADCONTEXT // @@ -11606,6 +11766,16 @@ HRESULT CordbProcess::Filter( HandleSetThreadContextNeeded(dwThreadId); *pContinueStatus = DBG_CONTINUE; } + else if (dwFirstChance && pRecord->ExceptionCode == STATUS_SINGLE_STEP && m_dwOutOfProcessStepping > 0) + { + // this may be an in-place step, and if so place the breakpoint instruction back to the patch location and resume the threads + + if (HandleInPlaceSingleStep(dwThreadId, pRecord->ExceptionAddress)) + { + // let the normal left side debugger stepper logic execute for this single step + *pContinueStatus = DBG_EXCEPTION_NOT_HANDLED; + } + } #endif } PUBLIC_API_END(hr); @@ -15118,6 +15288,13 @@ HRESULT CordbProcess::IsReadyForDetach() return CORDBG_E_DETACH_FAILED_OUTSTANDING_STEPPERS; } +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + if (m_dwOutOfProcessStepping > 0) + { + return CORDBG_E_DETACH_FAILED_OUTSTANDING_STEPPERS; + } +#endif + // // If there are any outstanding breakpoints then fail the detach. // @@ -15434,3 +15611,50 @@ void CordbProcess::HandleControlCTrapResult(HRESULT result) // Send the reply to the LS. SendIPCEvent(&eventControlCResult, sizeof(eventControlCResult)); } + +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT +void CordbProcess::HandleDebugEventForInPlaceStepping(const DEBUG_EVENT * pEvent) +{ + PUBLIC_API_ENTRY_FOR_SHIM(this); + + const DWORD dwDesiredAccess = THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_SUSPEND_RESUME; + + switch (pEvent->dwDebugEventCode) + { + case CREATE_PROCESS_DEBUG_EVENT: + if (GetProcessId(pEvent->u.CreateProcessInfo.hProcess) == GetProcessId(UnsafeGetProcessHandle())) + { + HANDLE hThread = INVALID_HANDLE_VALUE; + ::DuplicateHandle(::GetCurrentProcess(), pEvent->u.CreateProcessInfo.hThread, ::GetCurrentProcess(), &hThread, dwDesiredAccess, false, 0); + m_unmanagedThreadHashTable.AddNoThrow(new UnmanagedThreadTracker(pEvent->dwThreadId, hThread)); + } + break; + case CREATE_THREAD_DEBUG_EVENT: + { + HANDLE hThread = INVALID_HANDLE_VALUE; + ::DuplicateHandle(::GetCurrentProcess(), pEvent->u.CreateThread.hThread, ::GetCurrentProcess(), &hThread, dwDesiredAccess, false, 0); + UnmanagedThreadTracker * newItem = new UnmanagedThreadTracker(pEvent->dwThreadId, hThread); + m_unmanagedThreadHashTable.AddNoThrow(newItem); + if (newItem && m_dwOutOfProcessStepping > 0) // if the insert was successful and we have out-of-process stepping enabled + { + for (DWORD i = 0; i < m_dwOutOfProcessStepping; i++) + { + newItem->Suspend(); + } + } + } + break; + case EXIT_THREAD_DEBUG_EVENT: + { + UnmanagedThreadTracker * pUnmanagedThread = m_unmanagedThreadHashTable.Lookup(pEvent->dwThreadId); + if (pUnmanagedThread != NULL) + { + pUnmanagedThread->Close(); + m_unmanagedThreadHashTable.Remove(pEvent->dwThreadId); + delete pUnmanagedThread; + } + } + break; + } +} +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT \ No newline at end of file diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 4f5acbbdc1c0b6..7e2b49b3170107 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -50,6 +50,8 @@ struct MachineInfo; #include "eventchannel.h" +#include + #undef ASSERT #define CRASH(x) _ASSERTE(!(x)) #define ASSERT(x) _ASSERTE(x) @@ -2883,6 +2885,10 @@ class IProcessShimHooks #ifdef FEATURE_INTEROP_DEBUGGING virtual bool IsUnmanagedThreadHijacked(ICorDebugThread * pICorDebugThread) = 0; #endif + +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + virtual void HandleDebugEventForInPlaceStepping(const DEBUG_EVENT * pEvent) = 0; +#endif }; @@ -2921,6 +2927,42 @@ struct DbgAssertAppDomainDeletedData VMPTR_AppDomain m_vmAppDomainDeleted; }; +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT +class UnmanagedThreadTracker +{ + DWORD m_dwThreadId = (DWORD)-1; + HANDLE m_hThread = INVALID_HANDLE_VALUE; + CORDB_ADDRESS_TYPE *m_pPatchSkipAddress = NULL; + DWORD m_dwSuspendCount = 0; + +public: + UnmanagedThreadTracker(DWORD wThreadId, HANDLE hThread) : m_dwThreadId(wThreadId), m_hThread(hThread) {} + + DWORD GetThreadId() const { return m_dwThreadId; } + HANDLE GetThreadHandle() const { return m_hThread; } + bool IsInPlaceStepping() const { return m_pPatchSkipAddress != NULL; } + void SetPatchSkipAddress(CORDB_ADDRESS_TYPE *pPatchSkipAddress) { m_pPatchSkipAddress = pPatchSkipAddress; } + CORDB_ADDRESS_TYPE *GetPatchSkipAddress() const { return m_pPatchSkipAddress; } + void ClearPatchSkipAddress() { m_pPatchSkipAddress = NULL; } + void Suspend(); + void Resume(); + void Close(); +}; + +class EMPTY_BASES_DECL CUnmanagedThreadSHashTraits : public DefaultSHashTraits +{ + public: + typedef DWORD key_t; + + static key_t GetKey(const element_t &e) { return e->GetThreadId(); } + static BOOL Equals(const key_t &e, const key_t &f) { return e == f; } + static count_t Hash(const key_t &e) { return (count_t)(e ^ (e >> 16) * 0x45D9F43); } +}; + +typedef SHash CUnmanagedThreadHashTableImpl; +typedef SHash::Iterator CUnmanagedThreadHashTableIterator; +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT + class CordbProcess : public CordbBase, public ICorDebugProcess, @@ -3286,6 +3328,7 @@ class CordbProcess : #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT void HandleSetThreadContextNeeded(DWORD dwThreadId); + bool HandleInPlaceSingleStep(DWORD dwThreadId, PVOID pExceptionAddress); #endif // @@ -4125,6 +4168,14 @@ class CordbProcess : WriteableMetadataUpdateMode m_writableMetadataUpdateMode; COM_METHOD GetObjectInternal(CORDB_ADDRESS addr, CordbAppDomain* pAppDomainOverride, ICorDebugObjectValue **pObject); + +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + CUnmanagedThreadHashTableImpl m_unmanagedThreadHashTable; + DWORD m_dwOutOfProcessStepping; +public: + void HandleDebugEventForInPlaceStepping(const DEBUG_EVENT * pEvent); +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT + }; // Some IMDArocess APIs are supported as interop-only. diff --git a/src/coreclr/debug/di/shimprocess.cpp b/src/coreclr/debug/di/shimprocess.cpp index c44be066370a5e..4b09d7993a77cc 100644 --- a/src/coreclr/debug/di/shimprocess.cpp +++ b/src/coreclr/debug/di/shimprocess.cpp @@ -789,6 +789,14 @@ HRESULT ShimProcess::HandleWin32DebugEvent(const DEBUG_EVENT * pEvent) } } } +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + else if (pEvent->dwDebugEventCode == CREATE_THREAD_DEBUG_EVENT || + pEvent->dwDebugEventCode == EXIT_THREAD_DEBUG_EVENT || + pEvent->dwDebugEventCode == CREATE_PROCESS_DEBUG_EVENT) + { + m_pProcess->HandleDebugEventForInPlaceStepping(pEvent); + } +#endif // Do standard event handling, including Handling loader-breakpoint, // and callback into CordbProcess for Attach if needed. diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 4ce03c05704f75..2d01f1904436e9 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -2481,7 +2481,12 @@ bool DebuggerController::MatchPatch(Thread *thread, DebuggerPatchSkip *DebuggerController::ActivatePatchSkip(Thread *thread, const BYTE *PC, - BOOL fForEnC) + BOOL fForEnC +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + DebuggerSteppingInfo *pDebuggerSteppingInfo +#endif + ) { #ifdef _DEBUG BOOL shouldBreak = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_ActivatePatchSkip); @@ -2527,6 +2532,14 @@ DebuggerPatchSkip *DebuggerController::ActivatePatchSkip(Thread *thread, LOG((LF_CORDB,LL_INFO10000, "DC::APS: About to skip from PC=0x%p\n", PC)); skip = new (interopsafe) DebuggerPatchSkip(thread, patch); TRACE_ALLOC(skip); + +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + if (pDebuggerSteppingInfo != NULL && skip != NULL && skip->IsInPlaceSingleStep()) + { + // send the opcode to the right side so it can be restored during single-step + pDebuggerSteppingInfo->EnableInPlaceSingleStepOverCall(patch->opcode); + } +#endif } return skip; @@ -2838,7 +2851,11 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, // DebuggerController's TriggerPatch). Put any DCs that are interested into a queue and then calls // SendEvent on each. // Note that control will not return from this function in the case of EnC remap -DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTEXT *context, CORDB_ADDRESS_TYPE *address, SCAN_TRIGGER which) +DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTEXT *context, CORDB_ADDRESS_TYPE *address, SCAN_TRIGGER which +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT +,DebuggerSteppingInfo *pDebuggerSteppingInfo +#endif +) { CONTRACT(DPOSS_ACTION) { @@ -3032,7 +3049,11 @@ DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTE } #endif - ActivatePatchSkip(thread, dac_cast(GetIP(pCtx)), FALSE); + ActivatePatchSkip(thread, dac_cast(GetIP(pCtx)), FALSE +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , pDebuggerSteppingInfo +#endif + ); lockController.Release(); @@ -4132,7 +4153,12 @@ void ThisFunctionMayHaveTriggerAGC() bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException, CONTEXT *pContext, DWORD dwCode, - Thread *pCurThread) + Thread *pCurThread +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + DebuggerSteppingInfo *pDebuggerSteppingInfo +#endif + ) { CONTRACTL { @@ -4303,7 +4329,12 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException, result = DebuggerController::DispatchPatchOrSingleStep(pCurThread, pContext, ip, - ST_PATCH); + ST_PATCH +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + pDebuggerSteppingInfo +#endif + ); LOG((LF_CORDB, LL_EVERYTHING, "DC::DNE DispatchPatch call returned\n")); // If we detached, we should remove all our breakpoints. So if we try @@ -4320,7 +4351,12 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException, result = DebuggerController::DispatchPatchOrSingleStep(pCurThread, pContext, ip, - (SCAN_TRIGGER)(ST_PATCH|ST_SINGLE_STEP)); + (SCAN_TRIGGER)(ST_PATCH|ST_SINGLE_STEP) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + pDebuggerSteppingInfo +#endif + ); // We pass patch | single step since single steps actually // do both (eg, you SS onto a breakpoint). break; @@ -4434,12 +4470,19 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, NativeWalker::DecodeInstructionForPatchSkip(patchBypassRX, &(m_instrAttrib)); +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + if (g_pDebugInterface->IsOutOfProcessSetContextEnabled() && m_instrAttrib.m_fIsCall) + { + m_fInPlaceSS = true; + } +#endif + #if defined(TARGET_AMD64) // The code below handles RIP-relative addressing on AMD64. The original implementation made the assumption that // we are only using RIP-relative addressing to access read-only data (see VSW 246145 for more information). This // has since been expanded to handle RIP-relative writes as well. - if (m_instrAttrib.m_dwOffsetToDisp != 0) + if (m_instrAttrib.m_dwOffsetToDisp != 0 && !IsInPlaceSingleStep()) { _ASSERTE(m_instrAttrib.m_cbInstr != 0); @@ -4545,13 +4588,16 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, patchBypassRX = NativeWalker::SetupOrSimulateInstructionForPatchSkip(context, m_pSharedPatchBypassBuffer, (const BYTE *)patch->address, patch->opcode); #endif //TARGET_ARM64 - //set eip to point to buffer... - SetIP(context, (PCODE)patchBypassRX); + if (!IsInPlaceSingleStep()) + { + //set eip to point to buffer... + SetIP(context, (PCODE)patchBypassRX); - if (context ==(T_CONTEXT*) &c) - thread->SetThreadContext(&c); + if (context ==(T_CONTEXT*) &c) + thread->SetThreadContext(&c); - LOG((LF_CORDB, LL_INFO10000, "DPS::DPS Bypass at %p for opcode 0x%zx \n", patchBypassRX, patch->opcode)); + LOG((LF_CORDB, LL_INFO10000, "DPS::DPS Bypass at %p for opcode 0x%zx \n", patchBypassRX, patch->opcode)); + } // // Turn on single step (if the platform supports it) so we can @@ -4770,14 +4816,18 @@ TP_RESULT DebuggerPatchSkip::TriggerExceptionHook(Thread *thread, CONTEXT * cont { // Fixup return address on stack #if defined(TARGET_X86) || defined(TARGET_AMD64) - SIZE_T *sp = (SIZE_T *) GetSP(context); + if (!IsInPlaceSingleStep()) + { + // Fixup return address on stack + SIZE_T *sp = (SIZE_T *) GetSP(context); - LOG((LF_CORDB, LL_INFO10000, - "Bypass call return address redirected from %p\n", *sp)); + LOG((LF_CORDB, LL_INFO10000, + "Bypass call return address redirected from %p\n", *sp)); - *sp -= patchBypass - (BYTE*)m_address; + *sp -= patchBypass - (BYTE*)m_address; - LOG((LF_CORDB, LL_INFO10000, "to %p\n", *sp)); + LOG((LF_CORDB, LL_INFO10000, "to %p\n", *sp)); + } #else PORTABILITY_ASSERT("DebuggerPatchSkip::TriggerExceptionHook -- return address fixup NYI"); #endif @@ -4797,10 +4847,13 @@ TP_RESULT DebuggerPatchSkip::TriggerExceptionHook(Thread *thread, CONTEXT * cont ((size_t)GetIP(context) > (size_t)patchBypass && (size_t)GetIP(context) <= (size_t)(patchBypass + MAX_INSTRUCTION_LENGTH + 1))) { - LOG((LF_CORDB, LL_INFO10000, "Bypass instruction redirected because still in skip area.\n" - "\tm_fIsCall = %s, patchBypass = %p, m_address = %p\n", - (m_instrAttrib.m_fIsCall ? "true" : "false"), patchBypass, m_address)); - SetIP(context, (PCODE)((BYTE *)GetIP(context) - (patchBypass - (BYTE *)m_address))); + if (!IsInPlaceSingleStep()) + { + LOG((LF_CORDB, LL_INFO10000, "Bypass instruction redirected because still in skip area.\n" + "\tm_fIsCall = %s, patchBypass = %p, m_address = %p\n", + (m_instrAttrib.m_fIsCall ? "true" : "false"), patchBypass, m_address)); + SetIP(context, (PCODE)((BYTE *)GetIP(context) - (patchBypass - (BYTE *)m_address))); + } } else { @@ -4867,7 +4920,7 @@ bool DebuggerPatchSkip::TriggerSingleStep(Thread *thread, const BYTE *ip) #if defined(TARGET_AMD64) // Dev11 91932: for RIP-relative writes we need to copy the value that was written in our buffer to the actual address _ASSERTE(m_pSharedPatchBypassBuffer); - if (m_pSharedPatchBypassBuffer->RipTargetFixup) + if (m_pSharedPatchBypassBuffer->RipTargetFixup && !IsInPlaceSingleStep()) { _ASSERTE(m_pSharedPatchBypassBuffer->RipTargetFixupSize); diff --git a/src/coreclr/debug/ee/controller.h b/src/coreclr/debug/ee/controller.h index 5c14bb201554d9..30521e90a27a2d 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -1072,7 +1072,12 @@ class DebuggerController static bool DispatchNativeException(EXCEPTION_RECORD *exception, CONTEXT *context, DWORD code, - Thread *thread); + Thread *thread +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL +#endif + ); static bool DispatchUnwind(Thread *thread, MethodDesc *fd, DebuggerJitInfo * pDJI, SIZE_T offset, @@ -1114,13 +1119,23 @@ class DebuggerController static DebuggerPatchSkip *ActivatePatchSkip(Thread *thread, const BYTE *eip, - BOOL fForEnC); + BOOL fForEnC +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL +#endif + ); static DPOSS_ACTION DispatchPatchOrSingleStep(Thread *thread, CONTEXT *context, CORDB_ADDRESS_TYPE *ip, - SCAN_TRIGGER which); + SCAN_TRIGGER which +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + , + DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL +#endif + ); static int GetNumberOfPatches() @@ -1450,6 +1465,23 @@ class DebuggerController #if !defined(DACCESS_COMPILE) +// this structure stores useful information about single-stepping over a call instruction +// it is used to communicate the patch skip opcode and current state between the controller on left side and HandleSetThreadContextNeeded on the right side +class DebuggerSteppingInfo +{ + bool fIsInPlaceSingleStep = false; + PRD_TYPE opcode = 0; + +public: + bool IsInPlaceSingleStep() { return fIsInPlaceSingleStep; } + PRD_TYPE GetOpcode() { return opcode; } + void EnableInPlaceSingleStepOverCall(PRD_TYPE opcode) + { + this->fIsInPlaceSingleStep = true; + this->opcode = opcode; + } +}; + /* ------------------------------------------------------------------------- * * DebuggerPatchSkip routines * ------------------------------------------------------------------------- */ @@ -1485,6 +1517,9 @@ class DebuggerPatchSkip : public DebuggerController CORDB_ADDRESS_TYPE *m_address; int m_iOrigDisp; // the original displacement of a relative call or jump InstructionAttribute m_instrAttrib; // info about the instruction being skipped over +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + bool m_fInPlaceSS; // is this an in-place single-step instruction? +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT #ifndef FEATURE_EMULATE_SINGLESTEP // this is shared among all the skippers and the controller. see the comments // right before the definition of SharedPatchBypassBuffer for lifetime info. @@ -1497,7 +1532,23 @@ class DebuggerPatchSkip : public DebuggerController BYTE* patchBypass = m_pSharedPatchBypassBuffer->PatchBypass; return (CORDB_ADDRESS_TYPE *)patchBypass; } + #endif // !FEATURE_EMULATE_SINGLESTEP + + BOOL IsInPlaceSingleStep() + { +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT +#ifndef FEATURE_EMULATE_SINGLESTEP + // only in-place single steps over call intructions are supported at this time + _ASSERTE(m_instrAttrib.m_fIsCall); + return m_fInPlaceSS; +#else +#error only non-emulated single-steps with OUT_OF_PROCESS_SETTHREADCONTEXT enabled are supported +#endif +#else + return false; +#endif + } }; /* ------------------------------------------------------------------------- * diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 695f48752c16c3..3c3883cb6590ab 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -5462,6 +5462,9 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception, } bool retVal; +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + DebuggerSteppingInfo debuggerSteppingInfo; +#endif { // Don't stop for native debugging anywhere inside our inproc-Filters. @@ -5470,7 +5473,11 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception, if (!CORDBUnrecoverableError(this)) { retVal = DebuggerController::DispatchNativeException(exception, context, - code, thread); + code, thread +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + ,&debuggerSteppingInfo +#endif + ); } else { @@ -5481,7 +5488,9 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception, #if defined(OUT_OF_PROCESS_SETTHREADCONTEXT) && !defined(DACCESS_COMPILE) if (retVal && fIsVEH) { - SendSetThreadContextNeeded(context); + // This does not return. Out-of-proc debugger will update the thread context + // within this call. + SendSetThreadContextNeeded(context, &debuggerSteppingInfo); } #endif return retVal; @@ -16691,7 +16700,7 @@ void Debugger::StartCanaryThread() #ifndef DACCESS_COMPILE #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT -void Debugger::SendSetThreadContextNeeded(CONTEXT *context) +void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo) { STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_NOTRIGGER; @@ -16742,11 +16751,14 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context) // adjust context size if the context pointer is not aligned with the buffer we allocated contextSize -= (DWORD)((BYTE*)pContext-(BYTE*)pBuffer); + bool fIsInPlaceSingleStep = pDebuggerSteppingInfo != NULL && pDebuggerSteppingInfo->IsInPlaceSingleStep(); + PRD_TYPE opcode = pDebuggerSteppingInfo != NULL ? pDebuggerSteppingInfo->GetOpcode() : CORDbg_BREAK_INSTRUCTION; + // send the context to the right side - LOG((LF_CORDB, LL_INFO10000, "D::SSTCN ContextFlags=0x%X contextSize=%d..\n", contextFlags, contextSize)); + LOG((LF_CORDB, LL_INFO10000, "D::SSTCN ContextFlags=0x%X contextSize=%d fIsInPlaceSingleStep=%d opcode=%x\n", contextFlags, contextSize, fIsInPlaceSingleStep, opcode)); EX_TRY { - SetThreadContextNeededFlare((TADDR)pContext, contextSize, pContext->Rip, pContext->Rsp); + SetThreadContextNeededFlare((TADDR)pContext, contextSize, fIsInPlaceSingleStep, opcode); } EX_CATCH { @@ -16765,7 +16777,7 @@ BOOL Debugger::IsOutOfProcessSetContextEnabled() return m_fOutOfProcessSetContextEnabled; } #else -void Debugger::SendSetThreadContextNeeded(CONTEXT* context) +void Debugger::SendSetThreadContextNeeded(CONTEXT* context, DebuggerSteppingInfo *pDebuggerSteppingInfo) { _ASSERTE(!"SendSetThreadContextNeeded is not enabled on this platform"); } diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index d9513cac8f5abc..2b252c7b566d12 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -1873,7 +1873,7 @@ extern "C" void __stdcall NotifyRightSideOfSyncCompleteFlare(void); extern "C" void __stdcall NotifySecondChanceReadyForDataFlare(void); #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT #if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) -extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size, TADDR Rip, TADDR Rsp); +extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size, bool fIsInPlaceSingleStep, PRD_TYPE opcode); #else #error Platform not supported #endif @@ -1888,6 +1888,7 @@ extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size struct ShouldAttachDebuggerParams; struct EnsureDebuggerAttachedParams; struct SendMDANotificationParams; +class DebuggerSteppingInfo; // class Debugger: This class implements DebugInterface to provide // the hooks to the Runtime directly. @@ -3058,7 +3059,8 @@ class Debugger : public DebugInterface private: BOOL m_fOutOfProcessSetContextEnabled; public: - void SendSetThreadContextNeeded(CONTEXT *context); + // Used by Debugger::FirstChanceNativeException to update the context from out of process + void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL); BOOL IsOutOfProcessSetContextEnabled(); }; diff --git a/src/coreclr/vm/dbginterface.h b/src/coreclr/vm/dbginterface.h index 9a58eea52ad256..a50c33c1af108c 100644 --- a/src/coreclr/vm/dbginterface.h +++ b/src/coreclr/vm/dbginterface.h @@ -19,6 +19,8 @@ typedef DPTR(struct ICorDebugInfo::NativeVarInfo) PTR_NativeVarInfo; typedef void (*FAVORCALLBACK)(void *); +class DebuggerSteppingInfo; + // // The purpose of this object is to serve as an entry point to the // debugger, which used to reside in a separate DLL. @@ -192,8 +194,8 @@ class DebugInterface SIZE_T *nativeOffset) = 0; - // Used by FixContextAndResume - virtual void SendSetThreadContextNeeded(CONTEXT *context) = 0; + // Used by EditAndContinueModule::FixContextAndResume + virtual void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = nullptr) = 0; virtual BOOL IsOutOfProcessSetContextEnabled() = 0; #endif // FEATURE_METADATA_UPDATER diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 1506516f12447e..71f59672eba1c7 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -1001,6 +1001,7 @@ BOOL Thread::ReadyForAsyncException() else { CONTEXT ctx; + ctx.ContextFlags = CONTEXT_CONTROL; SetIP(&ctx, 0); SetSP(&ctx, 0); FillRegDisplay(&rd, &ctx);