mgorny updated this revision to Diff 339217.
mgorny marked an inline comment as done.
mgorny added a comment.

Remove the test, update the API to pass `std::unique_ptr<>` instance instead of 
ref.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100191/new/

https://reviews.llvm.org/D100191

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===================================================================
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,13 @@
   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));
+  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
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===================================================================
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -378,9 +378,7 @@
     return eServerPacketType_C;
 
   case 'D':
-    if (packet_size == 1)
-      return eServerPacketType_D;
-    break;
+    return eServerPacketType_D;
 
   case 'g':
     return eServerPacketType_g;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -78,6 +78,10 @@
 
   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 @@
   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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -249,14 +249,14 @@
 
   {
     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 @@
     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 @@
     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 @@
 
   // 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);
     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 "
@@ -1052,6 +1051,15 @@
   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));
 
@@ -3220,20 +3228,8 @@
 
 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.
@@ -3248,19 +3244,32 @@
       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();
 }
 
@@ -3600,8 +3609,8 @@
   if (bool(m_extensions_supported & NativeProcessProtocol::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;
 }
 
Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h
===================================================================
--- lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -222,6 +222,10 @@
                                      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-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to