[lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

[mgorny]

Summary

Support using the vCont packet to resume multiple processes
simultaneously when in non-stop mode. The new logic now assumes that:

• actions without a thread-id or with process id of "p-1" apply to all debugged processes

• actions with a thread-id without process id apply to the current process (m_continue_process)

As with the other continue packets, it is only possible to resume
processes that are currently stopped (or stop these that are running).
It is unsupported to resume or stop individual threads of a running
process.

Sponsored by: The FreeBSD Foundation

[mgorny]

@labath, gentle ping. We appreciate all the work you've done reviewing my patches already, and we realize this is a lot of extra work for you. However, could you please try to review this one as well? It's the last patch from our LLGS effort, so getting it in could help us close this topic for now.

[labath]

Looks good modulo the comments.

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

1786

https://sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets says "It is an error to specify all processes but a specific thread, such as ‘p-1.tid’", so I guess this should error out if tid != AllThreads.

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

153

See cd18e2ea3f4e87f8804a7d6661d5596ef1f07b81 for why that does not work.

[mgorny]

Thanks!

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

1786

Makes sense. For some reason, I thought we've covered it somewhere but maybe it got lost in refactoring.

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

case 't':
       thread_action.state = eStateSuspended;
       break;
 
     default:
       return SendIllFormedResponse(packet, "Unsupported vCont action");
       break;
     }
 
+    // If there's no thread-id (e.g. "vCont;c"), it's "p-1.-1".
     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();
 
-      auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+      auto pid_tid = packet.GetPidTid(LLDB_INVALID_PROCESS_ID);
       if (!pid_tid)
         return SendIllFormedResponse(packet, "Malformed 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 we get TID without PID, it's the current process.
+      if (pid == LLDB_INVALID_PROCESS_ID) {
         if (!m_continue_process) {
-        LLDB_LOG(log, "no debugged process");
+          LLDB_LOG(log, "no process selected via Hc");
           return SendErrorResponse(0x36);
         }
         pid = m_continue_process->GetID();
       }
+    }
 
+    assert(pid != LLDB_INVALID_PROCESS_ID);
     if (tid == StringExtractorGDBRemote::AllThreads)
       tid = LLDB_INVALID_THREAD_ID;
-
     thread_action.tid = tid;
 
+    if (pid == StringExtractorGDBRemote::AllProcesses) {
+      for (auto &process_it : m_debugged_processes)
+        thread_actions[process_it.first].Append(thread_action);
+    } else
       thread_actions[pid].Append(thread_action);
   }
 
   assert(thread_actions.size() >= 1);
-  if (thread_actions.size() > 1)
+  if (thread_actions.size() > 1 && !m_non_stop)
     return SendIllFormedResponse(
-        packet, "Resuming multiple processes not supported yet");
+        packet,
+        "Resuming multiple processes is supported in non-stop mode only");
 
   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);
     }

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

def test_c_both_nonstop(self):
             "read packet: $vStdio#00",
             "send packet: $OK#00",
             ], True)
         ret = self.expect_gdbremote_sequence()
         self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
                       ret["O_content"])
         self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
                       ret["O_content"])
+
+    @add_test_categories(["fork"])
+    def test_vCont_both_nonstop(self):
+        parent_pid, parent_tid, child_pid, child_tid = (
+            self.start_fork_test(["fork", "process:sync", "print-pid",
+                                  "process:sync", "stop"],
+                                 nonstop=True))
+
+        self.test_sequence.add_log_lines([
+            "read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+                parent_pid, parent_tid, child_pid, child_tid),
+            "send packet: $OK#00",
+            {"direction": "send", "regex": "%Stop:T.*"},
+            # see the comment in TestNonStop.py, test_stdio
+            "read packet: $vStdio#00",
+            "read packet: $vStdio#00",
+            "send packet: $OK#00",
+            ], True)
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
+                      ret["O_content"])
+        self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
+                      ret["O_content"])
+
+    def vCont_both_nonstop_test(self, vCont_packet):
+        parent_pid, parent_tid, child_pid, child_tid = (
+            self.start_fork_test(["fork", "process:sync", "print-pid",
+                                  "process:sync", "stop"],
+                                 nonstop=True))
+
+        self.test_sequence.add_log_lines([
+            "read packet: ${}#00".format(vCont_packet),
+            "send packet: $OK#00",
+            {"direction": "send", "regex": "%Stop:T.*"},
+            # see the comment in TestNonStop.py, test_stdio
+            "read packet: $vStdio#00",
+            "read packet: $vStdio#00",
+            "send packet: $OK#00",
+            ], True)
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
+                      ret["O_content"])
+        self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
+                      ret["O_content"])
+
+    @add_test_categories(["fork"])
+    def test_vCont_both_implicit_nonstop(self):
+        self.vCont_both_nonstop_test("vCont;c")
+
+    @add_test_categories(["fork"])
+    def test_vCont_both_minus_one_nonstop(self):
+        self.vCont_both_nonstop_test("vCont;c:p-1.-1")