[lldb] [llgs] Support multiprocess in qfThreadInfo

[mgorny]

Summary

Update the qfThreadInfo handler to report threads of all debugged
processes and include PIDs when in multiprocess mode.

Sponsored by: The FreeBSD Foundation

[labath]

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

2011

without multiprocess extensions, we should never have more than one process, right? Could we just unconditionally use the multiprocess loop here (perhaps with an assert(m_debugged_processes.size() == 1 || multiprocess)) ?

[mgorny]

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

2011

Yes, that makes sense. We enable fork & vfork only if multiprocess is supported, so that shouldn't happen unless we deal with broken Process plugin.

[mgorny]

Simplify to use a single loop for both multiprocess and non-multiprocess modes. Replace the static function with include_pid with a class method, to prepare for adding a new helper to print PID+TID.

[cmtice]

Just FYI, this commit appears to cause lldb//test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py to fail. The error I'm seeing is:

Traceback (most recent call last):

File "../lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py", line 72, in test_target_auto_install_main_executable

self.expect("process launch", substrs=["exited with status = 74"])

File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2295, in expect

self.runCmd(

File ".../lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1998, in runCmd

self.assertTrue(self.res.Succeeded(),

AssertionError: False is not True : Command 'process launch
Error output:
error: failed to get reply to handshake packet within timeout of 0.0 seconds
' did not return successfully

[mgorny]

Thanks for the notice. I was pretty sure it failed to me prior to my changes but it is possible that I read the output wrong, so I'm going to check it now.

[mgorny]

I'm sorry, it is indeed my fault. I'm going to try to fix it shortly, and if I don't manage to figure it out, I'm going to revert this and the commits depending on it.

[mgorny]

I've reverted the code changes and marked my new tests xfail. I will attempt to find the problem in the new code later. Once again, I'm sorry for missing this.

[mgorny]

Ok, apparently the changes that's breaking this is replacing the OK response with l when there's no debugged process.

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

 def parse_threadinfo_response(response_packet):
     if not response_packet:
         raise Exception("response_packet cannot be None")
 
     # Strip off prefix $ and suffix #xx if present.
     response_packet = _STRIP_COMMAND_PREFIX_M_REGEX.sub("", response_packet)
     response_packet = _STRIP_CHECKSUM_REGEX.sub("", response_packet)
 
-    # Return list of thread ids
-    return [int(thread_id_hex, 16) for thread_id_hex in response_packet.split(
-        ",") if len(thread_id_hex) > 0]
+    for tid in response_packet.split(","):
+        if not tid:
+            continue
+        if tid.startswith("p"):
+            pid, _, tid = tid.partition(".")
+            yield (int(pid[1:], 16), int(tid, 16))
+        else:
+            yield int(tid, 16)
 
 
 def unpack_endian_binary_string(endian, value_string):
     """Unpack a gdb-remote binary (post-unescaped, i.e. not escaped) response to an unsigned int given endianness of the inferior."""
     if not endian:
         raise Exception("endian cannot be None")
     if not value_string or len(value_string) < 1:
         raise Exception("value_string cannot be None or empty")

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

protected:
   PacketResult Handle_vCont(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vCont_actions(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_stop_reason(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qRegisterInfo(StringExtractorGDBRemote &packet);
 
+  void AddProcessThreads(StreamGDBRemote &response,
+                         NativeProcessProtocol &process, bool &had_any);
+
   PacketResult Handle_qfThreadInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qsThreadInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_p(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_P(StringExtractorGDBRemote &packet);
 

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

if (reg_info->invalidate_regs && reg_info->invalidate_regs[0]) {
     response.PutCString("invalidate-regs:");
     CollectRegNums(reg_info->invalidate_regs, response, true);
     response.PutChar(';');
   }
 
   return SendPacketNoLock(response.GetString());
 }
 
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo(
-    StringExtractorGDBRemote &packet) {
+void GDBRemoteCommunicationServerLLGS::AddProcessThreads(
+    StreamGDBRemote &response, NativeProcessProtocol &process, bool &had_any) {
   Log *log = GetLog(LLDBLog::Thread);
 
-  // Fail if we don't have a current process.
-  if (!m_current_process ||
-      (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-    LLDB_LOG(log, "no process ({0}), returning OK",
-             m_current_process ? "invalid process id"
-                               : "null m_current_process");
-    return SendOKResponse();
-  }
-
-  StreamGDBRemote response;
-  response.PutChar('m');
+  lldb::pid_t pid = process.GetID();
+  if (pid == LLDB_INVALID_PROCESS_ID)
+    return;
 
-  LLDB_LOG(log, "starting thread iteration");
+  LLDB_LOG(log, "iterating over threads of process {0}", process.GetID());
   NativeThreadProtocol *thread;
   uint32_t thread_index;
-  for (thread_index = 0,
-      thread = m_current_process->GetThreadAtIndex(thread_index);
-       thread; ++thread_index,
-      thread = m_current_process->GetThreadAtIndex(thread_index)) {
-    LLDB_LOG(log, "iterated thread {0}(tid={2})", thread_index,
+  for (thread_index = 0, thread = process.GetThreadAtIndex(thread_index);
+       thread;
+       ++thread_index, thread = process.GetThreadAtIndex(thread_index)) {
+    LLDB_LOG(log, "iterated thread {0} (tid={1})", thread_index,
              thread->GetID());
-    if (thread_index > 0)
-      response.PutChar(',');
-    response.Printf("%" PRIx64, thread->GetID());
-  }
+    response.PutChar(had_any ? ',' : 'm');
+    if (bool(m_extensions_supported &
+             NativeProcessProtocol::Extension::multiprocess))
+      response.Format("p{0:x-}.", pid);
+    response.Format("{0:x-}", thread->GetID());
+    had_any = true;
+  }
 
-  LLDB_LOG(log, "finished thread iteration");
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo(
+    StringExtractorGDBRemote &packet) {
+  assert(m_debugged_processes.size() == 1 ||
+         bool(m_extensions_supported &
+              NativeProcessProtocol::Extension::multiprocess));
+
+  bool had_any = false;
+  StreamGDBRemote response;
+
+  for (auto &pid_ptr : m_debugged_processes)
+    AddProcessThreads(response, *pid_ptr.second, had_any);
+
+  if (!had_any)
+    response.PutChar('l');
   return SendPacketNoLock(response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_qsThreadInfo(
     StringExtractorGDBRemote &packet) {
   // FIXME for now we return the full thread list in the initial packet and
   // always do nothing here.

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

def test_vCont_all_processes_implicit(self):
         self.test_sequence.add_log_lines([
             # try to resume all processes implicitly
             "read packet: $vCont;c#00",
             "send packet: $E03#00",
         ], True)
         self.expect_gdbremote_sequence()
 
     @add_test_categories(["fork"])
+    def test_threadinfo(self):
+        self.build()
+        self.prep_debug_monitor_and_inferior(
+            inferior_args=["fork",
+                           "thread:new",
+                           "trap",
+                           ])
+        self.add_qSupported_packets(["multiprocess+",
+                                     "fork-events+"])
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn("fork-events+", ret["qSupported_response"])
+        self.reset_test_sequence()
+
+        # continue and expect fork
+        self.test_sequence.add_log_lines([
+            "read packet: $c#00",
+            {"direction": "send", "regex": self.fork_regex.format("fork"),
+             "capture": self.fork_capture},
+        ], True)
+        self.add_threadinfo_collection_packets()
+        ret = self.expect_gdbremote_sequence()
+        pidtids = [
+            (ret["parent_pid"], ret["parent_tid"]),
+            (ret["child_pid"], ret["child_tid"]),
+        ]
+        prev_pidtids = set(self.parse_threadinfo_packets(ret))
+        self.assertEqual(prev_pidtids,
+                         frozenset((int(pid, 16), int(tid, 16))
+                                   for pid, tid in pidtids))
+        self.reset_test_sequence()
+
+        for pidtid in pidtids:
+            self.test_sequence.add_log_lines(
+                ["read packet: $Hcp{}.{}#00".format(*pidtid),
+                 "send packet: $OK#00",
+                 "read packet: $c#00",
+                 {"direction": "send",
+                  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+                  },
+                 ], True)
+            self.add_threadinfo_collection_packets()
+            ret = self.expect_gdbremote_sequence()
+            self.reset_test_sequence()
+            new_pidtids = set(self.parse_threadinfo_packets(ret))
+            added_pidtid = new_pidtids - prev_pidtids
+            prev_pidtids = new_pidtids
+
+            # verify that we've got exactly one new thread, and that
+            # the PID matches
+            self.assertEqual(len(added_pidtid), 1)
+            self.assertEqual(added_pidtid.pop()[0], int(pidtid[0], 16))
+
+        for pidtid in new_pidtids:
+            self.test_sequence.add_log_lines(
+                ["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+                 "send packet: $OK#00",
+                 ], True)
+        self.expect_gdbremote_sequence()
+
+    @add_test_categories(["fork"])
     def test_memory_read_write(self):
         self.build()
         INITIAL_DATA = "Initial message"
         self.prep_debug_monitor_and_inferior(
             inferior_args=["set-message:{}".format(INITIAL_DATA),
                            "get-data-address-hex:g_message",
                            "fork",
                            "print-message:",