[Lldb-commits] [lldb] 4384c96 - [lldb/linux] Handle main thread exits

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 5 02:23:14 PDT 2022


Author: Pavel Labath
Date: 2022-04-05T11:22:37+02:00
New Revision: 4384c96fe7eb5ceb5f2c1ece4a73c22a18bac19f

URL: https://github.com/llvm/llvm-project/commit/4384c96fe7eb5ceb5f2c1ece4a73c22a18bac19f
DIFF: https://github.com/llvm/llvm-project/commit/4384c96fe7eb5ceb5f2c1ece4a73c22a18bac19f.diff

LOG: [lldb/linux] Handle main thread exits

This patch handles the situation where the main thread exits (through
the SYS_exit syscall). In this case, the process as a whole continues
running until all of the other threads exit, or one of them issues an
exit_group syscall.

The patch consists of two changes:
- a moderate redesign of the handling of thread exit (WIFEXITED) events.
  Previously, we were removing (forgetting) a thread once we received
  the WIFEXITED (or WIFSIGNALED) event. This was problematic for the
  main thread, since the main thread WIFEXITED event (which is better thought
  of as a process-wide event) gets reported only after the entire process
  exits. This resulted in deadlocks, where we were waiting for the
  process to stop (because we still considered the main thread "live").

  This patch changes the logic such that the main thread is removed as
  soon as its PTRACE_EVENT_EXIT (the pre-exit) event is received. At
  this point we can consider the thread gone (for most purposes). As a
  corrolary, I needed to add special logic to catch process-wide exit
  events in the cases where we don't have the main thread around.

- The second part of the patch is the removal of the assumptions that
  the main thread is always available. This generally meant replacing
  the uses of GetThreadByID(process_id) with GetCurrentThread() in
  various process-wide operations (such as memory reads).

Differential Revision: https://reviews.llvm.org/D122716

Added: 
    lldb/test/API/functionalities/thread/main_thread_exit/Makefile
    lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
    lldb/test/API/functionalities/thread/main_thread_exit/main.cpp

Modified: 
    lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 4bc5711453939..3d8715050ae0a 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -447,13 +447,7 @@ void NativeProcessLinux::MonitorCallback(NativeThreadLinux &thread,
     // This is a thread that exited.  Ensure we're not tracking it anymore.
     StopTrackingThread(thread);
 
-    if (is_main_thread) {
-      // The main thread exited.  We're done monitoring.  Report to delegate.
-      SetExitStatus(status, true);
-
-      // Notify delegate that our process has exited.
-      SetState(StateType::eStateExited, true);
-    }
+    assert(!is_main_thread && "Main thread exits handled elsewhere");
     return;
   }
 
@@ -611,6 +605,13 @@ void NativeProcessLinux::MonitorSIGTRAP(const siginfo_t &info,
     }
     ResumeThread(thread, state, LLDB_INVALID_SIGNAL_NUMBER);
 
+    if (is_main_thread) {
+      // Main thread report the read (WIFEXITED) event only after all threads in
+      // the process exit, so we need to stop tracking it here instead of in
+      // MonitorCallback
+      StopTrackingThread(thread);
+    }
+
     break;
   }
 
@@ -1177,11 +1178,11 @@ Status NativeProcessLinux::PopulateMemoryRegionCache() {
 
   // Linux kernel since 2.6.14 has /proc/{pid}/smaps
   // if CONFIG_PROC_PAGE_MONITOR is enabled
-  auto BufferOrError = getProcFile(GetID(), "smaps");
+  auto BufferOrError = getProcFile(GetID(), GetCurrentThreadID(), "smaps");
   if (BufferOrError)
     ParseLinuxSMapRegions(BufferOrError.get()->getBuffer(), callback);
   else {
-    BufferOrError = getProcFile(GetID(), "maps");
+    BufferOrError = getProcFile(GetID(), GetCurrentThreadID(), "maps");
     if (!BufferOrError) {
       m_supports_mem_region = LazyBool::eLazyBoolNo;
       return BufferOrError.getError();
@@ -1232,7 +1233,7 @@ NativeProcessLinux::Syscall(llvm::ArrayRef<uint64_t> args) {
 
   addr_t exe_addr = region_it->first.GetRange().GetRangeBase();
 
-  NativeThreadLinux &thread = *GetThreadByID(GetID());
+  NativeThreadLinux &thread = *GetCurrentThread();
   assert(thread.GetState() == eStateStopped);
   NativeRegisterContextLinux &reg_ctx = thread.GetRegisterContext();
 
@@ -1384,8 +1385,9 @@ Status NativeProcessLinux::ReadMemoryTags(int32_t type, lldb::addr_t addr,
     tags_iovec.iov_len = num_tags;
 
     Status error = NativeProcessLinux::PtraceWrapper(
-        details->ptrace_read_req, GetID(), reinterpret_cast<void *>(read_addr),
-        static_cast<void *>(&tags_iovec), 0, nullptr);
+        details->ptrace_read_req, GetCurrentThreadID(),
+        reinterpret_cast<void *>(read_addr), static_cast<void *>(&tags_iovec),
+        0, nullptr);
 
     if (error.Fail()) {
       // Discard partial reads
@@ -1453,7 +1455,7 @@ Status NativeProcessLinux::WriteMemoryTags(int32_t type, lldb::addr_t addr,
     tags_vec.iov_len = num_tags;
 
     Status error = NativeProcessLinux::PtraceWrapper(
-        details->ptrace_write_req, GetID(),
+        details->ptrace_write_req, GetCurrentThreadID(),
         reinterpret_cast<void *>(write_addr), static_cast<void *>(&tags_vec), 0,
         nullptr);
 
@@ -1525,15 +1527,14 @@ Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
     // The process_vm_readv path is about 50 times faster than ptrace api. We
     // want to use this syscall if it is supported.
 
-    const ::pid_t pid = GetID();
-
     struct iovec local_iov, remote_iov;
     local_iov.iov_base = buf;
     local_iov.iov_len = size;
     remote_iov.iov_base = reinterpret_cast<void *>(addr);
     remote_iov.iov_len = size;
 
-    bytes_read = process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
+    bytes_read = process_vm_readv(GetCurrentThreadID(), &local_iov, 1,
+                                  &remote_iov, 1, 0);
     const bool success = bytes_read == size;
 
     Log *log = GetLog(POSIXLog::Process);
@@ -1557,7 +1558,7 @@ Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
 
   for (bytes_read = 0; bytes_read < size; bytes_read += remainder) {
     Status error = NativeProcessLinux::PtraceWrapper(
-        PTRACE_PEEKDATA, GetID(), (void *)addr, nullptr, 0, &data);
+        PTRACE_PEEKDATA, GetCurrentThreadID(), (void *)addr, nullptr, 0, &data);
     if (error.Fail())
       return error;
 
@@ -1592,8 +1593,8 @@ Status NativeProcessLinux::WriteMemory(lldb::addr_t addr, const void *buf,
       memcpy(&data, src, k_ptrace_word_size);
 
       LLDB_LOG(log, "[{0:x}]:{1:x}", addr, data);
-      error = NativeProcessLinux::PtraceWrapper(PTRACE_POKEDATA, GetID(),
-                                                (void *)addr, (void *)data);
+      error = NativeProcessLinux::PtraceWrapper(
+          PTRACE_POKEDATA, GetCurrentThreadID(), (void *)addr, (void *)data);
       if (error.Fail())
         return error;
     } else {
@@ -1857,46 +1858,64 @@ void NativeProcessLinux::ThreadWasCreated(NativeThreadLinux &thread) {
   }
 }
 
+static llvm::Optional<WaitStatus> HandlePid(::pid_t pid) {
+  Log *log = GetLog(POSIXLog::Process);
+
+  int status;
+  ::pid_t wait_pid = llvm::sys::RetryAfterSignal(
+      -1, ::waitpid, pid, &status, __WALL | __WNOTHREAD | WNOHANG);
+
+  if (wait_pid == 0)
+    return llvm::None;
+
+  if (wait_pid == -1) {
+    Status error(errno, eErrorTypePOSIX);
+    LLDB_LOG(log, "waitpid({0}, &status, _) failed: {1}", pid,
+             error);
+    return llvm::None;
+  }
+
+  assert(wait_pid == pid);
+
+  WaitStatus wait_status = WaitStatus::Decode(status);
+
+  LLDB_LOG(log, "waitpid({0})  got status = {1}", pid, wait_status);
+  return wait_status;
+}
+
 void NativeProcessLinux::SigchldHandler() {
   Log *log = GetLog(POSIXLog::Process);
 
   // Threads can appear or disappear as a result of event processing, so gather
   // the events upfront.
   llvm::DenseMap<lldb::tid_t, WaitStatus> tid_events;
+  bool checked_main_thread = false;
   for (const auto &thread_up : m_threads) {
-    int status = -1;
-    ::pid_t wait_pid =
-        llvm::sys::RetryAfterSignal(-1, ::waitpid, thread_up->GetID(), &status,
-                                    __WALL | __WNOTHREAD | WNOHANG);
-
-    if (wait_pid == 0)
-      continue; // Nothing to do for this thread.
-
-    if (wait_pid == -1) {
-      Status error(errno, eErrorTypePOSIX);
-      LLDB_LOG(log, "waitpid({0}, &status, _) failed: {1}", thread_up->GetID(),
-               error);
-      continue;
-    }
+    if (thread_up->GetID() == GetID())
+      checked_main_thread = true;
 
-    assert(wait_pid == static_cast<::pid_t>(thread_up->GetID()));
-
-    WaitStatus wait_status = WaitStatus::Decode(status);
-
-    LLDB_LOG(log, "waitpid({0})  got status = {1}", thread_up->GetID(),
-             wait_status);
-    tid_events.try_emplace(thread_up->GetID(), wait_status);
+    if (llvm::Optional<WaitStatus> status = HandlePid(thread_up->GetID()))
+      tid_events.try_emplace(thread_up->GetID(), *status);
+  }
+  // Check the main thread even when we're not tracking it as process exit
+  // events are reported that way.
+  if (!checked_main_thread) {
+    if (llvm::Optional<WaitStatus> status = HandlePid(GetID()))
+      tid_events.try_emplace(GetID(), *status);
   }
 
   for (auto &KV : tid_events) {
     LLDB_LOG(log, "processing {0}({1}) ...", KV.first, KV.second);
-    NativeThreadLinux *thread = GetThreadByID(KV.first);
-    if (thread) {
-      MonitorCallback(*thread, KV.second);
-    } else {
-      // This can happen if one of the events is an main thread exit.
-      LLDB_LOG(log, "... but the thread has disappeared");
+    if (KV.first == GetID() && (KV.second.type == WaitStatus::Exit ||
+                                KV.second.type == WaitStatus::Signal)) {
+
+      // The process exited.  We're done monitoring.  Report to delegate.
+      SetExitStatus(KV.second, true);
+      return;
     }
+    NativeThreadLinux *thread = GetThreadByID(KV.first);
+    assert(thread && "Why did this thread disappear?");
+    MonitorCallback(*thread, KV.second);
   }
 }
 

diff  --git a/lldb/test/API/functionalities/thread/main_thread_exit/Makefile b/lldb/test/API/functionalities/thread/main_thread_exit/Makefile
new file mode 100644
index 0000000000000..ebecfbf92416a
--- /dev/null
+++ b/lldb/test/API/functionalities/thread/main_thread_exit/Makefile
@@ -0,0 +1,3 @@
+ENABLE_THREADS := YES
+CXX_SOURCES := main.cpp
+include Makefile.rules

diff  --git a/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py b/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
new file mode 100644
index 0000000000000..adee1afb83928
--- /dev/null
+++ b/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
@@ -0,0 +1,31 @@
+"""
+Test handling of the situation where the main thread exits but the other threads
+in the process keep running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class ThreadExitTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # Needs os-specific implementation in the inferior
+    @skipIf(oslist=no_match(["linux"]))
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here",
+                lldb.SBFileSpec("main.cpp"))
+
+        # There should be one (non-main) thread left
+        self.assertEquals(self.process().GetNumThreads(), 1)
+
+        # Ensure we can evaluate_expressions in this state
+        self.expect_expr("call_me()", result_value="12345")
+
+        self.runCmd("continue")
+        self.assertEquals(self.process().GetExitStatus(), 47)

diff  --git a/lldb/test/API/functionalities/thread/main_thread_exit/main.cpp b/lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
new file mode 100644
index 0000000000000..f1589be2913b2
--- /dev/null
+++ b/lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
@@ -0,0 +1,23 @@
+#include <thread>
+
+#ifdef __linux__
+#include <sys/syscall.h>
+#include <unistd.h>
+
+void exit_thread(int result) { syscall(SYS_exit, result); }
+#else
+#error Needs OS-specific implementation
+#endif
+
+int call_me() { return 12345; }
+
+void thread() {
+  std::this_thread::sleep_for(
+      std::chrono::seconds(10)); // Let the main thread exit.
+  exit_thread(42);               // break here
+}
+
+int main() {
+  std::thread(thread).detach();
+  exit_thread(47);
+}


        


More information about the lldb-commits mailing list