From 0721e10f4242ddd2263b2fd41c9fafda781ad1b4 Mon Sep 17 00:00:00 2001 From: Tom Date: Fri, 5 Aug 2022 01:39:57 -0400 Subject: [PATCH 1/5] Support managed debugging when CET is enabled --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 25 +++ src/coreclr/debug/daccess/dacdbiimpl.h | 2 + src/coreclr/debug/di/process.cpp | 177 ++++++++++++++++++++++ src/coreclr/debug/di/rspriv.h | 12 ++ src/coreclr/debug/ee/amd64/dbghelpers.asm | 8 +- src/coreclr/debug/ee/debugger.cpp | 141 +++++++++++++++-- src/coreclr/debug/ee/debugger.h | 14 +- src/coreclr/debug/ee/rcthread.cpp | 6 + src/coreclr/debug/inc/dacdbiinterface.h | 3 + src/coreclr/debug/inc/dbgipcevents.h | 1 + src/coreclr/inc/clrconfigvalues.h | 2 + src/coreclr/vm/dbginterface.h | 10 +- src/coreclr/vm/encee.cpp | 9 +- 13 files changed, 390 insertions(+), 20 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 1c8e6863f83af8..fa489bdce2bbef 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -7535,6 +7535,31 @@ HRESULT DacDbiInterfaceImpl::EnableGCNotificationEvents(BOOL fEnable) return hr; } +HRESULT DacDbiInterfaceImpl::ReadData(TADDR pRemoteBuf, DWORD size, BYTE *pLocalBuf) +{ + DD_ENTER_MAY_THROW + + HRESULT hr = S_OK; + EX_TRY + { + ULONG32 cbRead = 0; + + HRESULT hr = m_pTarget->ReadVirtual(pRemoteBuf, pLocalBuf, size, &cbRead); + + if (FAILED(hr)) + { + ThrowHR(CORDBG_E_READVIRTUAL_FAILURE); + } + + if (cbRead != size) + { + ThrowWin32(ERROR_PARTIAL_COPY); + } + } + EX_CATCH_HRESULT(hr); + return hr; +} + DacRefWalker::DacRefWalker(ClrDataAccess *dac, BOOL walkStacks, BOOL walkFQ, UINT32 handleMask) : mDac(dac), mWalkStacks(walkStacks), mWalkFQ(walkFQ), mHandleMask(handleMask), mStackWalker(NULL), mHandleWalker(NULL), mFQStart(PTR_NULL), mFQEnd(PTR_NULL), mFQCurr(PTR_NULL) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.h b/src/coreclr/debug/daccess/dacdbiimpl.h index eb46bbcb48c48e..17be2182574015 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.h +++ b/src/coreclr/debug/daccess/dacdbiimpl.h @@ -365,6 +365,8 @@ class DacDbiInterfaceImpl : HRESULT IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isModuleMapped); + HRESULT ReadData(TADDR pRemoteBuf, DWORD size, BYTE *pLocalBuf); + bool MetadataUpdatesApplied(); // retrieves the list of COM interfaces implemented by vmObject, as it is known at diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index b325d122062949..f3bd561d3662d9 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -7659,6 +7659,8 @@ HRESULT CordbProcess::GetRuntimeOffsets() m_runtimeOffsets.m_notifyRSOfSyncCompleteBPAddr)); LOG((LF_CORDB, LL_INFO10000, " m_debuggerWordTLSIndex= 0x%08x\n", m_runtimeOffsets.m_debuggerWordTLSIndex)); + LOG((LF_CORDB, LL_INFO10000, " m_setThreadContextNeededAddr= 0x%p\n", + m_runtimeOffsets.m_setThreadContextNeededAddr)); #endif // FEATURE_INTEROP_DEBUGGING LOG((LF_CORDB, LL_INFO10000, " m_TLSIndex= 0x%08x\n", @@ -7719,6 +7721,7 @@ HRESULT CordbProcess::GetRuntimeOffsets() m_runtimeOffsets.m_signalHijackCompleteBPAddr, m_runtimeOffsets.m_excepNotForRuntimeBPAddr, m_runtimeOffsets.m_notifyRSOfSyncCompleteBPAddr, + m_runtimeOffsets.m_setThreadContextNeededAddr, }; const int NumFlares = ARRAY_SIZE(flares); @@ -11152,7 +11155,41 @@ void CordbProcess::FilterClrNotification( } } +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) +{ + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded\n")); + + CONTEXT context = { 0 }; + context.ContextFlags = CONTEXT_FULL; + + GetLiveContext(dwThreadId, &context); + TADDR lsContextAddr = (TADDR)context.Rcx; + DWORD contextSize = (DWORD)context.Rdx; + + if (contextSize == 0 || contextSize > sizeof(CONTEXT) + 25000) + { + _ASSERTE(!"Corrupted HandleSetThreadContextNeeded message received"); + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Corrupted HandleSetThreadContextNeeded message received\n")); + + ThrowHR(E_UNEXPECTED); + } + + PCONTEXT pContext = (PCONTEXT)_alloca(contextSize); + HRESULT hr = GetDAC()->ReadData(lsContextAddr, contextSize, (BYTE*)pContext); + if (FAILED(hr)) + { + _ASSERTE(!"ReadData failed"); + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from ReadData (error: 0x%X).\n", hr)); + + ThrowHR(hr); + } + + SetLiveContext(dwThreadId, pContext); +} +#endif // // If the thread has an unhandled managed exception, hijack it. @@ -11377,7 +11414,15 @@ HRESULT CordbProcess::Filter( // holder will invoke DeleteIPCEventHelper(pManagedEvent). } +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) + else if (dwFirstChance && pRecord->ExceptionCode == STATUS_BREAKPOINT && pRecord->ExceptionAddress == m_runtimeOffsets.m_setThreadContextNeededAddr) /*|| pRecord->ExceptionCode == STATUS_SINGLE_STEP*/ + { + // this is a request to set the thread context out of process + HandleSetThreadContextNeeded(dwThreadId); + *pContinueStatus = DBG_CONTINUE; + } +#endif } PUBLIC_API_END(hr); // we may not find the correct mscordacwks so fail gracefully @@ -15171,3 +15216,135 @@ void CordbProcess::HandleControlCTrapResult(HRESULT result) // Send the reply to the LS. SendIPCEvent(&eventControlCResult, sizeof(eventControlCResult)); } + + +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +void CordbProcess::GetLiveContext(DWORD dwThreadId, PCONTEXT pContext) +{ + LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext\n")); + + HandleHolder hThread = OpenThread( + THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME, + FALSE, // thread handle is not inheritable. + dwThreadId); + + if (hThread == NULL) + { + ThrowHR(E_UNEXPECTED); + } + + DWORD previousSuspendCount = ::SuspendThread(hThread); + if (previousSuspendCount == (DWORD)-1) + { + LOG((LF_CORDB, LL_INFO10000, "RS DB_IPCE_SET_THREADCONTEXT_NEEDED - Unexpected result from SuspendThread\n")); + ThrowHR(HRESULT_FROM_GetLastError()); + } + else + { + DWORD lastError = 0; + BOOL success = ::GetThreadContext(hThread, pContext); + if (!success) + { + lastError = ::GetLastError(); + } + + LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext - Get Thread Context Completed: Success=%d GetLastError=%d hr=0x%X\n", success, lastError, HRESULT_FROM_WIN32(lastError))); + _ASSERTE(success); + + DWORD suspendCount = ::ResumeThread(hThread); + if (suspendCount == (DWORD)-1) + { + LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext - Unexpected result from ResumeThread\n")); + ThrowHR(HRESULT_FROM_GetLastError()); + } + + if (!success) + { + LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext - Unexpected result from GetThreadContext\n")); + ThrowHR(HRESULT_FROM_WIN32(lastError)); + } + } +} + +void CordbProcess::SetLiveContext(DWORD dwThreadId, PCONTEXT pContext) +{ + // The initialize call should fail but return contextSize + DWORD contextSize = 0; + DWORD contextFlags = pContext->ContextFlags; + BOOL success = InitializeContext(NULL, contextFlags, NULL, &contextSize); + + _ASSERTE(!success && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)); + + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - InitializeContext ContextSize %d\n", contextSize)); + + PVOID pBuffer = _alloca(contextSize); + PCONTEXT pFrameContext = NULL; + success = InitializeContext(pBuffer, contextFlags, &pFrameContext, &contextSize); + if (!success) + { + HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); + _ASSERTE(!"InitializeContext failed"); + + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from InitializeContext (error: 0x%X [%d]).\n", hr, GetLastError())); + + ThrowHR(hr); + } + + _ASSERTE((BYTE*)pFrameContext == pBuffer); + + success = CopyContext(pFrameContext, contextFlags, pContext); + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - CopyContext=%s %d\n", success?"SUCCESS":"FAIL", GetLastError())); + if (!success) + { + HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); + _ASSERTE(!"CopyContext failed"); + + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from CopyContext (error: 0x%X [%d]).\n", hr, GetLastError())); + + ThrowHR(hr); + } + + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Set Thread Context - ID = 0x%X, SS enabled = %d\n", dwThreadId, /*(uint64_t)hThread,*/ (pContext->EFlags & 0x100) != 0)); + + HandleHolder hThread = OpenThread( + THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME, + FALSE, // thread handle is not inheritable. + dwThreadId); + + if (hThread != NULL) + { + DWORD previousSuspendCount = ::SuspendThread(hThread); + if (previousSuspendCount == (DWORD)-1) + { + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from SuspendThread\n")); + ThrowHR(HRESULT_FROM_GetLastError()); + } + else + { + DWORD lastError = 0; + + success = ::SetThreadContext(hThread, pFrameContext); + if (!success) + { + lastError = ::GetLastError(); + } + + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Set Thread Context Completed: Success=%d GetLastError=%d hr=0x%X\n", success, lastError, HRESULT_FROM_WIN32(lastError))); + _ASSERTE(success); + + DWORD suspendCount = ::ResumeThread(hThread); + if (suspendCount == (DWORD)-1 || suspendCount != previousSuspendCount + 1) + { + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from ResumeThread\n")); + ThrowHR(HRESULT_FROM_GetLastError()); + } + + if (!success) + { + LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from SetThreadContext\n")); + ThrowHR(HRESULT_FROM_WIN32(lastError)); + } + } + } +} +#endif diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 30e474a2fdc097..cf8d50aafacf69 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -3280,6 +3280,10 @@ class CordbProcess : #endif } +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) + void HandleSetThreadContextNeeded(DWORD dwThreadId); +#endif + // // Shim callbacks to simulate fake attach events. // @@ -4115,6 +4119,14 @@ class CordbProcess : WriteableMetadataUpdateMode m_writableMetadataUpdateMode; COM_METHOD GetObjectInternal(CORDB_ADDRESS addr, CordbAppDomain* pAppDomainOverride, ICorDebugObjectValue **pObject); + +private: + +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) + void GetLiveContext(DWORD dwThreadId, PCONTEXT pContext); + void SetLiveContext(DWORD dwThreadId, PCONTEXT pContext); +#endif + }; // Some IMDArocess APIs are supported as interop-only. diff --git a/src/coreclr/debug/ee/amd64/dbghelpers.asm b/src/coreclr/debug/ee/amd64/dbghelpers.asm index 49f01283829da5..2ec3483f68538a 100644 --- a/src/coreclr/debug/ee/amd64/dbghelpers.asm +++ b/src/coreclr/debug/ee/amd64/dbghelpers.asm @@ -157,7 +157,13 @@ LEAF_ENTRY NotifyRightSideOfSyncCompleteFlare, _TEXT ret LEAF_END NotifyRightSideOfSyncCompleteFlare, _TEXT - +; Flare for setting the context out of process +LEAF_ENTRY SetThreadContextNeededFlare, _TEXT + int 3 + ; make sure that the basic block is unique + test rax,7 + ret +LEAF_END SetThreadContextNeededFlare, _TEXT ; This goes at the end of the assembly file end diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index f404ef4570d92b..5a7f069a3bc4c3 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -971,7 +971,12 @@ Debugger::Debugger() // as data structure layouts change, add a new version number // and comment the changes m_mdDataStructureVersion = 1; - + m_fOutOfProcessSetContextEnabled = +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) && !defined(DACCESS_COMPILE) + Thread::AreCetShadowStacksEnabled() || CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_OutOfProcessSetContext) != 0; +#else + FALSE; +#endif } /****************************************************************************** @@ -5504,7 +5509,8 @@ bool Debugger::IsJMCMethod(Module* pModule, mdMethodDef tkMethod) bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception, CONTEXT *context, DWORD code, - Thread *thread) + Thread *thread, + BOOL fIsVEH) { // @@@ @@ -5537,19 +5543,27 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception, bool retVal; - // Don't stop for native debugging anywhere inside our inproc-Filters. - CantStopHolder hHolder; - - if (!CORDBUnrecoverableError(this)) { - retVal = DebuggerController::DispatchNativeException(exception, context, - code, thread); + // Don't stop for native debugging anywhere inside our inproc-Filters. + CantStopHolder hHolder; + + if (!CORDBUnrecoverableError(this)) + { + retVal = DebuggerController::DispatchNativeException(exception, context, + code, thread); + } + else + { + retVal = false; + } } - else + +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) && !defined(DACCESS_COMPILE) + if (retVal && fIsVEH) { - retVal = false; + SendSetThreadContextNeeded(context); } - +#endif return retVal; } @@ -13146,7 +13160,7 @@ void STDCALL ExceptionHijackWorker( break; case EHijackReason::kFirstChanceSuspend: _ASSERTE(pData == NULL); - g_pDebugger->FirstChanceSuspendHijackWorker(pContext, pRecord); + g_pDebugger->FirstChanceSuspendHijackWorker(pContext, pRecord, FALSE); break; case EHijackReason::kGenericHijack: _ASSERTE(pData == NULL); @@ -13463,7 +13477,8 @@ VOID Debugger::M2UHandoffHijackWorker(CONTEXT *pContext, okay = g_pDebugger->FirstChanceNativeException(pExceptionRecord, pContext, pExceptionRecord->ExceptionCode, - pEEThread); + pEEThread, + FALSE); _ASSERTE(okay == true); LOG((LF_CORDB, LL_INFO1000, "D::M2UHHW: FirstChanceNativeException returned\n")); } @@ -13502,7 +13517,8 @@ VOID Debugger::M2UHandoffHijackWorker(CONTEXT *pContext, // - this thread is not in cooperative mode. //----------------------------------------------------------------------------- LONG Debugger::FirstChanceSuspendHijackWorker(CONTEXT *pContext, - EXCEPTION_RECORD *pExceptionRecord) + EXCEPTION_RECORD *pExceptionRecord, + BOOL fIsVEH) { // if we aren't set up to do interop debugging this function should just bail out if(m_pRCThread == NULL) @@ -13638,6 +13654,12 @@ LONG Debugger::FirstChanceSuspendHijackWorker(CONTEXT *pContext, if (pFcd->action == HIJACK_ACTION_EXIT_HANDLED) { SPEW(fprintf(stderr, "0x%x D::FCHF: exiting with CONTINUE_EXECUTION\n", tid)); +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) && !defined(DACCESS_COMPILE) + if (fIsVEH) + { + SendSetThreadContextNeeded(pContext); + } +#endif return EXCEPTION_CONTINUE_EXECUTION; } else @@ -16631,4 +16653,95 @@ void Debugger::StartCanaryThread() } #endif // DACCESS_COMPILE +#ifndef DACCESS_COMPILE +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +void Debugger::SendSetThreadContextNeeded(CONTEXT *context) +{ + CONTRACTL + { + NOTHROW; + GC_TRIGGERS; + MODE_ANY; + PRECONDITION(context != NULL); + } + CONTRACTL_END; + + if (!m_fOutOfProcessSetContextEnabled) + return; + + DWORD contextFlags = context->ContextFlags; + DWORD contextSize = 0; + + // determine the context size + BOOL success = InitializeContext(NULL, contextFlags, NULL, &contextSize); + if (success || GetLastError() != ERROR_INSUFFICIENT_BUFFER || contextSize == 0) + { + // The initialize call should fail but return contextSize + _ASSERTE(!"InitializeContext unexpectedly failed\n"); + return; + } + + // allocate a temp buffer for the context + BYTE *pBuffer = (BYTE*)_alloca(contextSize); + if (pBuffer == NULL) + { + _ASSERTE(!"Failed to allocate context buffer"); + LOG((LF_CORDB, LL_INFO10000, "D::SSTCN Failed to allocate context buffer\n")); + return; + } + + // make a copy of the context + PCONTEXT pContext = NULL; + success = InitializeContext(pBuffer, contextFlags, &pContext, &contextSize); + if (!success) + { + _ASSERTE(!"InitializeContext failed"); + LOG((LF_CORDB, LL_INFO10000, "D::SSTCN Unexpected result from InitializeContext (error: %d).\n", GetLastError())); + return; + } + + success = CopyContext(pContext, contextFlags, context); + if (!success) + { + _ASSERTE(!"CopyContext failed"); + LOG((LF_CORDB, LL_INFO10000, "D::SSTCN Unexpected result from CopyContext (error: %d).\n", GetLastError())); + return; + } + + // adjust context size if the context pointer is not aligned with the buffer we allocated + contextSize -= (DWORD)((BYTE*)pContext-(BYTE*)pBuffer); + + // send the context to the right side + LOG((LF_CORDB, LL_INFO10000, "D::SSTCN ContextFlags=0x%X contextSize=%d..\n", contextFlags, contextSize)); + EX_TRY + { + SetThreadContextNeededFlare((TADDR)pContext, contextSize); + } + EX_CATCH + { + } + EX_END_CATCH(SwallowAllExceptions); + + LOG((LF_CORDB, LL_INFO10000, "D::SSTCN SetThreadContextNeededFlare returned\n")); + _ASSERTE(!"We failed to SetThreadContext from out of process!"); +} + +BOOL Debugger::IsOutOfProcessSetContextEnabled() +{ + return m_fOutOfProcessSetContextEnabled; +} +#else +void Debugger::SendSetThreadContextNeeded(CONTEXT* context) +{ + _ASSERTE(!"SendSetThreadContextNeeded is not supported on this platform"); +} + +BOOL Debugger::IsOutOfProcessSetContextEnabled() +{ + return FALSE; +} +#endif // defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#endif // DACCESS_COMPILE + #endif //DEBUGGING_SUPPORTED + diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 8b5a79543461f2..b0ee7ddae59b9c 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -1751,6 +1751,9 @@ extern "C" void __stdcall SignalHijackCompleteFlare(void); extern "C" void __stdcall ExceptionNotForRuntimeFlare(void); extern "C" void __stdcall NotifyRightSideOfSyncCompleteFlare(void); extern "C" void __stdcall NotifySecondChanceReadyForDataFlare(void); +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size); +#endif /* ------------------------------------------------------------------------ * * Debugger class @@ -1900,7 +1903,8 @@ class Debugger : public DebugInterface bool FirstChanceNativeException(EXCEPTION_RECORD *exception, T_CONTEXT *context, DWORD code, - Thread *thread); + Thread *thread, + BOOL fIsVEH = TRUE); bool IsJMCMethod(Module* pModule, mdMethodDef tkMethod); @@ -2346,7 +2350,8 @@ class Debugger : public DebugInterface LONG FirstChanceSuspendHijackWorker( T_CONTEXT *pContext, - EXCEPTION_RECORD *pExceptionRecord); + EXCEPTION_RECORD *pExceptionRecord, + BOOL fIsVEH = TRUE); static void GenericHijackFunc(void); static void SecondChanceHijackFunc(void); static void SecondChanceHijackFuncWorker(void); @@ -2917,6 +2922,11 @@ class Debugger : public DebugInterface private: HANDLE GetGarbageCollectionBlockerEvent() { return GetLazyData()->m_garbageCollectionBlockerEvent; } +private: + BOOL m_fOutOfProcessSetContextEnabled; +public: + void SendSetThreadContextNeeded(CONTEXT *context); + BOOL IsOutOfProcessSetContextEnabled(); }; diff --git a/src/coreclr/debug/ee/rcthread.cpp b/src/coreclr/debug/ee/rcthread.cpp index 24644e11cd6bd0..0191096aa5acbc 100644 --- a/src/coreclr/debug/ee/rcthread.cpp +++ b/src/coreclr/debug/ee/rcthread.cpp @@ -486,6 +486,12 @@ HRESULT DebuggerRCThread::SetupRuntimeOffsets(DebuggerIPCControlBlock * pDebugge pDebuggerRuntimeOffsets->m_debuggerWordTLSIndex = g_debuggerWordTLSIndex; #endif // FEATURE_INTEROP_DEBUGGING +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) + pDebuggerRuntimeOffsets->m_setThreadContextNeededAddr = (void*) SetThreadContextNeededFlare; +#else + pDebuggerRuntimeOffsets->m_setThreadContextNeededAddr = NULL; +#endif + pDebuggerRuntimeOffsets->m_pPatches = DebuggerController::GetPatchTable(); pDebuggerRuntimeOffsets->m_pPatchTableValid = (BOOL*)DebuggerController::GetPatchTableValidAddr(); pDebuggerRuntimeOffsets->m_offRgData = DebuggerPatchTable::GetOffsetOfEntries(); diff --git a/src/coreclr/debug/inc/dacdbiinterface.h b/src/coreclr/debug/inc/dacdbiinterface.h index ca36f58ff64392..802e317a4032f4 100644 --- a/src/coreclr/debug/inc/dacdbiinterface.h +++ b/src/coreclr/debug/inc/dacdbiinterface.h @@ -2747,6 +2747,9 @@ class IDacDbiInterface virtual HRESULT IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isModuleMapped) = 0; + virtual + HRESULT ReadData(TADDR pRemoteBuf, DWORD size, BYTE *pLocalBuf) = 0; + virtual bool MetadataUpdatesApplied() = 0; diff --git a/src/coreclr/debug/inc/dbgipcevents.h b/src/coreclr/debug/inc/dbgipcevents.h index 6dbd7fe25a7a70..f40eae3c6a4276 100644 --- a/src/coreclr/debug/inc/dbgipcevents.h +++ b/src/coreclr/debug/inc/dbgipcevents.h @@ -153,6 +153,7 @@ struct MSLAYOUT DebuggerIPCRuntimeOffsets SIZE_T m_cbOpcode; // Max size of opcode SIZE_T m_offTraceType; // Offset of the trace.type within a patch DWORD m_traceTypeUnmanaged; // TRACE_UNMANAGED + void *m_setThreadContextNeededAddr; // Address of SetThreadContextNeededFlare DebuggerIPCRuntimeOffsets() { diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index 747685b9767b6f..ce7d1836a363e9 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -232,6 +232,8 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_MiniMdBufferCapacity, W("MiniMdBufferCapacity" CONFIG_DWORD_INFO(INTERNAL_DbgNativeCodeBpBindsAcrossVersions, W("DbgNativeCodeBpBindsAcrossVersions"), 0, "If non-zero causes native breakpoints at offset 0 to bind in all tiered compilation versions of the given method") RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_RichDebugInfo, W("RichDebugInfo"), 0, "If non-zero store some additional debug information for each jitted method") +RETAIL_CONFIG_DWORD_INFO(EXTERNAL_OutOfProcessSetContext, W("OutOfProcessSetContext"), 0, "If enabled the debugger will not modify thread contexts in-process. Enabled by default when CET is enabled for the process.") + /// /// Diagnostics (internal general-purpose) /// diff --git a/src/coreclr/vm/dbginterface.h b/src/coreclr/vm/dbginterface.h index 9a30c5876ae090..25e4bf81722aca 100644 --- a/src/coreclr/vm/dbginterface.h +++ b/src/coreclr/vm/dbginterface.h @@ -108,7 +108,8 @@ class DebugInterface virtual bool FirstChanceNativeException(EXCEPTION_RECORD *exception, CONTEXT *context, DWORD code, - Thread *thread) = 0; + Thread *thread, + BOOL fIsVEH = TRUE) = 0; // pThread is thread that exception is on. // currentSP is stack frame of the throw site. @@ -193,6 +194,11 @@ class DebugInterface SIZE_T ilOffset, TADDR nativeFnxStart, SIZE_T *nativeOffset) = 0; + + + // Used by FixContextAndResume + virtual void SendSetThreadContextNeeded(CONTEXT *context) = 0; + virtual BOOL IsOutOfProcessSetContextEnabled() = 0; #endif // EnC_SUPPORTED // Get debugger variable information for a specific version of a method @@ -389,7 +395,7 @@ class DebugInterface virtual BOOL FallbackJITAttachPrompt() = 0; #ifdef FEATURE_INTEROP_DEBUGGING - virtual LONG FirstChanceSuspendHijackWorker(PCONTEXT pContext, PEXCEPTION_RECORD pExceptionRecord) = 0; + virtual LONG FirstChanceSuspendHijackWorker(PCONTEXT pContext, PEXCEPTION_RECORD pExceptionRecord, BOOL fIsVEH = TRUE) = 0; #endif // Helper method for cleaning up transport socket diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index f81ade62be7aed..c83fd50e565a1f 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -835,7 +835,14 @@ NOINLINE void EditAndContinueModule::FixContextAndResume( #if defined(TARGET_X86) ResumeAtJit(pContext, oldSP); #elif defined(TARGET_WINDOWS) && defined(TARGET_AMD64) - ClrRestoreNonvolatileContextWorker(pContext, ssp); + if (g_pDebugInterface->IsOutOfProcessSetContextEnabled()) + { + g_pDebugInterface->SendSetThreadContextNeeded(pContext); + } + else + { + ClrRestoreNonvolatileContextWorker(pContext, ssp); + } #else ClrRestoreNonvolatileContext(pContext); #endif From 05ae1b6278887149c709bf23120817c9b5e40e87 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 9 Aug 2022 02:05:26 -0400 Subject: [PATCH 2/5] Use DataTarget ReadVirtual function to read data Remove RS GetLiveContext/SetLiveContext implementations Update debugger contract --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 25 --- src/coreclr/debug/daccess/dacdbiimpl.h | 2 - src/coreclr/debug/di/process.cpp | 242 ++++++++++------------- src/coreclr/debug/di/rspriv.h | 8 - src/coreclr/debug/ee/rcthread.cpp | 2 +- src/coreclr/debug/inc/dacdbiinterface.h | 3 - 6 files changed, 103 insertions(+), 179 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index fa489bdce2bbef..1c8e6863f83af8 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -7535,31 +7535,6 @@ HRESULT DacDbiInterfaceImpl::EnableGCNotificationEvents(BOOL fEnable) return hr; } -HRESULT DacDbiInterfaceImpl::ReadData(TADDR pRemoteBuf, DWORD size, BYTE *pLocalBuf) -{ - DD_ENTER_MAY_THROW - - HRESULT hr = S_OK; - EX_TRY - { - ULONG32 cbRead = 0; - - HRESULT hr = m_pTarget->ReadVirtual(pRemoteBuf, pLocalBuf, size, &cbRead); - - if (FAILED(hr)) - { - ThrowHR(CORDBG_E_READVIRTUAL_FAILURE); - } - - if (cbRead != size) - { - ThrowWin32(ERROR_PARTIAL_COPY); - } - } - EX_CATCH_HRESULT(hr); - return hr; -} - DacRefWalker::DacRefWalker(ClrDataAccess *dac, BOOL walkStacks, BOOL walkFQ, UINT32 handleMask) : mDac(dac), mWalkStacks(walkStacks), mWalkFQ(walkFQ), mHandleMask(handleMask), mStackWalker(NULL), mHandleWalker(NULL), mFQStart(PTR_NULL), mFQEnd(PTR_NULL), mFQCurr(PTR_NULL) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.h b/src/coreclr/debug/daccess/dacdbiimpl.h index 17be2182574015..eb46bbcb48c48e 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.h +++ b/src/coreclr/debug/daccess/dacdbiimpl.h @@ -365,8 +365,6 @@ class DacDbiInterfaceImpl : HRESULT IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isModuleMapped); - HRESULT ReadData(TADDR pRemoteBuf, DWORD size, BYTE *pLocalBuf); - bool MetadataUpdatesApplied(); // retrieves the list of COM interfaces implemented by vmObject, as it is known at diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index f3bd561d3662d9..83241026c2541b 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -7659,10 +7659,10 @@ HRESULT CordbProcess::GetRuntimeOffsets() m_runtimeOffsets.m_notifyRSOfSyncCompleteBPAddr)); LOG((LF_CORDB, LL_INFO10000, " m_debuggerWordTLSIndex= 0x%08x\n", m_runtimeOffsets.m_debuggerWordTLSIndex)); - LOG((LF_CORDB, LL_INFO10000, " m_setThreadContextNeededAddr= 0x%p\n", - m_runtimeOffsets.m_setThreadContextNeededAddr)); #endif // FEATURE_INTEROP_DEBUGGING + LOG((LF_CORDB, LL_INFO10000, " m_setThreadContextNeededAddr= 0x%p\n", + m_runtimeOffsets.m_setThreadContextNeededAddr)); LOG((LF_CORDB, LL_INFO10000, " m_TLSIndex= 0x%08x\n", m_runtimeOffsets.m_TLSIndex)); LOG((LF_CORDB, LL_INFO10000, " m_EEThreadStateOffset= 0x%08x\n", @@ -11160,10 +11160,30 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) { LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded\n")); + HandleHolder hThread = OpenThread( + THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME, + FALSE, // thread handle is not inheritable. + dwThreadId); + + if (hThread == NULL) + { + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from OpenThread\n")); + ThrowHR(E_UNEXPECTED); + } + + DWORD previousSuspendCount = ::SuspendThread(hThread); + if (previousSuspendCount == (DWORD)-1) + { + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from SuspendThread\n")); + ThrowHR(HRESULT_FROM_GetLastError()); + } + CONTEXT context = { 0 }; context.ContextFlags = CONTEXT_FULL; - GetLiveContext(dwThreadId, &context); + HRESULT hr = GetDataTarget()->GetThreadContext(dwThreadId, CONTEXT_FULL, sizeof(CONTEXT), reinterpret_cast (&context)); + IfFailThrow(hr); + TADDR lsContextAddr = (TADDR)context.Rcx; DWORD contextSize = (DWORD)context.Rdx; @@ -11177,17 +11197,91 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) } PCONTEXT pContext = (PCONTEXT)_alloca(contextSize); - HRESULT hr = GetDAC()->ReadData(lsContextAddr, contextSize, (BYTE*)pContext); + ULONG32 cbRead; + hr = GetDataTarget()->ReadVirtual(lsContextAddr, reinterpret_cast(pContext), contextSize, &cbRead); if (FAILED(hr)) { - _ASSERTE(!"ReadData failed"); + ThrowHR(CORDBG_E_READVIRTUAL_FAILURE); + } + + if (cbRead != contextSize) + { + ThrowHR(ERROR_PARTIAL_COPY); + } - LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from ReadData (error: 0x%X).\n", hr)); + // The initialize call should fail but return contextSize + contextSize = 0; + DWORD contextFlags = pContext->ContextFlags; + BOOL success = InitializeContext(NULL, contextFlags, NULL, &contextSize); + + if(success || GetLastError() != ERROR_INSUFFICIENT_BUFFER) + { + _ASSERTE(!"InitializeContext unexpectedly succeeded or didn't return ERROR_INSUFFICIENT_BUFFER"); + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - InitializeContext unexpectedly succeeded or didn't return ERROR_INSUFFICIENT_BUFFER\n")); + + ThrowHR(E_UNEXPECTED); + } + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - InitializeContext ContextSize %d\n", contextSize)); + + PVOID pBuffer = _alloca(contextSize); + PCONTEXT pFrameContext = NULL; + success = InitializeContext(pBuffer, contextFlags, &pFrameContext, &contextSize); + if (!success) + { + HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); + _ASSERTE(!"InitializeContext failed"); + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from InitializeContext (error: 0x%X [%d]).\n", hr, GetLastError())); ThrowHR(hr); } - SetLiveContext(dwThreadId, pContext); + _ASSERTE((BYTE*)pFrameContext == pBuffer); + + success = CopyContext(pFrameContext, contextFlags, pContext); + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - CopyContext=%s %d\n", success?"SUCCESS":"FAIL", GetLastError())); + if (!success) + { + HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); + _ASSERTE(!"CopyContext failed"); + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from CopyContext (error: 0x%X [%d]).\n", hr, GetLastError())); + + ThrowHR(hr); + } + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Set Thread Context - ID = 0x%X, SS enabled = %d\n", dwThreadId, /*(uint64_t)hThread,*/ (pContext->EFlags & 0x100) != 0)); + + DWORD lastError = 0; + + success = ::SetThreadContext(hThread, pFrameContext); + if (!success) + { + lastError = ::GetLastError(); + } + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Set Thread Context Completed: Success=%d GetLastError=%d hr=0x%X\n", success, lastError, HRESULT_FROM_WIN32(lastError))); + _ASSERTE(success); + + DWORD suspendCount = ::ResumeThread(hThread); + if (suspendCount == (DWORD)-1) + { + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from ResumeThread\n")); + ThrowHR(HRESULT_FROM_GetLastError()); + } + if (suspendCount != previousSuspendCount + 1) + { + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from ResumeThread\n")); + ThrowHR(E_UNEXPECTED); + } + + if (!success) + { + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from SetThreadContext\n")); + ThrowHR(HRESULT_FROM_WIN32(lastError)); + } } #endif @@ -11415,7 +11509,7 @@ HRESULT CordbProcess::Filter( // holder will invoke DeleteIPCEventHelper(pManagedEvent). } #if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) - else if (dwFirstChance && pRecord->ExceptionCode == STATUS_BREAKPOINT && pRecord->ExceptionAddress == m_runtimeOffsets.m_setThreadContextNeededAddr) /*|| pRecord->ExceptionCode == STATUS_SINGLE_STEP*/ + else if (dwFirstChance && pRecord->ExceptionCode == STATUS_BREAKPOINT && pRecord->ExceptionAddress == m_runtimeOffsets.m_setThreadContextNeededAddr) { // this is a request to set the thread context out of process @@ -15216,135 +15310,3 @@ void CordbProcess::HandleControlCTrapResult(HRESULT result) // Send the reply to the LS. SendIPCEvent(&eventControlCResult, sizeof(eventControlCResult)); } - - -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) -void CordbProcess::GetLiveContext(DWORD dwThreadId, PCONTEXT pContext) -{ - LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext\n")); - - HandleHolder hThread = OpenThread( - THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME, - FALSE, // thread handle is not inheritable. - dwThreadId); - - if (hThread == NULL) - { - ThrowHR(E_UNEXPECTED); - } - - DWORD previousSuspendCount = ::SuspendThread(hThread); - if (previousSuspendCount == (DWORD)-1) - { - LOG((LF_CORDB, LL_INFO10000, "RS DB_IPCE_SET_THREADCONTEXT_NEEDED - Unexpected result from SuspendThread\n")); - ThrowHR(HRESULT_FROM_GetLastError()); - } - else - { - DWORD lastError = 0; - BOOL success = ::GetThreadContext(hThread, pContext); - if (!success) - { - lastError = ::GetLastError(); - } - - LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext - Get Thread Context Completed: Success=%d GetLastError=%d hr=0x%X\n", success, lastError, HRESULT_FROM_WIN32(lastError))); - _ASSERTE(success); - - DWORD suspendCount = ::ResumeThread(hThread); - if (suspendCount == (DWORD)-1) - { - LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext - Unexpected result from ResumeThread\n")); - ThrowHR(HRESULT_FROM_GetLastError()); - } - - if (!success) - { - LOG((LF_CORDB, LL_INFO10000, "RS GetLiveContext - Unexpected result from GetThreadContext\n")); - ThrowHR(HRESULT_FROM_WIN32(lastError)); - } - } -} - -void CordbProcess::SetLiveContext(DWORD dwThreadId, PCONTEXT pContext) -{ - // The initialize call should fail but return contextSize - DWORD contextSize = 0; - DWORD contextFlags = pContext->ContextFlags; - BOOL success = InitializeContext(NULL, contextFlags, NULL, &contextSize); - - _ASSERTE(!success && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)); - - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - InitializeContext ContextSize %d\n", contextSize)); - - PVOID pBuffer = _alloca(contextSize); - PCONTEXT pFrameContext = NULL; - success = InitializeContext(pBuffer, contextFlags, &pFrameContext, &contextSize); - if (!success) - { - HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); - _ASSERTE(!"InitializeContext failed"); - - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from InitializeContext (error: 0x%X [%d]).\n", hr, GetLastError())); - - ThrowHR(hr); - } - - _ASSERTE((BYTE*)pFrameContext == pBuffer); - - success = CopyContext(pFrameContext, contextFlags, pContext); - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - CopyContext=%s %d\n", success?"SUCCESS":"FAIL", GetLastError())); - if (!success) - { - HRESULT hr = HRESULT_FROM_WIN32(GetLastError()); - _ASSERTE(!"CopyContext failed"); - - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from CopyContext (error: 0x%X [%d]).\n", hr, GetLastError())); - - ThrowHR(hr); - } - - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Set Thread Context - ID = 0x%X, SS enabled = %d\n", dwThreadId, /*(uint64_t)hThread,*/ (pContext->EFlags & 0x100) != 0)); - - HandleHolder hThread = OpenThread( - THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME, - FALSE, // thread handle is not inheritable. - dwThreadId); - - if (hThread != NULL) - { - DWORD previousSuspendCount = ::SuspendThread(hThread); - if (previousSuspendCount == (DWORD)-1) - { - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from SuspendThread\n")); - ThrowHR(HRESULT_FROM_GetLastError()); - } - else - { - DWORD lastError = 0; - - success = ::SetThreadContext(hThread, pFrameContext); - if (!success) - { - lastError = ::GetLastError(); - } - - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Set Thread Context Completed: Success=%d GetLastError=%d hr=0x%X\n", success, lastError, HRESULT_FROM_WIN32(lastError))); - _ASSERTE(success); - - DWORD suspendCount = ::ResumeThread(hThread); - if (suspendCount == (DWORD)-1 || suspendCount != previousSuspendCount + 1) - { - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from ResumeThread\n")); - ThrowHR(HRESULT_FROM_GetLastError()); - } - - if (!success) - { - LOG((LF_CORDB, LL_INFO10000, "RS SetLiveContext - Unexpected result from SetThreadContext\n")); - ThrowHR(HRESULT_FROM_WIN32(lastError)); - } - } - } -} -#endif diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index cf8d50aafacf69..0e3d2cba0626f3 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -4119,14 +4119,6 @@ class CordbProcess : WriteableMetadataUpdateMode m_writableMetadataUpdateMode; COM_METHOD GetObjectInternal(CORDB_ADDRESS addr, CordbAppDomain* pAppDomainOverride, ICorDebugObjectValue **pObject); - -private: - -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) - void GetLiveContext(DWORD dwThreadId, PCONTEXT pContext); - void SetLiveContext(DWORD dwThreadId, PCONTEXT pContext); -#endif - }; // Some IMDArocess APIs are supported as interop-only. diff --git a/src/coreclr/debug/ee/rcthread.cpp b/src/coreclr/debug/ee/rcthread.cpp index 0191096aa5acbc..c074e8080c9c5a 100644 --- a/src/coreclr/debug/ee/rcthread.cpp +++ b/src/coreclr/debug/ee/rcthread.cpp @@ -476,7 +476,7 @@ HRESULT DebuggerRCThread::SetupRuntimeOffsets(DebuggerIPCControlBlock * pDebugge // Fill out the struct. #ifdef FEATURE_INTEROP_DEBUGGING pDebuggerRuntimeOffsets->m_genericHijackFuncAddr = Debugger::GenericHijackFunc; - // Set flares - these only exist for interop debugging. + // the following 6 flares only exist for interop debugging. pDebuggerRuntimeOffsets->m_signalHijackStartedBPAddr = (void*) SignalHijackStartedFlare; pDebuggerRuntimeOffsets->m_excepForRuntimeHandoffStartBPAddr = (void*) ExceptionForRuntimeHandoffStartFlare; pDebuggerRuntimeOffsets->m_excepForRuntimeHandoffCompleteBPAddr = (void*) ExceptionForRuntimeHandoffCompleteFlare; diff --git a/src/coreclr/debug/inc/dacdbiinterface.h b/src/coreclr/debug/inc/dacdbiinterface.h index 802e317a4032f4..ca36f58ff64392 100644 --- a/src/coreclr/debug/inc/dacdbiinterface.h +++ b/src/coreclr/debug/inc/dacdbiinterface.h @@ -2747,9 +2747,6 @@ class IDacDbiInterface virtual HRESULT IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isModuleMapped) = 0; - virtual - HRESULT ReadData(TADDR pRemoteBuf, DWORD size, BYTE *pLocalBuf) = 0; - virtual bool MetadataUpdatesApplied() = 0; From 5b73e5deb79253356ddb8b8a24674cd5b5370ea7 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 9 Aug 2022 13:37:08 -0400 Subject: [PATCH 3/5] Clean up contracts and add extra validation in SetThreadContextNeeded message --- src/coreclr/debug/di/process.cpp | 20 ++++++++++++++++++++ src/coreclr/debug/ee/debugger.cpp | 12 +++--------- src/coreclr/debug/ee/debugger.h | 2 +- src/coreclr/vm/encee.cpp | 5 +---- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index 83241026c2541b..1ad65fd400b036 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -11187,6 +11187,9 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) TADDR lsContextAddr = (TADDR)context.Rcx; DWORD contextSize = (DWORD)context.Rdx; + TADDR expectedRip = (TADDR)context.R8; + TADDR expectedRsp = (TADDR)context.R9; + if (contextSize == 0 || contextSize > sizeof(CONTEXT) + 25000) { _ASSERTE(!"Corrupted HandleSetThreadContextNeeded message received"); @@ -11201,14 +11204,31 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) hr = GetDataTarget()->ReadVirtual(lsContextAddr, reinterpret_cast(pContext), contextSize, &cbRead); if (FAILED(hr)) { + _ASSERTE(!"ReadVirtual failed"); + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - ReadVirtual (error: 0x%X).\n", hr)); + ThrowHR(CORDBG_E_READVIRTUAL_FAILURE); } if (cbRead != contextSize) { + _ASSERTE(!"ReadVirtual context size mismatch"); + + LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - ReadVirtual context size mismatch\n")); + 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); + } + // The initialize call should fail but return contextSize contextSize = 0; DWORD contextFlags = pContext->ContextFlags; diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 5a7f069a3bc4c3..dee74d440d304a 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -16657,14 +16657,8 @@ void Debugger::StartCanaryThread() #if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) void Debugger::SendSetThreadContextNeeded(CONTEXT *context) { - CONTRACTL - { - NOTHROW; - GC_TRIGGERS; - MODE_ANY; - PRECONDITION(context != NULL); - } - CONTRACTL_END; + STATIC_CONTRACT_NOTHROW; + STATIC_CONTRACT_GC_NOTRIGGER; if (!m_fOutOfProcessSetContextEnabled) return; @@ -16715,7 +16709,7 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context) LOG((LF_CORDB, LL_INFO10000, "D::SSTCN ContextFlags=0x%X contextSize=%d..\n", contextFlags, contextSize)); EX_TRY { - SetThreadContextNeededFlare((TADDR)pContext, contextSize); + SetThreadContextNeededFlare((TADDR)pContext, contextSize, pContext->Rip, pContext->Rsp); } EX_CATCH { diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index b0ee7ddae59b9c..f5c55d9abb9bea 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -1752,7 +1752,7 @@ extern "C" void __stdcall ExceptionNotForRuntimeFlare(void); extern "C" void __stdcall NotifyRightSideOfSyncCompleteFlare(void); extern "C" void __stdcall NotifySecondChanceReadyForDataFlare(void); #if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) -extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size); +extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size, TADDR Rip, TADDR Rsp); #endif /* ------------------------------------------------------------------------ * diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index c83fd50e565a1f..f7e7c616ef910e 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -751,9 +751,6 @@ NOINLINE void EditAndContinueModule::FixContextAndResume( STATIC_CONTRACT_GC_TRIGGERS; // Sends IPC event STATIC_CONTRACT_THROWS; -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) - DWORD64 ssp = GetSSP(pContext); -#endif // Create local copies of all structs passed as arguments to prevent them from being overwritten CONTEXT context; memcpy(&context, pContext, sizeof(CONTEXT)); @@ -841,7 +838,7 @@ NOINLINE void EditAndContinueModule::FixContextAndResume( } else { - ClrRestoreNonvolatileContextWorker(pContext, ssp); + ClrRestoreNonvolatileContext(pContext); } #else ClrRestoreNonvolatileContext(pContext); From 3e8704b14c3e0e196be201a6fbae38426a88f1b7 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 9 Aug 2022 14:10:47 -0400 Subject: [PATCH 4/5] Add OUT_OF_PROCESS_SETTHREADCONTEXT ifdef --- src/coreclr/clrdefinitions.cmake | 4 ++++ src/coreclr/debug/di/process.cpp | 10 +++++++--- src/coreclr/debug/di/rspriv.h | 2 +- src/coreclr/debug/ee/debugger.cpp | 16 ++++++++++------ src/coreclr/debug/ee/debugger.h | 4 ++++ src/coreclr/debug/ee/rcthread.cpp | 6 +++++- src/coreclr/vm/encee.cpp | 12 +++++++----- 7 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/coreclr/clrdefinitions.cmake b/src/coreclr/clrdefinitions.cmake index 19adf122925686..bcb533d0e2051d 100644 --- a/src/coreclr/clrdefinitions.cmake +++ b/src/coreclr/clrdefinitions.cmake @@ -62,6 +62,10 @@ if(CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_I386 OR CLR_CMAKE_TARGET endif(CLR_CMAKE_TARGET_WIN32) endif(CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_I386 OR CLR_CMAKE_TARGET_ARCH_ARM64) +if(CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64) +add_compile_definitions(OUT_OF_PROCESS_SETTHREADCONTEXT) +endif(CLR_CMAKE_TARGET_WIN32) + # Features - please keep them alphabetically sorted if(CLR_CMAKE_TARGET_WIN32) if(NOT CLR_CMAKE_TARGET_ARCH_I386) diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index 1ad65fd400b036..3dc4ede42f8081 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -11155,11 +11155,12 @@ void CordbProcess::FilterClrNotification( } } -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) { LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded\n")); +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) HandleHolder hThread = OpenThread( THREAD_GET_CONTEXT | THREAD_SET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME, FALSE, // thread handle is not inheritable. @@ -11302,8 +11303,11 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) LOG((LF_CORDB, LL_INFO10000, "RS HandleSetThreadContextNeeded - Unexpected result from SetThreadContext\n")); ThrowHR(HRESULT_FROM_WIN32(lastError)); } -} +#else + #error Platform not supported #endif +} +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT // // If the thread has an unhandled managed exception, hijack it. @@ -11528,7 +11532,7 @@ HRESULT CordbProcess::Filter( // holder will invoke DeleteIPCEventHelper(pManagedEvent). } -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT else if (dwFirstChance && pRecord->ExceptionCode == STATUS_BREAKPOINT && pRecord->ExceptionAddress == m_runtimeOffsets.m_setThreadContextNeededAddr) { // this is a request to set the thread context out of process diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 0e3d2cba0626f3..bcad293e339bf5 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -3280,7 +3280,7 @@ class CordbProcess : #endif } -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT void HandleSetThreadContextNeeded(DWORD dwThreadId); #endif diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index dee74d440d304a..df05ee034a0d96 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -972,7 +972,7 @@ Debugger::Debugger() // and comment the changes m_mdDataStructureVersion = 1; m_fOutOfProcessSetContextEnabled = -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) && !defined(DACCESS_COMPILE) +#if defined(OUT_OF_PROCESS_SETTHREADCONTEXT) && !defined(DACCESS_COMPILE) Thread::AreCetShadowStacksEnabled() || CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_OutOfProcessSetContext) != 0; #else FALSE; @@ -5558,7 +5558,7 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception, } } -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) && !defined(DACCESS_COMPILE) +#if defined(OUT_OF_PROCESS_SETTHREADCONTEXT) && !defined(DACCESS_COMPILE) if (retVal && fIsVEH) { SendSetThreadContextNeeded(context); @@ -13654,7 +13654,7 @@ LONG Debugger::FirstChanceSuspendHijackWorker(CONTEXT *pContext, if (pFcd->action == HIJACK_ACTION_EXIT_HANDLED) { SPEW(fprintf(stderr, "0x%x D::FCHF: exiting with CONTINUE_EXECUTION\n", tid)); -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) && !defined(DACCESS_COMPILE) +#if defined(OUT_OF_PROCESS_SETTHREADCONTEXT) && !defined(DACCESS_COMPILE) if (fIsVEH) { SendSetThreadContextNeeded(pContext); @@ -16654,7 +16654,7 @@ void Debugger::StartCanaryThread() #endif // DACCESS_COMPILE #ifndef DACCESS_COMPILE -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT void Debugger::SendSetThreadContextNeeded(CONTEXT *context) { STATIC_CONTRACT_NOTHROW; @@ -16663,6 +16663,7 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context) if (!m_fOutOfProcessSetContextEnabled) return; +#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) DWORD contextFlags = context->ContextFlags; DWORD contextSize = 0; @@ -16715,6 +16716,9 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context) { } EX_END_CATCH(SwallowAllExceptions); +#else + #error Platform not supported +#endif LOG((LF_CORDB, LL_INFO10000, "D::SSTCN SetThreadContextNeededFlare returned\n")); _ASSERTE(!"We failed to SetThreadContext from out of process!"); @@ -16727,14 +16731,14 @@ BOOL Debugger::IsOutOfProcessSetContextEnabled() #else void Debugger::SendSetThreadContextNeeded(CONTEXT* context) { - _ASSERTE(!"SendSetThreadContextNeeded is not supported on this platform"); + _ASSERTE(!"SendSetThreadContextNeeded is not enabled on this platform"); } BOOL Debugger::IsOutOfProcessSetContextEnabled() { return FALSE; } -#endif // defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT #endif // DACCESS_COMPILE #endif //DEBUGGING_SUPPORTED diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index f5c55d9abb9bea..695b88aaef1ba4 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -1751,9 +1751,13 @@ extern "C" void __stdcall SignalHijackCompleteFlare(void); extern "C" void __stdcall ExceptionNotForRuntimeFlare(void); 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); +#else +#error Platform not supported #endif +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT /* ------------------------------------------------------------------------ * * Debugger class diff --git a/src/coreclr/debug/ee/rcthread.cpp b/src/coreclr/debug/ee/rcthread.cpp index c074e8080c9c5a..23a61151de4368 100644 --- a/src/coreclr/debug/ee/rcthread.cpp +++ b/src/coreclr/debug/ee/rcthread.cpp @@ -486,8 +486,12 @@ HRESULT DebuggerRCThread::SetupRuntimeOffsets(DebuggerIPCControlBlock * pDebugge pDebuggerRuntimeOffsets->m_debuggerWordTLSIndex = g_debuggerWordTLSIndex; #endif // FEATURE_INTEROP_DEBUGGING -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT +#ifdef TARGET_WINDOWS pDebuggerRuntimeOffsets->m_setThreadContextNeededAddr = (void*) SetThreadContextNeededFlare; +#else + #error Platform not supported +#endif #else pDebuggerRuntimeOffsets->m_setThreadContextNeededAddr = NULL; #endif diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index f7e7c616ef910e..44e2a3512c3986 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -829,20 +829,22 @@ NOINLINE void EditAndContinueModule::FixContextAndResume( // and return because we are potentially writing new vars onto the stack. pCurThread->SetFilterContext( NULL ); -#if defined(TARGET_X86) - ResumeAtJit(pContext, oldSP); -#elif defined(TARGET_WINDOWS) && defined(TARGET_AMD64) +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT if (g_pDebugInterface->IsOutOfProcessSetContextEnabled()) { g_pDebugInterface->SendSetThreadContextNeeded(pContext); } else { - ClrRestoreNonvolatileContext(pContext); - } +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT +#if defined(TARGET_X86) + ResumeAtJit(pContext, oldSP); #else ClrRestoreNonvolatileContext(pContext); #endif +#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT + } +#endif // OUT_OF_PROCESS_SETTHREADCONTEXT // At this point we shouldn't have failed, so this is genuinely erroneous. LOG((LF_ENC, LL_ERROR, "**Error** EnCModule::ResumeInUpdatedFunction returned from ResumeAtJit")); From 24368a9d48dd264473630ee1269dc0f1c5010ebc Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 9 Aug 2022 16:39:52 -0400 Subject: [PATCH 5/5] Add todo comment to merge ICorDebugMutableDataTarget::SetThreadContext with HandleSetThreadContextNeeded implementation --- src/coreclr/debug/di/process.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index 3dc4ede42f8081..465936b2febcb2 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -11230,6 +11230,9 @@ void CordbProcess::HandleSetThreadContextNeeded(DWORD dwThreadId) 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 + // The initialize call should fail but return contextSize contextSize = 0; DWORD contextFlags = pContext->ContextFlags;