[lldb] [llgs] Support owning and detaching extra processes

Add a NativeDelegate API to pass new processes (forks) to LLGS,
and support detaching them via the 'D' packet.  A 'D' packet without
a specific PID detaches all processes, otherwise it detaches either
the specified subprocess or the main process, depending on the passed
PID.

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

lldb/include/lldb/Host/common/NativeProcessProtocol.h

@@ -222,6 +222,10 @@ public:
                                      lldb::StateType state) = 0;
 
     virtual void DidExec(NativeProcessProtocol *process) = 0;
+
+    virtual void
+    NewSubprocess(NativeProcessProtocol *parent_process,
+                  std::unique_ptr<NativeProcessProtocol> child_process) = 0;
   };
 
   virtual Status GetLoadedModuleFileSpec(const char *module_path,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

@@ -249,14 +249,14 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
 
   {
     std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
-    assert(!m_debugged_process_up && "lldb-server creating debugged "
-                                     "process but one already exists");
+    assert(m_debugged_processes.empty() && "lldb-server creating debugged "
+                                           "process but one already exists");
     auto process_or =
         m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
     if (!process_or)
       return Status(process_or.takeError());
-    m_debugged_process_up = std::move(*process_or);
-    m_continue_process = m_current_process = m_debugged_process_up.get();
+    m_continue_process = m_current_process = process_or->get();
+    m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
   }
 
   SetEnabledExtensions(*m_current_process);
@@ -273,10 +273,10 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
     LLDB_LOG(log,
              "pid = {0}: setting up stdout/stderr redirection via $O "
              "gdb-remote commands",
-             m_debugged_process_up->GetID());
+             m_current_process->GetID());
 
     // Setup stdout/stderr mapping from inferior to $O
-    auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+    auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
     if (terminal_fd >= 0) {
       LLDB_LOGF(log,
                 "ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -295,12 +295,12 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
     LLDB_LOG(log,
              "pid = {0} skipping stdout/stderr redirection via $O: inferior "
              "will communicate over client-provided file descriptors",
-             m_debugged_process_up->GetID());
+             m_current_process->GetID());
   }
 
   printf("Launched '%s' as process %" PRIu64 "...\n",
          m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
-         m_debugged_process_up->GetID());
+         m_current_process->GetID());
 
   return Status();
 }
@@ -312,12 +312,11 @@ Status GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
 
   // Before we try to attach, make sure we aren't already monitoring something
   // else.
-  if (m_debugged_process_up &&
-      m_debugged_process_up->GetID() != LLDB_INVALID_PROCESS_ID)
+  if (!m_debugged_processes.empty())
     return Status("cannot attach to process %" PRIu64
                   " when another process with pid %" PRIu64
                   " is being debugged.",
-                  pid, m_debugged_process_up->GetID());
+                  pid, m_current_process->GetID());
 
   // Try to attach.
   auto process_or = m_process_factory.Attach(pid, *this, m_mainloop);
@@ -327,12 +326,12 @@ Status GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
                                   status);
     return status;
   }
-  m_debugged_process_up = std::move(*process_or);
-  m_continue_process = m_current_process = m_debugged_process_up.get();
+  m_continue_process = m_current_process = process_or->get();
+  m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
   SetEnabledExtensions(*m_current_process);
 
   // Setup stdout/stderr mapping from inferior.
-  auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+  auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
   if (terminal_fd >= 0) {
     LLDB_LOGF(log,
               "ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -1060,6 +1059,15 @@ void GDBRemoteCommunicationServerLLGS::DidExec(NativeProcessProtocol *process) {
   ClearProcessSpecificData();
 }
 
+void GDBRemoteCommunicationServerLLGS::NewSubprocess(
+    NativeProcessProtocol *parent_process,
+    std::unique_ptr<NativeProcessProtocol> child_process) {
+  lldb::pid_t child_pid = child_process->GetID();
+  assert(child_pid != LLDB_INVALID_PROCESS_ID);
+  assert(m_debugged_processes.find(child_pid) == m_debugged_processes.end());
+  m_debugged_processes[child_pid] = std::move(child_process);
+}
+
 void GDBRemoteCommunicationServerLLGS::DataAvailableCallback() {
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_COMM));
 
@@ -3228,20 +3236,8 @@ GDBRemoteCommunicationServerLLGS::Handle_vAttachOrWait(
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
-
   StopSTDIOForwarding();
 
-  // Fail if we don't have a current process.
-  if (!m_current_process ||
-      (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-    LLDB_LOGF(
-        log,
-        "GDBRemoteCommunicationServerLLGS::%s failed, no process available",
-        __FUNCTION__);
-    return SendErrorResponse(0x15);
-  }
-
   lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
 
   // Consume the ';' after D.
@@ -3256,19 +3252,32 @@ GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
       return SendIllFormedResponse(packet, "D failed to parse the process id");
   }
 
-  if (pid != LLDB_INVALID_PROCESS_ID && m_current_process->GetID() != pid) {
-    return SendIllFormedResponse(packet, "Invalid pid");
-  }
-
-  const Status error = m_current_process->Detach();
-  if (error.Fail()) {
-    LLDB_LOGF(log,
-              "GDBRemoteCommunicationServerLLGS::%s failed to detach from "
-              "pid %" PRIu64 ": %s\n",
-              __FUNCTION__, m_current_process->GetID(), error.AsCString());
-    return SendErrorResponse(0x01);
+  // Detach forked children if their PID was specified *or* no PID was requested
+  // (i.e. detach-all packet).
+  llvm::Error detach_error = llvm::Error::success();
+  bool detached = false;
+  for (auto it = m_debugged_processes.begin();
+       it != m_debugged_processes.end();) {
+    if (pid == LLDB_INVALID_PROCESS_ID || pid == it->first) {
+      if (llvm::Error e = it->second->Detach().ToError())
+        detach_error = llvm::joinErrors(std::move(detach_error), std::move(e));
+      else {
+        if (it->second.get() == m_current_process)
+          m_current_process = nullptr;
+        if (it->second.get() == m_continue_process)
+          m_continue_process = nullptr;
+        it = m_debugged_processes.erase(it);
+        detached = true;
+        continue;
+      }
+    }
+    ++it;
   }
 
+  if (detach_error)
+    return SendErrorResponse(std::move(detach_error));
+  if (!detached)
+    return SendErrorResponse(Status("PID %" PRIu64 " not traced", pid));
   return SendOKResponse();
 }
 
@@ -3616,8 +3625,8 @@ std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
   if (bool(m_extensions_supported & Extension::vfork))
     ret.push_back("vfork-events+");
 
-  if (m_debugged_process_up)
-    SetEnabledExtensions(*m_debugged_process_up);
+  for (auto &x : m_debugged_processes)
+    SetEnabledExtensions(*x.second);
   return ret;
 }
 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

@@ -78,6 +78,10 @@ public:
 
   void DidExec(NativeProcessProtocol *process) override;
 
+  void
+  NewSubprocess(NativeProcessProtocol *parent_process,
+                std::unique_ptr<NativeProcessProtocol> child_process) override;
+
   Status InitializeConnection(std::unique_ptr<Connection> connection);
 
 protected:
@@ -89,7 +93,8 @@ protected:
   NativeProcessProtocol *m_current_process;
   NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
-  std::unique_ptr<NativeProcessProtocol> m_debugged_process_up;
+  std::unordered_map<lldb::pid_t, std::unique_ptr<NativeProcessProtocol>>
+      m_debugged_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;

lldb/source/Utility/StringExtractorGDBRemote.cpp

@@ -378,9 +378,7 @@ StringExtractorGDBRemote::GetServerPacketType() const {
     return eServerPacketType_C;
 
   case 'D':
-    if (packet_size == 1)
-      return eServerPacketType_D;
-    break;
+    return eServerPacketType_D;
 
   case 'g':
     return eServerPacketType_g;

lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

@@ -0,0 +1,59 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
+    mydir = TestBase.compute_mydir(__file__)
+
+    def fork_and_detach_test(self, variant):
+        self.build()
+        self.prep_debug_monitor_and_inferior(inferior_args=[variant])
+        self.add_qSupported_packets(["multiprocess+",
+                                     "{}-events+".format(variant)])
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+        self.reset_test_sequence()
+
+        # continue and expect fork
+        fork_regex = "[$]T.*;{}:p([0-9a-f]*)[.]([0-9a-f]*).*".format(variant)
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": fork_regex,
+             "capture": {1: "pid", 2: "tid"}},
+        ], True)
+        ret = self.expect_gdbremote_sequence()
+        pid = int(ret["pid"], 16)
+        self.reset_test_sequence()
+
+        # detach the forked child
+        self.test_sequence.add_log_lines([
+            "read packet: $D;{:x}#00".format(pid),
+            {"direction": "send", "regex": r"[$]OK#.*"},
+        ], True)
+        ret = self.expect_gdbremote_sequence()
+        self.reset_test_sequence()
+
+    @add_test_categories(["fork"])
+    def test_fork(self):
+        self.fork_and_detach_test("fork")
+
+        # resume the parent
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": r"[$]W00#.*"},
+        ], True)
+        self.expect_gdbremote_sequence()
+
+    @add_test_categories(["fork"])
+    def test_vfork(self):
+        self.fork_and_detach_test("vfork")
+
+        # resume the parent
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": r"[$]T.*vforkdone.*"},
+            "read packet: $c#00",
+            {"direction": "send", "regex": r"[$]W00#.*"},
+        ], True)
+        self.expect_gdbremote_sequence()

lldb/test/API/tools/lldb-server/main.cpp

@@ -216,7 +216,13 @@ int main(int argc, char **argv) {
 
   sig_result = signal(SIGSEGV, signal_handler);
   if (sig_result == SIG_ERR) {
-    fprintf(stderr, "failed to set SIGUSR1 handler: errno=%d\n", errno);
+    fprintf(stderr, "failed to set SIGSEGV handler: errno=%d\n", errno);
+    exit(1);
+  }
+
+  sig_result = signal(SIGCHLD, SIG_IGN);
+  if (sig_result == SIG_ERR) {
+    fprintf(stderr, "failed to set SIGCHLD handler: errno=%d\n", errno);
     exit(1);
   }
 #endif
@@ -293,6 +299,14 @@ int main(int argc, char **argv) {
       else if (arg == "swap_chars")
         func_p = swap_chars;
       func_p();
+#if !defined(_WIN32)
+    } else if (arg == "fork") {
+      if (fork() == 0)
+        _exit(0);
+    } else if (arg == "vfork") {
+      if (vfork() == 0)
+        _exit(0);
+#endif
     } else if (consume_front(arg, "thread:new")) {
         threads.push_back(std::thread(thread_func, nullptr));
     } else if (consume_front(arg, "thread:print-ids")) {

lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

@@ -25,6 +25,15 @@ public:
   MOCK_METHOD2(ProcessStateChanged,
                void(NativeProcessProtocol *Process, StateType State));
   MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(NewSubprocessImpl,
+               void(NativeProcessProtocol *parent_process,
+                    std::unique_ptr<NativeProcessProtocol> &child_process));
+  // This is a hack to avoid MOCK_METHOD2 incompatibility with std::unique_ptr
+  // passed as value.
+  void NewSubprocess(NativeProcessProtocol *parent_process,
+                     std::unique_ptr<NativeProcessProtocol> child_process) {
+    NewSubprocessImpl(parent_process, child_process);
+  }
 };
 
 // NB: This class doesn't use the override keyword to avoid