Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Target Process may crash or freezes on detaching process on windows #115712

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 49 additions & 16 deletions lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 32 additions & 0 deletions lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that if the thread/process gets another exception before we manage to detach from it, it will remain suspended? I'm not familiar with windows debugging API, so I don't know if there's a way to handle this, but I believe this is the reason why linux (and others) have a PTRACE_DETACH call which (atomically) detaches from a process/thread and resumes it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that if the thread/process gets another exception before we manage to detach from it, it will remain suspended?

No, the process will not suspend.
The process will receive the exception and the process handles it instead.
For simple programs, it will crash the process but some process may handle in their way.
We can see this behavior with TestDetachResumes by reverting 5ad1a98.

I'm not familiar with windows debugging API, so I don't know if there's a way to handle this, but I believe this is the reason why linux (and others) have a PTRACE_DETACH call which (atomically) detaches from a process/thread and resumes it.

I agree with that but as far as I researched I could not figure out the API to detach and resume thread simultaneously.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the process will not suspend.
The process will receive the exception and the process handles it instead.

Ok, just so we're clear: If the exception happens after the last call to WaitForDebugEvent (but before the call to DebugActiveProcessStop), the process will not stay stopped, but it will receive the exception (and then either handle it or get killed by it). True or false?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's True, (as far as I know)

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading