[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

1131

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;
   }
 
+  void GetCurrentProcessAndThreadIDs(
+      std::vector<std::pair<lldb::pid_t, lldb::tid_t>> &ids,
+      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();
+      std::vector<std::pair<lldb::pid_t, lldb::tid_t>> ids;
+      GetCurrentProcessAndThreadIDs(ids, 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();
+void GDBRemoteCommunicationClient::GetCurrentProcessAndThreadIDs(
+    std::vector<std::pair<lldb::pid_t, lldb::tid_t>> &ids,
+    bool &sequence_mutex_unavailable) {
+  ids.clear();
 
   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;
   }
+}
+
+size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs(
+    std::vector<lldb::tid_t> &thread_ids, bool &sequence_mutex_unavailable) {
+  lldb::pid_t pid = GetCurrentProcessID();
+  thread_ids.clear();
+
+  std::vector<std::pair<lldb::pid_t, lldb::tid_t>> ids;
+  GetCurrentProcessAndThreadIDs(ids, 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

 #define LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_PROCESSGDBREMOTE_H
 
 #include <atomic>
 #include <map>
 #include <mutex>
 #include <string>
 #include <vector>
 
 #include "lldb/Core/LoadedModuleInfoList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/ThreadSafeValue.h"
 #include "lldb/Host/HostThread.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/ConstString.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

 //===-- ProcessGDBRemote.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/Config.h"
 
 #include <errno.h>
 #include <stdlib.h>
 #if LLDB_ENABLE_POSIX
 #include <netinet/in.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <unistd.h>

 }
 
 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)
+      if (GetID() == LLDB_INVALID_PROCESS_ID && pid != LLDB_INVALID_PROCESS_ID)
           SetID(pid);
-      }
       BuildDynamicRegisterInfo(true);
     }
     // Stop with signal and thread info
     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;
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 && pid_tid->first == pid)
+          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(',');