Skip to content

Commit

Permalink
[lldb/linux] Fix a bug in wait status handling
Browse files Browse the repository at this point in the history
The MonitorCallback function was assuming that the "exited" argument is
set whenever a thread exits, but the caller was only setting that flag
for the main thread.

This patch deletes the argument altogether, and lets MonitorCallback
compute what it needs itself.

This is almost NFC, since previously we would end up in the
"GetSignalInfo failed for unknown reasons" branch, which was doing the
same thing -- forgetting about the thread.
  • Loading branch information
labath committed Dec 29, 2021
1 parent 633b002 commit fdd741d
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 17 deletions.
22 changes: 8 additions & 14 deletions lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,16 +426,15 @@ Status NativeProcessLinux::SetDefaultPtraceOpts(lldb::pid_t pid) {
}

// Handles all waitpid events from the inferior process.
void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, bool exited,
WaitStatus status) {
void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, WaitStatus status) {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));

// Certain activities differ based on whether the pid is the tid of the main
// thread.
const bool is_main_thread = (pid == GetID());

// Handle when the thread exits.
if (exited) {
if (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal) {
LLDB_LOG(log,
"got exit status({0}) , tid = {1} ({2} main thread), process "
"state = {3}",
Expand Down Expand Up @@ -485,7 +484,7 @@ void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, bool exited,
if (info.si_signo == SIGTRAP)
MonitorSIGTRAP(info, *thread_sp);
else
MonitorSignal(info, *thread_sp, exited);
MonitorSignal(info, *thread_sp);
} else {
if (info_err.GetError() == EINVAL) {
// This is a group stop reception for this tid. We can reach here if we
Expand Down Expand Up @@ -753,7 +752,7 @@ void NativeProcessLinux::MonitorSIGTRAP(const siginfo_t &info,
default:
LLDB_LOG(log, "received unknown SIGTRAP stop event ({0}, pid {1} tid {2}",
info.si_code, GetID(), thread.GetID());
MonitorSignal(info, thread, false);
MonitorSignal(info, thread);
break;
}
}
Expand Down Expand Up @@ -801,7 +800,7 @@ void NativeProcessLinux::MonitorWatchpoint(NativeThreadLinux &thread,
}

void NativeProcessLinux::MonitorSignal(const siginfo_t &info,
NativeThreadLinux &thread, bool exited) {
NativeThreadLinux &thread) {
const int signo = info.si_signo;
const bool is_from_llgs = info.si_pid == getpid();

Expand Down Expand Up @@ -1962,16 +1961,11 @@ void NativeProcessLinux::SigchldHandler() {
}

WaitStatus wait_status = WaitStatus::Decode(status);
bool exited = wait_status.type == WaitStatus::Exit ||
(wait_status.type == WaitStatus::Signal &&
wait_pid == static_cast<::pid_t>(GetID()));

LLDB_LOG(
log,
"waitpid (-1, &status, _) => pid = {0}, status = {1}, exited = {2}",
wait_pid, wait_status, exited);
LLDB_LOG(log, "waitpid (-1, &status, _) => pid = {0}, status = {1}",
wait_pid, wait_status);

MonitorCallback(wait_pid, exited, wait_status);
MonitorCallback(wait_pid, wait_status);
}
}

Expand Down
5 changes: 2 additions & 3 deletions lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class NativeProcessLinux : public NativeProcessELF,

static Status SetDefaultPtraceOpts(const lldb::pid_t);

void MonitorCallback(lldb::pid_t pid, bool exited, WaitStatus status);
void MonitorCallback(lldb::pid_t pid, WaitStatus status);

void WaitForCloneNotification(::pid_t pid);

Expand All @@ -176,8 +176,7 @@ class NativeProcessLinux : public NativeProcessELF,

void MonitorWatchpoint(NativeThreadLinux &thread, uint32_t wp_index);

void MonitorSignal(const siginfo_t &info, NativeThreadLinux &thread,
bool exited);
void MonitorSignal(const siginfo_t &info, NativeThreadLinux &thread);

bool HasThreadNoLock(lldb::tid_t thread_id);

Expand Down

0 comments on commit fdd741d

Please sign in to comment.