[Lldb-commits] [lldb] [lldb][Windows] Fix races in the DebuggerThread exception handshake (PR #213075)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 30 10:01:40 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Charles Zablit (charles-zablit)
<details>
<summary>Changes</summary>
`DebuggerThread::m_active_exception` and `m_exception_pred` are accessed both from the Windows debug-event loop and from the thread driving the debugger. There is no sync mechanism between the two. That caused two distinct failures.
1. Use after free: The `m_active_exception.reset()` in `ContinueAsyncException()` can destroy the exception while the delegate uses it.
2. `ContinueAsyncException()` can be called between the end of the delegate and `SetValue(result)`. Causing `WaitForValueNotEqualTo(BreakInDebugger) to spin forever`.
This patch guards `m_active_exception` with a mutex and returns a `ExceptionRecordSP` from `GetActiveException()` so callers no longer have to lock a weak_ptr.
To verify this, I ran the test suite and injected a 50ms delay into the window between the delegate returning and the `SetValue()`. This reproduces the packet timeout in `TestGdbRemoteExitCode` deterministically before this change, and all lldb-server tests pass with the same delay after it.
---
Full diff: https://github.com/llvm/llvm-project/pull/213075.diff
4 Files Affected:
- (modified) lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp (+25-8)
- (modified) lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h (+7-4)
- (modified) lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp (+1-1)
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (+3-4)
``````````diff
diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
index 228efe18e81d1..ade798baadb65 100644
--- a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
@@ -203,7 +203,7 @@ Status DebuggerThread::StopDebugging(bool terminate) {
// breakpoint messing around in the debugger), continue it now. But only
// AFTER calling TerminateProcess to make sure that the very next call to
// WaitForDebugEventEx is an exit process event.
- if (m_active_exception.get()) {
+ if (GetActiveException()) {
LLDB_LOG(log, "masking active exception");
ContinueAsyncException(ExceptionResult::MaskException);
}
@@ -239,15 +239,23 @@ Status DebuggerThread::StopDebugging(bool terminate) {
return error;
}
+ExceptionRecordSP DebuggerThread::GetActiveException() {
+ std::lock_guard<std::mutex> guard(m_active_exception_mutex);
+ return m_active_exception;
+}
+
void DebuggerThread::ContinueAsyncException(ExceptionResult result) {
- if (!m_active_exception.get())
- return;
+ {
+ std::lock_guard<std::mutex> guard(m_active_exception_mutex);
+ if (!m_active_exception)
+ return;
+ m_active_exception.reset();
+ }
Log *log = GetLog(WindowsLog::Process | WindowsLog::Exception);
LLDB_LOG(log, "broadcasting for inferior process {0}.",
m_process.GetProcessId());
- m_active_exception.reset();
m_exception_pred.SetValue(result, eBroadcastAlways);
}
@@ -401,15 +409,24 @@ DebuggerThread::HandleExceptionEvent(const EXCEPTION_DEBUG_INFO &info,
bool first_chance = (info.dwFirstChance != 0);
- m_active_exception.reset(
- new ExceptionRecord(info.ExceptionRecord, thread_id));
+ ExceptionRecordSP active_exception =
+ std::make_shared<ExceptionRecord>(info.ExceptionRecord, thread_id);
+ {
+ std::lock_guard<std::mutex> guard(m_active_exception_mutex);
+ m_active_exception = active_exception;
+ }
+ m_exception_pred.SetValue(ExceptionResult::BreakInDebugger, eBroadcastNever);
+
LLDB_LOG(log, "encountered {0} chance exception {1:x} on thread {2:x}",
first_chance ? "first" : "second",
info.ExceptionRecord.ExceptionCode, thread_id);
ExceptionResult result =
- m_debug_delegate->OnDebugException(first_chance, *m_active_exception);
- m_exception_pred.SetValue(result, eBroadcastNever);
+ m_debug_delegate->OnDebugException(first_chance, *active_exception);
+ // If the delegate dealt with the exception itself, continue it now. This is
+ // a no-op if the other thread got there first, in which case its result wins.
+ if (result != ExceptionResult::BreakInDebugger)
+ ContinueAsyncException(result);
LLDB_LOG(log, "waiting for ExceptionPred != BreakInDebugger");
result = *m_exception_pred.WaitForValueNotEqualTo(
diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h
index 70204e2f9e5eb..03202264b01ee 100644
--- a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h
+++ b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h
@@ -11,6 +11,7 @@
#include <atomic>
#include <memory>
+#include <mutex>
#include "ForwardDecl.h"
#include "lldb/Host/HostProcess.h"
@@ -34,9 +35,10 @@ class DebuggerThread : public std::enable_shared_from_this<DebuggerThread> {
HostProcess GetProcess() const { return m_process; }
HostThread GetMainThread() const { return m_main_thread; }
- std::weak_ptr<ExceptionRecord> GetActiveException() {
- return m_active_exception;
- }
+
+ /// Returns the exception the debug loop is currently reporting, or null if
+ /// there is none. Safe to call from any thread.
+ ExceptionRecordSP GetActiveException();
Status StopDebugging(bool terminate);
@@ -74,8 +76,9 @@ class DebuggerThread : public std::enable_shared_from_this<DebuggerThread> {
// The image file of the process being debugged.
HANDLE m_image_file = nullptr;
- // The current exception waiting to be handled
+ // The current exception waiting to be handled.
ExceptionRecordSP m_active_exception;
+ std::mutex m_active_exception_mutex;
// A predicate which gets signalled when an exception is finished processing
// and the debug loop can be continued.
diff --git a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
index f87fd23f5a047..17dd05af35e1b 100644
--- a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -152,7 +152,7 @@ Status NativeProcessWindows::Resume(const ResumeActionList &resume_actions) {
// Resume the debug loop.
ExceptionRecordSP active_exception =
- m_session_data->m_debugger->GetActiveException().lock();
+ m_session_data->m_debugger->GetActiveException();
if (active_exception) {
// Resume the process and continue processing debug events. Mask the
// exception so that from the process's view, there is no indication that
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index 561710ccec3c8..289223fb5bb67 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -278,7 +278,7 @@ Status ProcessWindows::DoResume(RunDirection direction) {
}
ExceptionRecordSP active_exception =
- m_session_data->m_debugger->GetActiveException().lock();
+ m_session_data->m_debugger->GetActiveException();
if (active_exception) {
// Resume the process and continue processing debug events. Mask the
// exception so that from the process's view, there is no indication that
@@ -337,9 +337,8 @@ void ProcessWindows::RefreshStateAfterStop() {
m_thread_list.RefreshStateAfterStop();
- std::weak_ptr<ExceptionRecord> exception_record =
+ ExceptionRecordSP active_exception =
m_session_data->m_debugger->GetActiveException();
- ExceptionRecordSP active_exception = exception_record.lock();
if (!active_exception) {
LLDB_LOG(log,
"there is no active exception in process {0}. Why is the "
@@ -901,7 +900,7 @@ std::optional<uint32_t> ProcessWindows::GetWatchpointSlotCount() {
std::optional<DWORD> ProcessWindows::GetActiveExceptionCode() const {
if (!m_session_data || !m_session_data->m_debugger)
return std::nullopt;
- auto exc = m_session_data->m_debugger->GetActiveException().lock();
+ auto exc = m_session_data->m_debugger->GetActiveException();
if (!exc)
return std::nullopt;
return exc->GetExceptionValue();
``````````
</details>
https://github.com/llvm/llvm-project/pull/213075
More information about the lldb-commits
mailing list