[lldb] [client] Support for multiprocess extension

[mgorny]

Summary

Add a minimal support for the multiprocess extension in gdb-remote
client. It accepts PIDs as part of thread-ids, and rejects PIDs that
do not match the current inferior.

[mgorny]

@labath, split GetCurrentThreadIDs() as suggested. I've also modified GetCurrentProcessID() to prefer explicit PID over TID when available.

[mgorny]

Added better PID mismatch handling in SetThreadStopInfo(). Not that most of the call sites actually check the return value...

[labath]

I think this looks fine. Could you also create a gdb-client test case (you can ignore the default thread thingy)?

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

1130

It looks like this is only used in the (probably completely broken) non-stop mode. I should put that on my to-delete list...

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

370

lets make this an actual return value

[mgorny]

Changed GetCurrentProcessAndThreadIDs() to return the vector, and reformatted.

Tests next.

[mgorny]

Add some tests.

@labath, I'd use some help now. When the stop reason includes wrong PID, the GDB exchange seems to enter a dead loop:

b-remote.async>  <   5> send packet: $c#63
b-remote.async>  <  25> read packet: $S02thread:p404.10210;#cc
intern-state     <  16> send packet: $qfThreadInfo#bb
intern-state     <  26> read packet: $mp400.10200,p400.10204#e7
intern-state     <  16> send packet: $qsThreadInfo#c8
intern-state     <   5> read packet: $l#6c

I think this is because most of the code doesn't really care about SetThreadStopInfo()'s return value, and I'm not really convinced that's the best way of reporting an error. Could you suggest how to handle 'wrong PID' error here?

[mgorny]

Added a skip to the hanging new test.

[labath]

Yeah, that's tricky. For a inconsistent/unsupported response like this, probably the only reasonable thing we can do print some error message, terminate the connection and put the inferior into some terminal state. However, I don't think we have anything like that (and I suspect there are plenty of other ways one could trigger a loop like this). I'd say we should just delete the test right now.

[mgorny]

Removed the hanging test.

[mgorny]

@labath, any further comments or is this good to go then?

[labath]

Yea, I think it should be fine.

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

public:
   LazyBool SupportsAllocDeallocMemory() // const
   {
     // Uncomment this to have lldb pretend the debug server doesn't respond to
     // alloc/dealloc memory packets.
     // m_supports_alloc_dealloc_memory = lldb_private::eLazyBoolNo;
     return m_supports_alloc_dealloc_memory;
   }
 
+  std::vector<std::pair<lldb::pid_t, lldb::tid_t>>
+  GetCurrentProcessAndThreadIDs(bool &sequence_mutex_unavailable);
+
   size_t GetCurrentThreadIDs(std::vector<lldb::tid_t> &thread_ids,
                              bool &sequence_mutex_unavailable);
 
   lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
                            mode_t mode, Status &error);
 
   bool CloseFile(lldb::user_id_t fd, Status &error);
 

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

: GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_qXfer_features_read(eLazyBoolCalculate),
       m_supports_qXfer_memory_map_read(eLazyBoolCalculate),
       m_supports_augmented_libraries_svr4_read(eLazyBoolCalculate),
       m_supports_jThreadExtendedInfo(eLazyBoolCalculate),
       m_supports_jLoadedDynamicLibrariesInfos(eLazyBoolCalculate),
       m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
       m_supports_QPassSignals(eLazyBoolCalculate),
       m_supports_error_string_reply(eLazyBoolCalculate),
+      m_supports_multiprocess(eLazyBoolCalculate),
       m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
       m_supports_qUserName(true), m_supports_qGroupName(true),
       m_supports_qThreadStopInfo(true), m_supports_z0(true),
       m_supports_z1(true), m_supports_z2(true), m_supports_z3(true),
       m_supports_z4(true), m_supports_QEnvironment(true),
       m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
       m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
       m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
if (!did_exec) {
     m_qHostInfo_is_valid = eLazyBoolCalculate;
     m_curr_pid_is_valid = eLazyBoolCalculate;
     m_qGDBServerVersion_is_valid = eLazyBoolCalculate;
     m_supports_alloc_dealloc_memory = eLazyBoolCalculate;
     m_supports_memory_region_info = eLazyBoolCalculate;
     m_prepare_for_reg_writing_reply = eLazyBoolCalculate;
     m_attach_or_wait_reply = eLazyBoolCalculate;
     m_avoid_g_packets = eLazyBoolCalculate;
+    m_supports_multiprocess = eLazyBoolCalculate;
     m_supports_qXfer_auxv_read = eLazyBoolCalculate;
     m_supports_qXfer_libraries_read = eLazyBoolCalculate;
     m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;
     m_supports_qXfer_features_read = eLazyBoolCalculate;
     m_supports_qXfer_memory_map_read = eLazyBoolCalculate;
     m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
     m_supports_qProcessInfoPID = true;
     m_supports_qfProcessInfo = true;

 void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   // Clear out any capabilities we expect to see in the qSupported response
   m_supports_qXfer_auxv_read = eLazyBoolNo;
   m_supports_qXfer_libraries_read = eLazyBoolNo;
   m_supports_qXfer_libraries_svr4_read = eLazyBoolNo;
   m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
+  m_supports_multiprocess = eLazyBoolNo;
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
                                   // not, we assume no limit
 
   // build the qSupported packet
-  std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc"};
+  std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc",
+                                       "multiprocess+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {
     packet.PutCString(i == 0 ? ":" : ";");
     packet.PutCString(features[i]);
   }
 
   StringExtractorGDBRemote response;
if (SendPacketAndWaitForResponse(packet.GetString(), response,
     else
       m_supports_qEcho = eLazyBoolNo;
 
     if (::strstr(response_cstr, "QPassSignals+"))
       m_supports_QPassSignals = eLazyBoolYes;
     else
       m_supports_QPassSignals = eLazyBoolNo;
 
+    if (::strstr(response_cstr, "multiprocess+"))
+      m_supports_multiprocess = eLazyBoolYes;
+    else
+      m_supports_multiprocess = eLazyBoolNo;
+
     const char *packet_size_str = ::strstr(response_cstr, "PacketSize=");
     if (packet_size_str) {
       StringExtractorGDBRemote packet_response(packet_size_str +
                                                strlen("PacketSize="));
       m_max_packet_size =
           packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
       if (m_max_packet_size == 0) {
         m_max_packet_size = UINT64_MAX; // Must have been a garbled response
if (SendPacketAndWaitForResponse("qC", response, false) ==
           }
         }
       }
     }
 
     // If we don't get a response for $qC, check if $qfThreadID gives us a
     // result.
     if (m_curr_pid == LLDB_INVALID_PROCESS_ID) {
-      std::vector<lldb::tid_t> thread_ids;
       bool sequence_mutex_unavailable;
-      size_t size;
-      size = GetCurrentThreadIDs(thread_ids, sequence_mutex_unavailable);
-      if (size && !sequence_mutex_unavailable) {
-        m_curr_pid = thread_ids.front();
+      auto ids = GetCurrentProcessAndThreadIDs(sequence_mutex_unavailable);
+      if (!ids.empty() && !sequence_mutex_unavailable) {
+        // If server returned an explicit PID, use that.
+        m_curr_pid = ids.front().first;
+        // Otherwise, use the TID of the first thread (Linux hack).
+        if (m_curr_pid == LLDB_INVALID_PROCESS_ID)
+          m_curr_pid = ids.front().second;
         m_curr_pid_is_valid = eLazyBoolYes;
         return m_curr_pid;
       }
     }
   }
 
   return LLDB_INVALID_PROCESS_ID;
 }

 }
 
 uint32_t GDBRemoteCommunicationClient::GetGDBServerProgramVersion() {
   if (GetGDBServerVersion())
     return m_gdb_server_version;
   return 0;
 }
 
 bool GDBRemoteCommunicationClient::GetDefaultThreadId(lldb::tid_t &tid) {
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse("qC", response, false) !=
       PacketResult::Success)
     return false;
 
   if (!response.IsNormalResponse())
     return false;
 
-  if (response.GetChar() == 'Q' && response.GetChar() == 'C')
-    tid = response.GetHexMaxU64(true, -1);
+  if (response.GetChar() == 'Q' && response.GetChar() == 'C') {
+    auto pid_tid = response.GetPidTid(0);
+    if (!pid_tid)
+      return false;
+
+    lldb::pid_t pid = pid_tid->first;
+    // invalid
+    if (pid == StringExtractorGDBRemote::AllProcesses)
+      return false;
+
+    // if we get pid as well, update m_curr_pid
+    if (pid != 0) {
+      m_curr_pid = pid;
+      m_curr_pid_is_valid = eLazyBoolYes;
+    }
+    tid = pid_tid->second;
+  }
 
   return true;
 }
 
 static void ParseOSType(llvm::StringRef value, std::string &os_name,
                         std::string &environment) {
   if (value.equals("iossimulator") || value.equals("tvossimulator") ||
       value.equals("watchossimulator")) {
if (response.IsUnsupportedResponse()) {
         return UINT8_MAX;
       }
     }
   }
   // Signal generic failure
   return UINT8_MAX;
 }
 
-size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs(
-    std::vector<lldb::tid_t> &thread_ids, bool &sequence_mutex_unavailable) {
-  thread_ids.clear();
+std::vector<std::pair<lldb::pid_t, lldb::tid_t>>
+GDBRemoteCommunicationClient::GetCurrentProcessAndThreadIDs(
+    bool &sequence_mutex_unavailable) {
+  std::vector<std::pair<lldb::pid_t, lldb::tid_t>> ids;
 
   Lock lock(*this, false);
   if (lock) {
     sequence_mutex_unavailable = false;
     StringExtractorGDBRemote response;
 
     PacketResult packet_result;
     for (packet_result =
              SendPacketAndWaitForResponseNoLock("qfThreadInfo", response);
          packet_result == PacketResult::Success && response.IsNormalResponse();
          packet_result =
              SendPacketAndWaitForResponseNoLock("qsThreadInfo", response)) {
       char ch = response.GetChar();
       if (ch == 'l')
         break;
       if (ch == 'm') {
         do {
-          tid_t tid = response.GetHexMaxU64(false, LLDB_INVALID_THREAD_ID);
+          auto pid_tid = response.GetPidTid(LLDB_INVALID_PROCESS_ID);
+          if (!pid_tid)
+            return {};
 
-          if (tid != LLDB_INVALID_THREAD_ID) {
-            thread_ids.push_back(tid);
-          }
+          ids.push_back(pid_tid.getValue());
           ch = response.GetChar(); // Skip the command separator
         } while (ch == ',');       // Make sure we got a comma separator
       }
     }
 
     /*
      * Connected bare-iron target (like YAMON gdb-stub) may not have support for
      * qProcessInfo, qC and qfThreadInfo packets. The reply from '?' packet
      * could
      * be as simple as 'S05'. There is no packet which can give us pid and/or
      * tid.
      * Assume pid=tid=1 in such cases.
      */
     if ((response.IsUnsupportedResponse() || response.IsNormalResponse()) &&
-        thread_ids.size() == 0 && IsConnected()) {
-      thread_ids.push_back(1);
+        ids.size() == 0 && IsConnected()) {
+      ids.emplace_back(1, 1);
     }
   } else {
     Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS |
                                                            GDBR_LOG_PACKETS));
     LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending "
                   "packet 'qfThreadInfo'");
     sequence_mutex_unavailable = true;
   }
+
+  return ids;
+}
+
+size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs(
+    std::vector<lldb::tid_t> &thread_ids, bool &sequence_mutex_unavailable) {
+  lldb::pid_t pid = GetCurrentProcessID();
+  thread_ids.clear();
+
+  auto ids = GetCurrentProcessAndThreadIDs(sequence_mutex_unavailable);
+  if (ids.empty() || sequence_mutex_unavailable)
+    return 0;
+
+  for (auto id : ids) {
+    // skip threads that do not belong to the current process
+    if (id.first != LLDB_INVALID_PROCESS_ID && id.first != pid)
+      continue;
+    if (id.second != LLDB_INVALID_THREAD_ID &&
+        id.second != StringExtractorGDBRemote::AllThreads)
+      thread_ids.push_back(id.second);
+  }
+
   return thread_ids.size();
 }
 
 lldb::addr_t GDBRemoteCommunicationClient::GetShlibInfoAddr() {
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse("qShlibInfoAddr", response, false) !=
           PacketResult::Success ||
       !response.IsNormalResponse())

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

protected:
   StructuredData::ObjectSP GetExtendedInfoForThread(lldb::tid_t tid);
 
   void GetMaxMemorySize();
 
   bool CalculateThreadStopInfo(ThreadGDBRemote *thread);
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote &packet);
 
   bool StartAsyncThread();
 
   void StopAsyncThread();
 
   static lldb::thread_result_t AsyncThread(void *arg);

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

 }
 
 void ProcessGDBRemote::ClearThreadIDList() {
   std::lock_guard<std::recursive_mutex> guard(m_thread_list_real.GetMutex());
   m_thread_ids.clear();
   m_thread_pcs.clear();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value) {
+size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
+    llvm::StringRef value) {
   m_thread_ids.clear();
-  size_t comma_pos;
-  lldb::tid_t tid;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-    value[comma_pos] = '\0';
-    // thread in big endian hex
-    tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-    if (tid != LLDB_INVALID_THREAD_ID)
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+  StringExtractorGDBRemote thread_ids{value};
+
+  do {
+    auto pid_tid = thread_ids.GetPidTid(pid);
+    if (pid_tid && pid_tid->first == pid) {
+      lldb::tid_t tid = pid_tid->second;
+      if (tid != LLDB_INVALID_THREAD_ID &&
+          tid != StringExtractorGDBRemote::AllProcesses)
         m_thread_ids.push_back(tid);
-    value.erase(0, comma_pos + 1);
     }
-  tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-  if (tid != LLDB_INVALID_THREAD_ID)
-    m_thread_ids.push_back(tid);
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
 size_t
 ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(std::string &value) {
   m_thread_pcs.clear();
   size_t comma_pos;
   lldb::addr_t pc;
   while ((comma_pos = value.find(',')) != std::string::npos) {
     value[comma_pos] = '\0';
     pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
     if (pc != LLDB_INVALID_ADDRESS)
       m_thread_pcs.push_back(pc);
     value.erase(0, comma_pos + 1);
   }
   pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != LLDB_INVALID_THREAD_ID)
+  if (pc != LLDB_INVALID_ADDRESS)
     m_thread_pcs.push_back(pc);
   return m_thread_pcs.size();
 }
 
 bool ProcessGDBRemote::UpdateThreadIDList() {
   std::lock_guard<std::recursive_mutex> guard(m_thread_list_real.GetMutex());
 
   if (m_jthreadsinfo_sp) {
ProcessGDBRemote::SetThreadStopInfo(StructuredData::Dictionary *thread_dict) {
   return SetThreadStopInfo(tid, expedited_register_map, signo, thread_name,
                            reason, description, exc_type, exc_data,
                            thread_dispatch_qaddr, queue_vars_valid,
                            associated_with_dispatch_queue, dispatch_queue_t,
                            queue_name, queue_kind, queue_serial_number);
 }
 
 StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
   stop_packet.SetFilePos(0);
   const char stop_type = stop_packet.GetChar();
   switch (stop_type) {
   case 'T':
   case 'S': {
     // This is a bit of a hack, but is is required. If we did exec, we need to
     // clear our thread lists and also know to rebuild our dynamic register
     // info before we lookup and threads and populate the expedited register
     // values so we need to know this right away so we can cleanup and update
     // our registers.
     const uint32_t stop_id = GetStopID();
     if (stop_id == 0) {
       // Our first stop, make sure we have a process ID, and also make sure we
       // know about our registers
-      if (GetID() == LLDB_INVALID_PROCESS_ID) {
-        lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
-        if (pid != LLDB_INVALID_PROCESS_ID)
-          SetID(pid);
-      }
+      if (GetID() == LLDB_INVALID_PROCESS_ID && pid != LLDB_INVALID_PROCESS_ID)
       BuildDynamicRegisterInfo(true);
     }
     // Stop with signal and thread info
+    lldb::pid_t stop_pid = LLDB_INVALID_PROCESS_ID;
     lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
     const uint8_t signo = stop_packet.GetHexU8();
     llvm::StringRef key;
     llvm::StringRef value;
     std::string thread_name;
     std::string reason;
     std::string description;
     uint32_t exc_type = 0;
while (stop_packet.GetNameColonValue(key, value)) {
         // exception type in big endian hex
         value.getAsInteger(16, exc_type);
       } else if (key.compare("medata") == 0) {
         // exception data in big endian hex
         uint64_t x;
         value.getAsInteger(16, x);
         exc_data.push_back(x);
       } else if (key.compare("thread") == 0) {
-        // thread in big endian hex
-        if (value.getAsInteger(16, tid))
+        // thread-id
+        StringExtractorGDBRemote thread_id{value};
+        auto pid_tid = thread_id.GetPidTid(pid);
+        if (pid_tid) {
+          stop_pid = pid_tid->first;
+          tid = pid_tid->second;
+        } else
           tid = LLDB_INVALID_THREAD_ID;
       } else if (key.compare("threads") == 0) {
         std::lock_guard<std::recursive_mutex> guard(
             m_thread_list_real.GetMutex());
-
-        m_thread_ids.clear();
-        // A comma separated list of all threads in the current
-        // process that includes the thread for this stop reply packet
-        lldb::tid_t tid;
-        while (!value.empty()) {
-          llvm::StringRef tid_str;
-          std::tie(tid_str, value) = value.split(',');
-          if (tid_str.getAsInteger(16, tid))
-            tid = LLDB_INVALID_THREAD_ID;
-          m_thread_ids.push_back(tid);
-        }
+        UpdateThreadIDsFromStopReplyThreadsValue(value);
       } else if (key.compare("thread-pcs") == 0) {
         m_thread_pcs.clear();
         // A comma separated list of all threads in the current
         // process that includes the thread for this stop reply packet
         lldb::addr_t pc;
         while (!value.empty()) {
           llvm::StringRef pc_str;
           std::tie(pc_str, value) = value.split(',');
while (stop_packet.GetNameColonValue(key, value)) {
         }
       } else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
         uint32_t reg = UINT32_MAX;
         if (!key.getAsInteger(16, reg))
           expedited_register_map[reg] = std::string(std::move(value));
       }
     }
 
+    if (stop_pid != LLDB_INVALID_PROCESS_ID && stop_pid != pid) {
+      Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+      LLDB_LOG(log,
+               "Received stop for incorrect PID = {0} (inferior PID = {1})",
+               stop_pid, pid);
+      return eStateInvalid;
+    }
+
     if (tid == LLDB_INVALID_THREAD_ID) {
       // A thread id may be invalid if the response is old style 'S' packet
       // which does not provide the
       // thread information. So update the thread list and choose the first
       // one.
       UpdateThreadIDList();
 
       if (!m_thread_ids.empty()) {

lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py

+from __future__ import print_function
+import lldb
+import unittest
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestMultiprocess(GDBRemoteTestBase):
+    def test_qfThreadInfo(self):
+        class MyResponder(MockGDBServerResponder):
+            def qfThreadInfo(self):
+                return "mp400.10200,p400.10204,p401.10300,p400.10208"
+
+        self.server.responder = MyResponder()
+        target = self.dbg.CreateTarget('')
+        if self.TraceOn():
+          self.runCmd("log enable gdb-remote packets")
+          self.addTearDownHook(
+                lambda: self.runCmd("log disable gdb-remote packets"))
+        process = self.connect(target)
+        self.assertEqual(process.id, 0x400)
+        self.assertEqual(
+            [process.threads[i].id for i in range(process.num_threads)],
+            [0x10200, 0x10204, 0x10208])
+
+    def test_stop_reason(self):
+        class MyResponder(MockGDBServerResponder):
+            def qfThreadInfo(self):
+                return "mp400.10200,p400.10204"
+
+            def cont(self):
+                return "S02thread:p400.10200;"
+
+        self.server.responder = MyResponder()
+        target = self.dbg.CreateTarget('')
+        if self.TraceOn():
+          self.runCmd("log enable gdb-remote packets")
+          self.addTearDownHook(
+                lambda: self.runCmd("log disable gdb-remote packets"))
+        process = self.connect(target)
+        process.Continue()
+        self.assertEqual(process.GetThreadByID(0x10200).stop_reason,
+                         lldb.eStopReasonSignal)
+        self.assertEqual(process.GetThreadByID(0x10204).stop_reason,
+                         lldb.eStopReasonNone)