[lldb] [client] Support for multiprocess extension
authorMichał Górny <mgorny@moritz.systems>
Fri, 12 Mar 2021 08:02:36 +0000 (09:02 +0100)
committerMichał Górny <mgorny@moritz.systems>
Thu, 8 Apr 2021 11:45:07 +0000 (13:45 +0200)
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.

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

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py [new file with mode: 0644]

index 22e05c0..c032fc2 100644 (file)
@@ -89,6 +89,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
       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),
@@ -292,6 +293,7 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
     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;
@@ -342,11 +344,13 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   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) {
@@ -433,6 +437,11 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
     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 +
@@ -741,12 +750,14 @@ lldb::pid_t GDBRemoteCommunicationClient::GetCurrentProcessID(bool allow_lazy) {
     // 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;
       }
@@ -1125,8 +1136,23 @@ bool GDBRemoteCommunicationClient::GetDefaultThreadId(lldb::tid_t &tid) {
   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;
 }
@@ -2766,9 +2792,10 @@ uint8_t GDBRemoteCommunicationClient::SendGDBStoppointTypePacket(
   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) {
@@ -2786,11 +2813,11 @@ size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs(
         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
       }
@@ -2803,10 +2830,10 @@ size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs(
      * 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 |
@@ -2815,6 +2842,28 @@ size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs(
                   "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();
 }
 
index aaa6f78..96519d3 100644 (file)
@@ -366,6 +366,9 @@ public:
     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);
 
index 7363c15..97368d5 100644 (file)
@@ -1487,22 +1487,22 @@ void ProcessGDBRemote::ClearThreadIDList() {
   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)
-      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);
+  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);
+    }
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
@@ -1519,7 +1519,7 @@ ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(std::string &value) {
     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();
 }
@@ -2141,6 +2141,7 @@ ProcessGDBRemote::SetThreadStopInfo(StructuredData::Dictionary *thread_dict) {
 }
 
 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) {
@@ -2155,14 +2156,12 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
     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)
+        SetID(pid);
       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;
@@ -2191,24 +2190,18 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
         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
@@ -2321,6 +2314,14 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
       }
     }
 
+    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
index aafe1c3..032067e 100644 (file)
@@ -335,7 +335,7 @@ protected:
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote &packet);
 
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py b/lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
new file mode 100644 (file)
index 0000000..baf9f9c
--- /dev/null
@@ -0,0 +1,46 @@
+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)