llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: anatawa12 (anatawa12) <details> <summary>Changes</summary> I hope this fixes #<!-- -->67825 Fixes [RIDER-99436](https://youtrack.jetbrains.com/issue/RIDER-99436/Unity-Editor-will-be-crashed-when-detaching-LLDB-debugger-in-Rider), which is upstream issue of #<!-- -->67825. This PR changes the timing of calling `DebugActiveProcessStop` to after calling `ContinueDebugEvent` for last debugger exception. I confirmed the crashing behavior is because we call `DebugActiveProcessStop` before `ContinueDebugEvent` for last debugger exception with https://github.com/anatawa12/debug-api-test. I have no idea how should I test this so I did not add tests. Could you let me know how should I test for this chages? --- Full diff: https://github.com/llvm/llvm-project/pull/115712.diff 2 Files Affected: - (modified) lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp (+29-16) - (modified) lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h (+1-1) ``````````diff diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp index ca8e9c078e1f99..ac63997abe823d 100644 --- a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp @@ -239,12 +239,13 @@ void DebuggerThread::DebugLoop() { BOOL wait_result = WaitForDebugEvent(&dbe, INFINITE); if (wait_result) { DWORD continue_status = DBG_CONTINUE; + bool shutting_down = m_is_shutting_down; switch (dbe.dwDebugEventCode) { default: llvm_unreachable("Unhandle debug event code!"); case EXCEPTION_DEBUG_EVENT: { - ExceptionResult status = - HandleExceptionEvent(dbe.u.Exception, dbe.dwThreadId); + ExceptionResult status = HandleExceptionEvent( + dbe.u.Exception, dbe.dwThreadId, shutting_down); if (status == ExceptionResult::MaskException) continue_status = DBG_CONTINUE; @@ -292,6 +293,25 @@ void DebuggerThread::DebugLoop() { ::ContinueDebugEvent(dbe.dwProcessId, dbe.dwThreadId, continue_status); + // We have to DebugActiveProcessStop after ContinueDebugEvent, otherwise + // the target process will crash + if (shutting_down) { + // A breakpoint that occurs while `m_pid_to_detach` is non-zero is a + // magic exception that we use simply to wake up the DebuggerThread so + // that we can close out the debug loop. + if (m_pid_to_detach != 0 && + (dbe.u.Exception.ExceptionRecord.ExceptionCode == + EXCEPTION_BREAKPOINT || + dbe.u.Exception.ExceptionRecord.ExceptionCode == + STATUS_WX86_BREAKPOINT)) { + LLDB_LOG(log, + "Breakpoint exception is cue to detach from process {0:x}", + m_pid_to_detach.load()); + ::DebugActiveProcessStop(m_pid_to_detach); + m_detached = true; + } + } + if (m_detached) { should_debug = false; } @@ -310,25 +330,18 @@ void DebuggerThread::DebugLoop() { ExceptionResult DebuggerThread::HandleExceptionEvent(const EXCEPTION_DEBUG_INFO &info, - DWORD thread_id) { + DWORD thread_id, bool shutting_down) { Log *log = GetLog(WindowsLog::Event | WindowsLog::Exception); - if (m_is_shutting_down) { - // A breakpoint that occurs while `m_pid_to_detach` is non-zero is a magic - // exception that - // we use simply to wake up the DebuggerThread so that we can close out the - // debug loop. - if (m_pid_to_detach != 0 && + if (shutting_down) { + bool is_breakpoint = (info.ExceptionRecord.ExceptionCode == EXCEPTION_BREAKPOINT || - info.ExceptionRecord.ExceptionCode == STATUS_WX86_BREAKPOINT)) { - LLDB_LOG(log, "Breakpoint exception is cue to detach from process {0:x}", - m_pid_to_detach.load()); - ::DebugActiveProcessStop(m_pid_to_detach); - m_detached = true; - } + info.ExceptionRecord.ExceptionCode == STATUS_WX86_BREAKPOINT); // Don't perform any blocking operations while we're shutting down. That // will cause TerminateProcess -> WaitForSingleObject to time out. - return ExceptionResult::SendToApplication; + // We should not send breakpoint exceptions to the application. + return is_breakpoint ? ExceptionResult::MaskException + : ExceptionResult::SendToApplication; } bool first_chance = (info.dwFirstChance != 0); diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h index e3439ff34584be..5960237b778673 100644 --- a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h +++ b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h @@ -46,7 +46,7 @@ class DebuggerThread : public std::enable_shared_from_this<DebuggerThread> { void FreeProcessHandles(); void DebugLoop(); ExceptionResult HandleExceptionEvent(const EXCEPTION_DEBUG_INFO &info, - DWORD thread_id); + DWORD thread_id, bool shutting_down); DWORD HandleCreateThreadEvent(const CREATE_THREAD_DEBUG_INFO &info, DWORD thread_id); DWORD HandleCreateProcessEvent(const CREATE_PROCESS_DEBUG_INFO &info, `````````` </details> https://github.com/llvm/llvm-project/pull/115712 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
