[Lldb-commits] [lldb] fix: Target Process may crash on detaching process on windows (PR #115712)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 12 21:09:22 PST 2024
https://github.com/anatawa12 updated https://github.com/llvm/llvm-project/pull/115712
>From cabe26fc14cb4a924b2aebe6f3dd080408fed6ce Mon Sep 17 00:00:00 2001
From: anatawa12 <anatawa12 at icloud.com>
Date: Mon, 11 Nov 2024 20:22:33 +0900
Subject: [PATCH 1/3] fix: Target Process may crash on detaching process
---
.../Process/Windows/Common/DebuggerThread.cpp | 45 ++++++++++++-------
.../Process/Windows/Common/DebuggerThread.h | 2 +-
2 files changed, 30 insertions(+), 17 deletions(-)
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,
>From 2138286671d92f3959eb0d05697f3b1cb4a6a232 Mon Sep 17 00:00:00 2001
From: anatawa12 <anatawa12 at icloud.com>
Date: Wed, 13 Nov 2024 14:04:01 +0900
Subject: [PATCH 2/3] chore: resume threads before detach
---
.../Process/Windows/Common/ProcessWindows.cpp | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index 703aa082f0476f..1bdacec221695e 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -169,6 +169,38 @@ Status ProcessWindows::DoDetach(bool keep_stopped) {
Log *log = GetLog(WindowsLog::Process);
StateType private_state = GetPrivateState();
if (private_state != eStateExited && private_state != eStateDetached) {
+ if (!keep_stopped) {
+ // if the thread is suspended by lldb, we have to resume threads before
+ // detaching process. When we do after DetachProcess(), thread handles
+ // become invalid so we do before detach.
+ if (private_state == eStateStopped || private_state == eStateCrashed) {
+ LLDB_LOG(log, "process {0} is in state {1}. Resuming for detach...",
+ m_session_data->m_debugger->GetProcess().GetProcessId(),
+ GetPrivateState());
+
+ LLDB_LOG(log, "resuming {0} threads for detach.",
+ m_thread_list.GetSize());
+
+ bool failed = false;
+ for (uint32_t i = 0; i < m_thread_list.GetSize(); ++i) {
+ auto thread = std::static_pointer_cast<TargetThreadWindows>(
+ m_thread_list.GetThreadAtIndex(i));
+ Status result = thread->DoResume();
+ if (result.Fail()) {
+ failed = true;
+ LLDB_LOG(log,
+ "Trying to resume thread at index {0}, but failed with "
+ "error {1}.",
+ i, result);
+ }
+ }
+
+ if (failed) {
+ error = Status::FromErrorString("Resuming Threads for Detach failed");
+ }
+ }
+ }
+
error = DetachProcess();
if (error.Success())
SetPrivateState(eStateDetached);
>From 5ad1a98a8b07e1d75829870995fbf36fb8cb99ed Mon Sep 17 00:00:00 2001
From: anatawa12 <anatawa12 at icloud.com>
Date: Wed, 13 Nov 2024 14:01:30 +0900
Subject: [PATCH 3/3] chore: process debugger events as possible
---
.../Process/Windows/Common/DebuggerThread.cpp | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
index ac63997abe823d..b637238007b882 100644
--- a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
@@ -307,6 +307,26 @@ void DebuggerThread::DebugLoop() {
LLDB_LOG(log,
"Breakpoint exception is cue to detach from process {0:x}",
m_pid_to_detach.load());
+
+ // detaching with leaving breakpoint exception event on the queue may
+ // cause target process to crash so process events as possible since
+ // target threads are running at this time, there is possibility to
+ // have some breakpoint exception between last WaitForDebugEvent and
+ // DebugActiveProcessStop but ignore for now.
+ while (WaitForDebugEvent(&dbe, 0)) {
+ continue_status = DBG_CONTINUE;
+ if (dbe.dwDebugEventCode == EXCEPTION_DEBUG_EVENT &&
+ !(dbe.u.Exception.ExceptionRecord.ExceptionCode ==
+ EXCEPTION_BREAKPOINT ||
+ dbe.u.Exception.ExceptionRecord.ExceptionCode ==
+ STATUS_WX86_BREAKPOINT ||
+ dbe.u.Exception.ExceptionRecord.ExceptionCode ==
+ EXCEPTION_SINGLE_STEP))
+ continue_status = DBG_EXCEPTION_NOT_HANDLED;
+ ::ContinueDebugEvent(dbe.dwProcessId, dbe.dwThreadId,
+ continue_status);
+ }
+
::DebugActiveProcessStop(m_pid_to_detach);
m_detached = true;
}
More information about the lldb-commits
mailing list