[lldb] [server] Support for multiprocess extension

Add a minimal support for the multiprocess extension in lldb-server.
The server indicates support for it via qSupported, and accepts
thread-ids containing a PID.  However, it still does not support
debugging more than one inferior, so any other PID value results
in an error.

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

Author: mgorny

lldb/include/lldb/Utility/StringExtractorGDBRemote.h

@@ -193,6 +193,15 @@ class StringExtractorGDBRemote : public StringExtractor {
 
   size_t GetEscapedBinaryData(std::string &str);
 
+  static constexpr lldb::pid_t AllProcesses = UINT64_MAX;
+  static constexpr lldb::tid_t AllThreads = UINT64_MAX;
+
+  // Read thread-id from the packet.  If the packet is valid, returns
+  // the pair (PID, TID), otherwise returns llvm::None.  If the packet
+  // does not list a PID, default_pid is used.
+  llvm::Optional<std::pair<lldb::pid_t, lldb::tid_t>>
+  GetPidTid(lldb::pid_t default_pid);
+
 protected:
   ResponseValidatorCallback m_validator;
   void *m_validator_baton;

lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

@@ -902,7 +902,8 @@ def add_qSupported_packets(self):
         "qXfer:libraries-svr4:read",
         "qXfer:features:read",
         "qEcho",
-        "QPassSignals"
+        "QPassSignals",
+        "multiprocess",
     ]
 
     def parse_qSupported_response(self, context):

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

@@ -558,6 +558,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   LazyBool m_supports_jGetSharedCacheInfo;
   LazyBool m_supports_QPassSignals;
   LazyBool m_supports_error_string_reply;
+  LazyBool m_supports_multiprocess;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,

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

@@ -848,6 +848,7 @@ GDBRemoteCommunicationServerCommon::Handle_qSupported(
   response.PutCString(";qXfer:auxv:read+");
   response.PutCString(";qXfer:libraries-svr4:read+");
 #endif
+  response.PutCString(";multiprocess+");
 
   return SendPacketNoLock(response.GetString());
 }

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

@@ -1732,10 +1732,13 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
       // Consume the separator.
       packet.GetChar();
 
-      thread_action.tid = packet.GetHexMaxU64(false, LLDB_INVALID_THREAD_ID);
-      if (thread_action.tid == LLDB_INVALID_THREAD_ID)
-        return SendIllFormedResponse(
-            packet, "Could not parse thread number in vCont packet");
+      llvm::Expected<lldb::tid_t> tid_ret = ReadTid(packet, /*allow_all=*/true);
+      if (!tid_ret)
+        return SendErrorResponse(tid_ret.takeError());
+
+      thread_action.tid = tid_ret.get();
+      if (thread_action.tid == StringExtractorGDBRemote::AllThreads)
+        thread_action.tid = LLDB_INVALID_THREAD_ID;
     }
 
     thread_actions.Append(thread_action);
@@ -2220,10 +2223,11 @@ GDBRemoteCommunicationServerLLGS::Handle_H(StringExtractorGDBRemote &packet) {
   }
 
   // Parse out the thread number.
-  // FIXME return a parse success/fail value.  All values are valid here.
-  const lldb::tid_t tid =
-      packet.GetHexMaxU64(false, std::numeric_limits<lldb::tid_t>::max());
+  llvm::Expected<lldb::tid_t> tid_ret = ReadTid(packet, /*allow_all=*/true);
+  if (!tid_ret)
+    return SendErrorResponse(tid_ret.takeError());
 
+  lldb::tid_t tid = tid_ret.get();
   // Ensure we have the given thread when not specifying -1 (all threads) or 0
   // (any thread).
   if (tid != LLDB_INVALID_THREAD_ID && tid != 0) {
@@ -3653,3 +3657,36 @@ std::string GDBRemoteCommunicationServerLLGS::XMLEncodeAttributeValue(
   }
   return result;
 }
+
+llvm::Expected<lldb::tid_t>
+GDBRemoteCommunicationServerLLGS::ReadTid(StringExtractorGDBRemote &packet,
+                                          bool allow_all) {
+  assert(m_debugged_process_up);
+  assert(m_debugged_process_up->GetID() != LLDB_INVALID_PROCESS_ID);
+
+  auto pid_tid = packet.GetPidTid(m_debugged_process_up->GetID());
+  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_debugged_process_up->GetID())
+      return llvm::make_error<StringError>(
+          inconvertibleErrorCode(), llvm::formatv("PID {0} not debugged", pid));
+  }
+
+  return tid;
+}

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

@@ -244,6 +244,15 @@ class GDBRemoteCommunicationServerLLGS
 
   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 = false);
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
       delete;

lldb/source/Utility/StringExtractorGDBRemote.cpp

@@ -11,6 +11,9 @@
 #include <ctype.h>
 #include <string.h>
 
+constexpr lldb::pid_t StringExtractorGDBRemote::AllProcesses;
+constexpr lldb::tid_t StringExtractorGDBRemote::AllThreads;
+
 StringExtractorGDBRemote::ResponseType
 StringExtractorGDBRemote::GetResponseType() const {
   if (m_packet.empty())
@@ -606,3 +609,46 @@ 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::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) {
+      // not a valid hex integer OR unsupported pid 0
+      m_index = UINT64_MAX;
+      return llvm::None;
+    }
+
+    // "." must follow if we expect TID too; otherwise, we assume -1
+    if (!view.consume_front(".")) {
+      // update m_index
+      m_index += initial_length - view.size();
+
+      return {{pid, AllThreads}};
+    }
+  }
+
+  // thread identifier
+  if (view.consume_front("-1")) {
+    // -1 is a special case
+    tid = AllThreads;
+  } else if (view.consumeInteger(16, tid) || tid == 0 || pid == AllProcesses) {
+    // 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}};
+}

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

@@ -49,6 +49,13 @@ def send_and_check_signal(self, vCont_data, threads):
                       for x in range(1, len(threads)+1))
         self.assertEqual(tids, sorted(threads))
 
+    def get_pid(self):
+        self.add_process_info_collection_packets()
+        context = self.expect_gdbremote_sequence()
+        self.assertIsNotNone(context)
+        procinfo = self.parse_process_info_response(context)
+        return int(procinfo['pid'], 16)
+
     @skipIfWindows
     @expectedFailureNetBSD
     @expectedFailureDarwin # No signals delivered
@@ -88,6 +95,82 @@ def test_signal_all_threads(self):
                 *threads),
             threads)
 
+    @skipIfWindows
+    @expectedFailureNetBSD
+    def test_signal_process_by_pid(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = self.start_threads(1)
+        self.send_and_check_signal(
+            "C{0:x}:p{1:x}".format(
+                lldbutil.get_signal_number('SIGUSR1'),
+                self.get_pid()),
+            threads)
+
+    @skipIfWindows
+    @expectedFailureNetBSD
+    def test_signal_process_minus_one(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = self.start_threads(1)
+        self.send_and_check_signal(
+            "C{0:x}:p-1".format(
+                lldbutil.get_signal_number('SIGUSR1')),
+            threads)
+
+    @skipIfWindows
+    @expectedFailureNetBSD
+    def test_signal_minus_one(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = self.start_threads(1)
+        self.send_and_check_signal(
+            "C{0:x}:-1".format(lldbutil.get_signal_number('SIGUSR1')),
+            threads)
+
+    @skipIfWindows
+    @expectedFailureNetBSD
+    def test_signal_all_threads_by_pid(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = self.start_threads(1)
+        # try sending a signal to two threads (= the process)
+        self.send_and_check_signal(
+            "C{0:x}:p{1:x}.{2:x};C{0:x}:p{1:x}.{3:x}".format(
+                lldbutil.get_signal_number('SIGUSR1'),
+                self.get_pid(),
+                *threads),
+            threads)
+
+    @skipIfWindows
+    @expectedFailureNetBSD
+    def test_signal_minus_one_by_pid(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = self.start_threads(1)
+        self.send_and_check_signal(
+            "C{0:x}:p{1:x}.-1".format(
+                lldbutil.get_signal_number('SIGUSR1'),
+                self.get_pid()),
+            threads)
+
+    @skipIfWindows
+    @expectedFailureNetBSD
+    def test_signal_minus_one_by_minus_one(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = self.start_threads(1)
+        self.send_and_check_signal(
+            "C{0:x}:p-1.-1".format(
+                lldbutil.get_signal_number('SIGUSR1')),
+            threads)
+
     @skipUnlessPlatform(["netbsd"])
     def test_signal_two_of_three_threads(self):
         self.build()

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

@@ -349,7 +349,7 @@ def test_p_returns_correct_data_size_for_each_qRegisterInfo_launch(self):
             # Increment loop
             reg_index += 1
 
-    def Hg_switches_to_3_threads(self):
+    def Hg_switches_to_3_threads(self, pass_pid=False):
         # Startup the inferior with three threads (main + 2 new ones).
         procs = self.prep_debug_monitor_and_inferior(
             inferior_args=["thread:new", "thread:new"])
@@ -363,12 +363,16 @@ def Hg_switches_to_3_threads(self):
         threads = self.wait_for_thread_count(3)
         self.assertEqual(len(threads), 3)
 
+        pid_str = ""
+        if pass_pid:
+            pid_str = "p{0:x}.".format(procs["inferior"].pid)
+
         # verify we can $H to each thead, and $qC matches the thread we set.
         for thread in threads:
             # Change to each thread, verify current thread id.
             self.reset_test_sequence()
             self.test_sequence.add_log_lines(
-                ["read packet: $Hg{0:x}#00".format(thread),  # Set current thread.
+                ["read packet: $Hg{0}{1:x}#00".format(pid_str, thread),  # Set current thread.
                  "send packet: $OK#00",
                  "read packet: $qC#00",
                  {"direction": "send", "regex": r"^\$QC([0-9a-fA-F]+)#", "capture": {1: "thread_id"}}],
@@ -393,6 +397,50 @@ def test_Hg_switches_to_3_threads_attach(self):
         self.set_inferior_startup_attach()
         self.Hg_switches_to_3_threads()
 
+    @expectedFailureAll(oslist=["windows"]) # expect 4 threads
+    def test_Hg_switches_to_3_threads_attach_pass_correct_pid(self):
+        self.build()
+        self.set_inferior_startup_attach()
+        self.Hg_switches_to_3_threads(pass_pid=True)
+
+    def Hg_fails_on_pid(self, pass_pid):
+        # Start the inferior.
+        procs = self.prep_debug_monitor_and_inferior(
+            inferior_args=["thread:new"])
+
+        self.run_process_then_stop(run_seconds=1)
+
+        threads = self.wait_for_thread_count(2)
+        self.assertEqual(len(threads), 2)
+
+        if pass_pid == -1:
+            pid_str = "p-1."
+        else:
+            pid_str = "p{0:x}.".format(pass_pid)
+        thread = threads[1]
+
+        self.test_sequence.add_log_lines(
+            ["read packet: $Hg{0}{1:x}#00".format(pid_str, thread),  # Set current thread.
+             "send packet: $Eff#00"],
+            True)
+
+        self.expect_gdbremote_sequence()
+
+    def test_Hg_fails_on_another_pid(self):
+        self.build()
+        self.set_inferior_startup_launch()
+        self.Hg_fails_on_pid(1)
+
+    def test_Hg_fails_on_zero_pid(self):
+        self.build()
+        self.set_inferior_startup_launch()
+        self.Hg_fails_on_pid(0)
+
+    def test_Hg_fails_on_minus_one_pid(self):
+        self.build()
+        self.set_inferior_startup_launch()
+        self.Hg_fails_on_pid(-1)
+
     def Hc_then_Csignal_signals_correct_thread(self, segfault_signo):
         # NOTE only run this one in inferior-launched mode: we can't grab inferior stdout when running attached,
         # and the test requires getting stdout from the exe.

lldb/unittests/Utility/CMakeLists.txt

@@ -29,6 +29,7 @@ add_lldb_unittest(UtilityTests
   StatusTest.cpp
   StreamTeeTest.cpp
   StreamTest.cpp
+  StringExtractorGDBRemoteTest.cpp
   StringExtractorTest.cpp
   StringLexerTest.cpp
   StringListTest.cpp