This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3422793e064: [lldb] [llgs] Support resuming one process 
with PID!=current via vCont (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127862

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -354,7 +354,7 @@
     def test_vkill_both(self):
         self.vkill_test(kill_parent=True, kill_child=True)
 
-    def resume_one_test(self, run_order):
+    def resume_one_test(self, run_order, use_vCont=False):
         self.build()
         self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
         self.add_qSupported_packets(["multiprocess+",
@@ -395,11 +395,19 @@
             else:
                 assert False, "unexpected x={}".format(x)
 
+            if use_vCont:
+                self.test_sequence.add_log_lines([
+                    # continue the selected process
+                    "read packet: $vCont;c:p{}.{}#00".format(*pidtid),
+                ], True)
+            else:
+                self.test_sequence.add_log_lines([
+                    # continue the selected process
+                    "read packet: $Hcp{}.{}#00".format(*pidtid),
+                    "send packet: $OK#00",
+                    "read packet: $c#00",
+                ], True)
             self.test_sequence.add_log_lines([
-                # continue the selected process
-                "read packet: $Hcp{}.{}#00".format(*pidtid),
-                "send packet: $OK#00",
-                "read packet: $c#00",
                 {"direction": "send", "regex": expect},
             ], True)
             # if at least one process remained, check both PIDs
@@ -431,3 +439,117 @@
     @add_test_categories(["fork"])
     def test_c_interspersed(self):
         self.resume_one_test(run_order=["parent", "child", "parent", "child"])
+
+    @add_test_categories(["fork"])
+    def test_vCont_parent(self):
+        self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
+
+    @add_test_categories(["fork"])
+    def test_vCont_child(self):
+        self.resume_one_test(run_order=["child", "child"], use_vCont=True)
+
+    @add_test_categories(["fork"])
+    def test_vCont_parent_then_child(self):
+        self.resume_one_test(run_order=["parent", "parent", "child", "child"],
+                             use_vCont=True)
+
+    @add_test_categories(["fork"])
+    def test_vCont_child_then_parent(self):
+        self.resume_one_test(run_order=["child", "child", "parent", "parent"],
+                             use_vCont=True)
+
+    @add_test_categories(["fork"])
+    def test_vCont_interspersed(self):
+        self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+                             use_vCont=True)
+
+    @add_test_categories(["fork"])
+    def test_vCont_two_processes(self):
+        self.build()
+        self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+        self.add_qSupported_packets(["multiprocess+",
+                                     "fork-events+"])
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("fork-events+", ret["qSupported_response"])
+        self.reset_test_sequence()
+
+        # continue and expect fork
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": self.fork_regex.format("fork"),
+             "capture": self.fork_capture},
+        ], True)
+        ret = self.expect_gdbremote_sequence()
+        parent_pid = ret["parent_pid"]
+        parent_tid = ret["parent_tid"]
+        child_pid = ret["child_pid"]
+        child_tid = ret["child_tid"]
+        self.reset_test_sequence()
+
+        self.test_sequence.add_log_lines([
+            # try to resume both processes
+            "read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+                parent_pid, parent_tid, child_pid, child_tid),
+            "send packet: $E03#00",
+        ], True)
+        self.expect_gdbremote_sequence()
+
+    @add_test_categories(["fork"])
+    def test_vCont_all_processes_explicit(self):
+        self.build()
+        self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+        self.add_qSupported_packets(["multiprocess+",
+                                     "fork-events+"])
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("fork-events+", ret["qSupported_response"])
+        self.reset_test_sequence()
+
+        # continue and expect fork
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": self.fork_regex.format("fork"),
+             "capture": self.fork_capture},
+        ], True)
+        ret = self.expect_gdbremote_sequence()
+        parent_pid = ret["parent_pid"]
+        parent_tid = ret["parent_tid"]
+        child_pid = ret["child_pid"]
+        child_tid = ret["child_tid"]
+        self.reset_test_sequence()
+
+        self.test_sequence.add_log_lines([
+            # try to resume all processes implicitly
+            "read packet: $vCont;c:p-1.-1#00",
+            "send packet: $E03#00",
+        ], True)
+        self.expect_gdbremote_sequence()
+
+    @add_test_categories(["fork"])
+    def test_vCont_all_processes_implicit(self):
+        self.build()
+        self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+        self.add_qSupported_packets(["multiprocess+",
+                                     "fork-events+"])
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("fork-events+", ret["qSupported_response"])
+        self.reset_test_sequence()
+
+        # continue and expect fork
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": self.fork_regex.format("fork"),
+             "capture": self.fork_capture},
+        ], True)
+        ret = self.expect_gdbremote_sequence()
+        parent_pid = ret["parent_pid"]
+        parent_tid = ret["parent_tid"]
+        child_pid = ret["child_pid"]
+        child_tid = ret["child_tid"]
+        self.reset_test_sequence()
+
+        self.test_sequence.add_log_lines([
+            # try to resume all processes implicitly
+            "read packet: $vCont;c#00",
+            "send packet: $E03#00",
+        ], True)
+        self.expect_gdbremote_sequence()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===================================================================
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -639,7 +639,7 @@
 StringExtractorGDBRemote::GetPidTid(lldb::pid_t default_pid) {
   llvm::StringRef view = llvm::StringRef(m_packet).substr(m_index);
   size_t initial_length = view.size();
-  lldb::pid_t pid = default_pid;
+  lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
   lldb::tid_t tid;
 
   if (view.consume_front("p")) {
@@ -675,5 +675,5 @@
   // update m_index
   m_index += initial_length - view.size();
 
-  return {{pid, tid}};
+  return {{pid != LLDB_INVALID_PROCESS_ID ? pid : default_pid, tid}};
 }
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
@@ -293,15 +293,6 @@
 
   void StopSTDIOForwarding();
 
-  // Read thread-id from packet.  If the thread-id is correct, returns it.
-  // Otherwise, returns the error.
-  //
-  // If allow_all is true, then the pid/tid value of -1 ('all') will be allowed.
-  // In any case, the function assumes that exactly one inferior is being
-  // debugged and rejects pid values that do no match that inferior.
-  llvm::Expected<lldb::tid_t> ReadTid(StringExtractorGDBRemote &packet,
-                                      bool allow_all, lldb::pid_t default_pid);
-
   // Call SetEnabledExtensions() with appropriate flags on the process.
   void SetEnabledExtensions(NativeProcessProtocol &process);
 
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
@@ -1674,12 +1674,7 @@
     return SendIllFormedResponse(packet, "Missing action from vCont package");
   }
 
-  // Check if this is all continue (no options or ";c").
-  if (::strcmp(packet.Peek(), ";c") == 0) {
-    // Move past the ';', then do a simple 'c'.
-    packet.SetFilePos(packet.GetFilePos() + 1);
-    return Handle_c(packet);
-  } else if (::strcmp(packet.Peek(), ";s") == 0) {
+  if (::strcmp(packet.Peek(), ";s") == 0) {
     // Move past the ';', then do a simple 's'.
     packet.SetFilePos(packet.GetFilePos() + 1);
     return Handle_s(packet);
@@ -1688,13 +1683,7 @@
     return SendOKResponse();
   }
 
-  // Ensure we have a native process.
-  if (!m_continue_process) {
-    LLDB_LOG(log, "no debugged process");
-    return SendErrorResponse(0x36);
-  }
-
-  ResumeActionList thread_actions;
+  std::unordered_map<lldb::pid_t, ResumeActionList> thread_actions;
 
   while (packet.GetBytesLeft() && *packet.Peek() == ';') {
     // Skip the semi-colon.
@@ -1737,32 +1726,62 @@
       break;
     }
 
+    lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
+    lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
+
     // Parse out optional :{thread-id} value.
     if (packet.GetBytesLeft() && (*packet.Peek() == ':')) {
       // Consume the separator.
       packet.GetChar();
 
-      llvm::Expected<lldb::tid_t> tid_ret =
-          ReadTid(packet, /*allow_all=*/true, m_continue_process->GetID());
-      if (!tid_ret)
-        return SendErrorResponse(tid_ret.takeError());
+      auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+      if (!pid_tid)
+        return SendIllFormedResponse(packet, "Malformed thread-id");
 
-      thread_action.tid = tid_ret.get();
-      if (thread_action.tid == StringExtractorGDBRemote::AllThreads)
-        thread_action.tid = LLDB_INVALID_THREAD_ID;
+      pid = pid_tid->first;
+      tid = pid_tid->second;
     }
 
-    thread_actions.Append(thread_action);
-  }
+    if (pid == StringExtractorGDBRemote::AllProcesses) {
+      if (m_debugged_processes.size() > 1)
+        return SendIllFormedResponse(
+            packet, "Resuming multiple processes not supported yet");
+      if (!m_continue_process) {
+        LLDB_LOG(log, "no debugged process");
+        return SendErrorResponse(0x36);
+      }
+      pid = m_continue_process->GetID();
+    }
 
-  Status error = m_continue_process->Resume(thread_actions);
-  if (error.Fail()) {
-    LLDB_LOG(log, "vCont failed for process {0}: {1}",
-             m_continue_process->GetID(), error);
-    return SendErrorResponse(GDBRemoteServerError::eErrorResume);
+    if (tid == StringExtractorGDBRemote::AllThreads)
+      tid = LLDB_INVALID_THREAD_ID;
+
+    thread_action.tid = tid;
+
+    thread_actions[pid].Append(thread_action);
   }
 
-  LLDB_LOG(log, "continued process {0}", m_continue_process->GetID());
+  assert(thread_actions.size() >= 1);
+  if (thread_actions.size() > 1)
+    return SendIllFormedResponse(
+        packet, "Resuming multiple processes not supported yet");
+
+  for (std::pair<lldb::pid_t, ResumeActionList> x : thread_actions) {
+    auto process_it = m_debugged_processes.find(x.first);
+    if (process_it == m_debugged_processes.end()) {
+      LLDB_LOG(log, "vCont failed for process {0}: process not debugged",
+               x.first);
+      return SendErrorResponse(GDBRemoteServerError::eErrorResume);
+    }
+
+    Status error = process_it->second->Resume(x.second);
+    if (error.Fail()) {
+      LLDB_LOG(log, "vCont failed for process {0}: {1}", x.first, error);
+      return SendErrorResponse(GDBRemoteServerError::eErrorResume);
+    }
+
+    LLDB_LOG(log, "continued process {0}", x.first);
+  }
 
   return SendContinueSuccessResponse();
 }
@@ -4011,38 +4030,6 @@
   return result;
 }
 
-llvm::Expected<lldb::tid_t> GDBRemoteCommunicationServerLLGS::ReadTid(
-    StringExtractorGDBRemote &packet, bool allow_all, lldb::pid_t default_pid) {
-  assert(m_current_process);
-  assert(m_current_process->GetID() != LLDB_INVALID_PROCESS_ID);
-
-  auto pid_tid = packet.GetPidTid(default_pid);
-  if (!pid_tid)
-    return llvm::make_error<StringError>(inconvertibleErrorCode(),
-                                         "Malformed thread-id");
-
-  lldb::pid_t pid = pid_tid->first;
-  lldb::tid_t tid = pid_tid->second;
-
-  if (!allow_all && pid == StringExtractorGDBRemote::AllProcesses)
-    return llvm::make_error<StringError>(
-        inconvertibleErrorCode(),
-        llvm::formatv("PID value {0} not allowed", pid == 0 ? 0 : -1));
-
-  if (!allow_all && tid == StringExtractorGDBRemote::AllThreads)
-    return llvm::make_error<StringError>(
-        inconvertibleErrorCode(),
-        llvm::formatv("TID value {0} not allowed", tid == 0 ? 0 : -1));
-
-  if (pid != StringExtractorGDBRemote::AllProcesses) {
-    if (pid != m_current_process->GetID())
-      return llvm::make_error<StringError>(
-          inconvertibleErrorCode(), llvm::formatv("PID {0} not debugged", pid));
-  }
-
-  return tid;
-}
-
 std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
     const llvm::ArrayRef<llvm::StringRef> client_features) {
   std::vector<std::string> ret =
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to