[Lldb-commits] [lldb] 63d7564 - Reland "[lldb] [Process] Watch for fork/vfork notifications" for FreeBSD

Michał Górny via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 13 04:19:51 PDT 2021


Author: Michał Górny
Date: 2021-04-13T13:19:42+02:00
New Revision: 63d75641054afb0fe43928e9430a2bb92f09e121

URL: https://github.com/llvm/llvm-project/commit/63d75641054afb0fe43928e9430a2bb92f09e121
DIFF: https://github.com/llvm/llvm-project/commit/63d75641054afb0fe43928e9430a2bb92f09e121.diff

LOG: Reland "[lldb] [Process] Watch for fork/vfork notifications" for FreeBSD

The original commit was reverted because of the problems it introduced
on Linux.  However, FreeBSD should not be affected, so restore that part
and we will address Linux separately.

While at it, remove the dbreg hack as the underlying issue has been
fixed in the FreeBSD kernel and the problem is unlikely to happen
in real life use anyway.

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

Added: 
    lldb/test/Shell/Subprocess/Inputs/fork.cpp
    lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
    lldb/test/Shell/Subprocess/fork-follow-parent.test
    lldb/test/Shell/Subprocess/lit.local.cfg
    lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
    lldb/test/Shell/Subprocess/vfork-follow-parent.test

Modified: 
    lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
    lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 5961ff4439db..67a7b737c7d6 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -247,6 +247,19 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
     return;
   }
 
+  if (info.pl_flags & PL_FLAG_FORKED) {
+    MonitorClone(info.pl_child_pid);
+    return;
+  }
+
+  if (info.pl_flags & PL_FLAG_VFORK_DONE) {
+    Status error =
+        PtraceWrapper(PT_CONTINUE, pid, reinterpret_cast<void *>(1), 0);
+    if (error.Fail())
+      SetState(StateType::eStateInvalid);
+    return;
+  }
+
   if (info.pl_lwpid > 0) {
     for (const auto &t : m_threads) {
       if (t->GetID() == static_cast<lldb::tid_t>(info.pl_lwpid))
@@ -705,17 +718,17 @@ NativeProcessFreeBSD::GetFileLoadAddress(const llvm::StringRef &file_name,
 
 void NativeProcessFreeBSD::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-  // Process all pending waitpid notifications.
   int status;
   ::pid_t wait_pid =
       llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), &status, WNOHANG);
 
   if (wait_pid == 0)
-    return; // We are done.
+    return;
 
   if (wait_pid == -1) {
     Status error(errno, eErrorTypePOSIX);
     LLDB_LOG(log, "waitpid ({0}, &status, _) failed: {1}", GetID(), error);
+    return;
   }
 
   WaitStatus wait_status = WaitStatus::Decode(status);
@@ -885,7 +898,7 @@ Status NativeProcessFreeBSD::SetupTrace() {
       PtraceWrapper(PT_GET_EVENT_MASK, GetID(), &events, sizeof(events));
   if (status.Fail())
     return status;
-  events |= PTRACE_LWP;
+  events |= PTRACE_LWP | PTRACE_FORK | PTRACE_VFORK;
   status = PtraceWrapper(PT_SET_EVENT_MASK, GetID(), &events, sizeof(events));
   if (status.Fail())
     return status;
@@ -919,3 +932,39 @@ Status NativeProcessFreeBSD::ReinitializeThreads() {
 bool NativeProcessFreeBSD::SupportHardwareSingleStepping() const {
   return !m_arch.IsMIPS();
 }
+
+void NativeProcessFreeBSD::MonitorClone(::pid_t child_pid) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
+  LLDB_LOG(log, "fork, child_pid={0}", child_pid);
+
+  int status;
+  ::pid_t wait_pid =
+      llvm::sys::RetryAfterSignal(-1, ::waitpid, child_pid, &status, 0);
+  if (wait_pid != child_pid) {
+    LLDB_LOG(log,
+             "waiting for pid {0} failed. Assuming the pid has "
+             "disappeared in the meantime",
+             child_pid);
+    return;
+  }
+  if (WIFEXITED(status)) {
+    LLDB_LOG(log,
+             "waiting for pid {0} returned an 'exited' event. Not "
+             "tracking it.",
+             child_pid);
+    return;
+  }
+
+  MainLoop unused_loop;
+  NativeProcessFreeBSD child_process{static_cast<::pid_t>(child_pid),
+                                     m_terminal_fd, m_delegate, m_arch,
+                                     unused_loop};
+  child_process.Detach();
+  Status pt_error =
+      PtraceWrapper(PT_CONTINUE, GetID(), reinterpret_cast<void *>(1), 0);
+  if (pt_error.Fail()) {
+    LLDB_LOG_ERROR(log, pt_error.ToError(),
+                   "unable to resume parent process {1}: {0}", GetID());
+    SetState(StateType::eStateInvalid);
+  }
+}

diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
index ceffc370ca33..3ed5f743deba 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -113,6 +113,7 @@ class NativeProcessFreeBSD : public NativeProcessELF,
   void MonitorSIGSTOP(lldb::pid_t pid);
   void MonitorSIGTRAP(lldb::pid_t pid);
   void MonitorSignal(lldb::pid_t pid, int signal);
+  void MonitorClone(::pid_t child_pid);
 
   Status PopulateMemoryRegionCache();
   void SigchldHandler();

diff  --git a/lldb/test/Shell/Subprocess/Inputs/fork.cpp b/lldb/test/Shell/Subprocess/Inputs/fork.cpp
new file mode 100644
index 000000000000..44242934bd3e
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -0,0 +1,50 @@
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <assert.h>
+#if defined(TEST_CLONE)
+#include <sched.h>
+#endif
+#include <stdint.h>
+#include <stdio.h>
+#include <unistd.h>
+
+int g_val = 0;
+
+void parent_func() {
+  g_val = 1;
+  printf("function run in parent\n");
+}
+
+int child_func(void *unused) {
+  // we need to avoid memory modifications for vfork(), yet we want
+  // to be able to test watchpoints, so do the next best thing
+  // and restore the original value
+  g_val = 2;
+  g_val = 0;
+  return 0;
+}
+
+int main() {
+  alignas(uintmax_t) char stack[4096];
+
+#if defined(TEST_CLONE)
+  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, NULL);
+#elif defined(TEST_FORK)
+  pid_t pid = TEST_FORK();
+  if (pid == 0)
+    _exit(child_func(NULL));
+#endif
+  assert(pid != -1);
+
+  parent_func();
+  int status, wait_flags = 0;
+#if defined(TEST_CLONE)
+  wait_flags = __WALL;
+#endif
+  pid_t waited = waitpid(pid, &status, wait_flags);
+  assert(waited == pid);
+  assert(WIFEXITED(status));
+  printf("child exited: %d\n", WEXITSTATUS(status));
+
+  return 0;
+}

diff  --git a/lldb/test/Shell/Subprocess/fork-follow-parent-wp.test b/lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
new file mode 100644
index 000000000000..cde5b41f228b
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && dbregs-set
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0

diff  --git a/lldb/test/Shell/Subprocess/fork-follow-parent.test b/lldb/test/Shell/Subprocess/fork-follow-parent.test
new file mode 100644
index 000000000000..1ffc2cbc1f99
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/fork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0

diff  --git a/lldb/test/Shell/Subprocess/lit.local.cfg b/lldb/test/Shell/Subprocess/lit.local.cfg
new file mode 100644
index 000000000000..c9b378b7a8a5
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/lit.local.cfg
@@ -0,0 +1,2 @@
+if 'lldb-repro' in config.available_features:
+  config.unsupported = True

diff  --git a/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test b/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
new file mode 100644
index 000000000000..8c74e28dbc19
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
@@ -0,0 +1,14 @@
+# REQUIRES: native && dbregs-set
+# UNSUPPORTED: system-windows
+# UNSUPPORTED: system-darwin
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0

diff  --git a/lldb/test/Shell/Subprocess/vfork-follow-parent.test b/lldb/test/Shell/Subprocess/vfork-follow-parent.test
new file mode 100644
index 000000000000..6d9850a706e6
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0


        


More information about the lldb-commits mailing list