mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Implement the support for the vKill packet.  This is the modern packet
used by the GDB Remote Serial Protocol to kill one of the debugged
processes.  Unlike the `k` packet, it has well-defined semantics.

The `vKill` packet takes the PID of the process to kill, and always
replies with an `OK` reply (rather than the exit status, as LLGS does
for `k` packets at the moment).  Additionally, unlike the `k` packet
it does not cause the connection to be terminated once the last process
is killed — the client needs to close it explicitly.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D127667

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -297,3 +297,61 @@
         ret = self.expect_gdbremote_sequence()
         self.assertEqual(set([ret["pid1"], ret["pid2"]]),
                          set([parent_pid, child_pid]))
+
+    def vkill_test(self, kill_parent=False, kill_child=False):
+        assert kill_parent or kill_child
+        self.build()
+        self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+        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)
+        ret = self.expect_gdbremote_sequence()
+        parent_pid = ret["parent_pid"]
+        parent_tid = ret["parent_tid"]
+        child_pid = ret["child_pid"]
+        child_tid = ret["child_tid"]
+        self.reset_test_sequence()
+
+        exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+        if kill_parent:
+            self.test_sequence.add_log_lines([
+                # kill the process
+                "read packet: $vKill;{}#00".format(parent_pid),
+                "send packet: $OK#00",
+            ], True)
+        if kill_child:
+            self.test_sequence.add_log_lines([
+                # kill the process
+                "read packet: $vKill;{}#00".format(child_pid),
+                "send packet: $OK#00",
+            ], True)
+        self.test_sequence.add_log_lines([
+            # check child PID/TID
+            "read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+            "send packet: ${}#00".format("Eff" if kill_child else "OK"),
+            # check parent PID/TID
+            "read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+            "send packet: ${}#00".format("Eff" if kill_parent else "OK"),
+        ], True)
+        self.expect_gdbremote_sequence()
+
+    @add_test_categories(["fork"])
+    def test_vkill_child(self):
+        self.vkill_test(kill_child=True)
+
+    @add_test_categories(["fork"])
+    def test_vkill_parent(self):
+        self.vkill_test(kill_parent=True)
+
+    @add_test_categories(["fork"])
+    def test_vkill_both(self):
+        self.vkill_test(kill_parent=True, kill_child=True)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===================================================================
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -367,6 +367,8 @@
         return eServerPacketType_vCont;
       if (PACKET_MATCHES("vCont?"))
         return eServerPacketType_vCont_actions;
+      if (PACKET_STARTS_WITH("vKill;"))
+        return eServerPacketType_vKill;
       if (PACKET_STARTS_WITH("vRun;"))
         return eServerPacketType_vRun;
     }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -11,6 +11,7 @@
 
 #include <mutex>
 #include <unordered_map>
+#include <unordered_set>
 
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/MainLoop.h"
@@ -96,6 +97,7 @@
   std::unordered_map<lldb::pid_t, std::unique_ptr<NativeProcessProtocol>>
       m_debugged_processes;
   std::unique_ptr<NativeProcessProtocol> m_last_exited_process;
+  std::unordered_set<lldb::pid_t> m_vkilled_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
@@ -123,6 +125,8 @@
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vKill(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_qProcessInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qC(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -232,6 +232,10 @@
                           return this->Handle_k(packet);
                         });
 
+  RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_vKill,
+      &GDBRemoteCommunicationServerLLGS::Handle_vKill);
+
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
       &GDBRemoteCommunicationServerLLGS::Handle_qSaveCore);
@@ -472,6 +476,13 @@
   LLDB_LOG(log, "pid = {0}, returning exit type {1}", process->GetID(),
            *wait_status);
 
+  // if the process was killed through vKill, return "OK"
+  auto kill_found = m_vkilled_processes.find(process->GetID());
+  if (kill_found != m_vkilled_processes.end()) {
+    m_vkilled_processes.erase(kill_found);
+    return SendOKResponse();
+  }
+
   StreamGDBRemote response;
   response.Format("{0:g}", *wait_status);
   if (bool(m_extensions_supported & NativeProcessProtocol::Extension::multiprocess))
@@ -993,6 +1004,9 @@
   Log *log = GetLog(LLDBLog::Process);
   LLDB_LOGF(log, "GDBRemoteCommunicationServerLLGS::%s called", __FUNCTION__);
 
+  bool vkill_found =
+      m_vkilled_processes.find(process->GetID()) != m_vkilled_processes.end();
+
   PacketResult result =
       SendStopReasonForState(*process, StateType::eStateExited);
   if (result != PacketResult::Success) {
@@ -1013,7 +1027,11 @@
   m_last_exited_process = std::move(process_it->second);
   m_debugged_processes.erase(process_it);
 
-  if (m_debugged_processes.empty()) {
+  // We normally terminate if the last process exited, unless it exited
+  // due to vKill.
+  if (m_debugged_processes.empty() && !vkill_found) {
+    LLDB_LOG(log, "Last process exited, we can exit now");
+
     // Close the pipe to the inferior terminal i/o if we launched it and set one
     // up.
     MaybeCloseInferiorTerminalConnection();
@@ -1424,6 +1442,30 @@
   return PacketResult::Success;
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_vKill(
+    StringExtractorGDBRemote &packet) {
+  StopSTDIOForwarding();
+
+  packet.SetFilePos(6); // vKill;
+  uint32_t pid = packet.GetU32(LLDB_INVALID_PROCESS_ID, 16);
+  if (pid == LLDB_INVALID_PROCESS_ID)
+    return SendIllFormedResponse(packet,
+                                 "vKill failed to parse the process id");
+
+  auto it = m_debugged_processes.find(pid);
+  if (it == m_debugged_processes.end())
+    return SendErrorResponse(42);
+
+  Status error = it->second->Kill();
+  if (error.Fail())
+    return SendErrorResponse(error.ToError());
+
+  // OK response is sent when the process dies.
+  m_vkilled_processes.insert(pid);
+  return PacketResult::Success;
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_QSetDisableASLR(
     StringExtractorGDBRemote &packet) {
Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h
===================================================================
--- lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -136,6 +136,7 @@
     eServerPacketType_vAttachName,
     eServerPacketType_vCont,
     eServerPacketType_vCont_actions, // vCont?
+    eServerPacketType_vKill,
     eServerPacketType_vRun,
 
     eServerPacketType_stop_reason, // '?'
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D12... Michał Górny via Phabricator via lldb-commits

Reply via email to