[Lldb-commits] [lldb] 5bbe63e - fix: Target Process may crash or freezes on detaching process on windows (#115712)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Nov 15 01:52:40 PST 2024
Author: anatawa12
Date: 2024-11-15T10:52:36+01:00
New Revision: 5bbe63ec91226c0026c6f1ed726c45bb117544e0
URL: https://github.com/llvm/llvm-project/commit/5bbe63ec91226c0026c6f1ed726c45bb117544e0
DIFF: https://github.com/llvm/llvm-project/commit/5bbe63ec91226c0026c6f1ed726c45bb117544e0.diff
LOG: fix: Target Process may crash or freezes on detaching process on windows (#115712)
Fixes #67825 Fixes #89077
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.
Added:
Modified:
lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
lldb/source/Plugins/Process/Windows/Common/DebuggerThread.h
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
index ca8e9c078e1f99..b637238007b882 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,45 @@ 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());
+
+ // 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;
+ }
+ }
+
if (m_detached) {
should_debug = false;
}
@@ -310,25 +350,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,
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);
diff --git a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
index 797db55a45b9b7..57727294ddc3d3 100644
--- a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
+++ b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
@@ -12,7 +12,6 @@
class DetachResumesTestCase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
- @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr89077")
def test_detach_resumes(self):
self.build()
exe = self.getBuildArtifact()
More information about the lldb-commits
mailing list