[Lldb-commits] [lldb] [lldb][Windows] Fix races in the DebuggerThread exception handshake (PR #213075)

[Charles Zablit via lldb-commits]

https://github.com/charles-zablit created https://github.com/llvm/llvm-project/pull/213075

`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.

>From b893900632ac06bc3b989ae2a876e237260ec677 Mon Sep 17 00:00:00 2001
From: Charles Zablit <c_zablit at apple.com>
Date: Thu, 30 Jul 2026 17:54:02 +0100
Subject: [PATCH] [lldb][Windows] Fix races in the DebuggerThread exception
 handshake

---
 .../Process/Windows/Common/DebuggerThread.cpp | 33 ++++++++++++++-----
 .../Process/Windows/Common/DebuggerThread.h   | 11 ++++---
 .../Windows/Common/NativeProcessWindows.cpp   |  2 +-
 .../Process/Windows/Common/ProcessWindows.cpp |  7 ++--
 4 files changed, 36 insertions(+), 17 deletions(-)

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();