[lldb] [llgs] Support resuming one process with PID!=current via vCont

[mgorny]

Summary

Extend vCont function to support resuming a process with an arbitrary
PID, that could be different than the one selected via Hc (or no process
at all may be selected). Resuming more than one process simultaneously
is not supported yet.

Remove the ReadTid() method that was only used by Handle_vCont(),
and furthermore it was wrongly using m_current_process rather than
m_continue_process.

Sponsored by: The FreeBSD Foundation

[mgorny]

Rebase.

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

private:
   void DataAvailableCallback();
 
   void SendProcessOutput();
 
   void StartSTDIOForwarding();
 
   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);
 
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
       delete;
   const GDBRemoteCommunicationServerLLGS &
   operator=(const GDBRemoteCommunicationServerLLGS &) = delete;
 };
 
 std::string LLGSArgToURL(llvm::StringRef url_arg, bool reverse_connect);
 
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_GDBREMOTECOMMUNICATIONSERVERLLGS_H

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

GDBRemoteCommunicationServerLLGS::Handle_vCont(
   if (packet.GetBytesLeft() == 0) {
     LLDB_LOGF(log,
               "GDBRemoteCommunicationServerLLGS::%s missing action from "
               "vCont package",
               __FUNCTION__);
     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);
   } else if (m_non_stop && ::strcmp(packet.Peek(), ";t") == 0) {
     // TODO: add full support for "t" action
     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.
     packet.GetChar();
 
     // Build up the thread action.
     ResumeAction thread_action;
     thread_action.tid = LLDB_INVALID_THREAD_ID;
case 's':
       thread_action.state = eStateStepping;
       break;
 
     default:
       return SendIllFormedResponse(packet, "Unsupported vCont action");
       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;
+    }
+
+    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();
+    }
+
+    if (tid == StringExtractorGDBRemote::AllThreads)
+      tid = LLDB_INVALID_THREAD_ID;
+
+    thread_action.tid = tid;
+
+    thread_actions[pid].Append(thread_action);
 
-    thread_actions.Append(thread_action);
+  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 = m_continue_process->Resume(thread_actions);
+    Status error = process_it->second->Resume(x.second);
     if (error.Fail()) {
-    LLDB_LOG(log, "vCont failed for process {0}: {1}",
-             m_continue_process->GetID(), error);
+      LLDB_LOG(log, "vCont failed for process {0}: {1}", x.first, error);
       return SendErrorResponse(GDBRemoteServerError::eErrorResume);
     }
 
-  LLDB_LOG(log, "continued process {0}", m_continue_process->GetID());
+    LLDB_LOG(log, "continued process {0}", x.first);
+  }
 
   return SendContinueSuccessResponse();
 }
 
 void GDBRemoteCommunicationServerLLGS::SetCurrentThreadID(lldb::tid_t tid) {
   Log *log = GetLog(LLDBLog::Thread);
   LLDB_LOG(log, "setting current thread id to {0}", tid);
 
for (const char &c : value) {
     default:
       result += c;
       break;
     }
   }
   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 =
       GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
   ret.insert(ret.end(), {
                             "QThreadSuffixSupported+",
                             "QListThreadsInStopReply+",
                             "qXfer:features:read+",

lldb/source/Utility/StringExtractorGDBRemote.cpp

bool StringExtractorGDBRemote::ValidateResponse() const {
   else
     return true; // No validator, so response is valid
 }
 
 llvm::Optional<std::pair<lldb::pid_t, lldb::tid_t>>
 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")) {
     // process identifier
     if (view.consume_front("-1")) {
       // -1 is a special case
       pid = AllProcesses;
     } else if (view.consumeInteger(16, pid) || pid == 0) {
if (view.consume_front("-1")) {
     // not a valid hex integer OR tid 0 OR pid -1 + a specific tid
     m_index = UINT64_MAX;
     return llvm::None;
   }
 
   // update m_index
   m_index += initial_length - view.size();
 
-  return {{pid, tid}};
+  return {{pid != LLDB_INVALID_PROCESS_ID ? pid : default_pid, tid}};
 }

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

class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
     @add_test_categories(["fork"])
     def test_vkill_parent(self):
         self.vkill_test(kill_parent=True)
 
     @add_test_categories(["fork"])
     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+",
                                      "fork-events+"])
         ret = self.expect_gdbremote_sequence()
         self.assertIn("fork-events+", ret["qSupported_response"])
         self.reset_test_sequence()
 
def resume_one_test(self, run_order, use_vCont=False):
                 pidtid = (parent_pid, parent_tid)
                 expect = parent_expect.pop(0)
             elif x == "child":
                 pidtid = (child_pid, child_tid)
                 expect = child_expect.pop(0)
             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([
                 {"direction": "send", "regex": expect},
             ], True)
             # if at least one process remained, check both PIDs
             if parent_expect or child_expect:
                 self.test_sequence.add_log_lines([
                     "read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
                     "send packet: ${}#00".format("OK" if parent_expect else "Eff"),
                     "read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 
     @add_test_categories(["fork"])
     def test_c_child_then_parent(self):
         self.resume_one_test(run_order=["child", "child", "parent", "parent"])
 
     @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()