[lldb] [llgs] Implement the vKill packet

[mgorny]

Summary

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

[mgorny]

Remove leftover exit_regex from test.

[mgorny]

Rebase and update for changes in D127500.

[mgorny]

Rebase.

[labath]

Do we need the kill command to be asynchronous (return before the process actually dies) for anything. Could we just make it synchronous instead (call wait inside the process plugin, and return the wait result)?

[mgorny]

In D127667#3603910, @labath wrote:

Do we need the kill command to be asynchronous (return before the process actually dies) for anything. Could we just make it synchronous instead (call wait inside the process plugin, and return the wait result)?

Well, I have two concerns with that:

1. I have no clue about Windows or an environment to test it, so I wouldn't be able to update the Windows plugin code.
2. I'm a bit concerned about the Linux PTRACE_EVENT_EXIT handling. I mean, I suppose it's unlikely for something to go wrong there and cause us to hang but I'm not sure.

[labath]

Yeah, the pre-exit event makes this somewhat tricky. Let's stick with this then.

lldb/include/lldb/Utility/StringExtractorGDBRemote.h

enum ServerPacketType {
     eServerPacketType_jModulesInfo,
 
     eServerPacketType_vAttach,
     eServerPacketType_vAttachWait,
     eServerPacketType_vAttachOrWait,
     eServerPacketType_vAttachName,
     eServerPacketType_vCont,
     eServerPacketType_vCont_actions, // vCont?
+    eServerPacketType_vKill,
     eServerPacketType_vRun,
 
     eServerPacketType_stop_reason, // '?'
 
     eServerPacketType_c,
     eServerPacketType_C,
     eServerPacketType_D,
     eServerPacketType_g,

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

 //===-- GDBRemoteCommunicationServerLLGS.h ----------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_GDBREMOTECOMMUNICATIONSERVERLLGS_H
 #define LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_GDBREMOTECOMMUNICATIONSERVERLLGS_H
 
 #include <mutex>
 #include <unordered_map>
+#include <unordered_set>
 
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/lldb-private-forward.h"
 
 #include "GDBRemoteCommunicationServerCommon.h"
 
protected:
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
   NativeProcessProtocol *m_current_process;
   NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
   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;
   int m_stdio_handle_refcount = 0;
 
   lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
   llvm::StringMap<std::unique_ptr<llvm::MemoryBuffer>> m_xfer_buffer_map;
   std::mutex m_saved_registers_mutex;
protected:
   PacketResult SendStopReplyPacketForThread(NativeProcessProtocol &process,
                                             lldb::tid_t tid);
 
   PacketResult SendStopReasonForState(NativeProcessProtocol &process,
                                       lldb::StateType process_state);
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vKill(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_qProcessInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qC(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_QSetDisableASLR(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_QSetWorkingDir(StringExtractorGDBRemote &packet);
 

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

void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
   RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_k,
                         [this](StringExtractorGDBRemote packet, Status &error,
                                bool &interrupt, bool &quit) {
                           quit = true;
                           return this->Handle_k(packet);
                         });
 
   RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_vKill,
+      &GDBRemoteCommunicationServerLLGS::Handle_vKill);
+
+  RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
       &GDBRemoteCommunicationServerLLGS::Handle_qSaveCore);
 }
 
 void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo &info) {
   m_process_launch_info = info;
 }
 
if (!wait_status) {
     response.PutChar('E');
     response.PutHex8(GDBRemoteServerError::eErrorExitStatus);
     return SendPacketNoLock(response.GetString());
   }
 
   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))
     response.Format(";process:{0:x-}", process->GetID());
   return SendPacketNoLock(response.GetString());
 }
 
 static void AppendHexValue(StreamString &response, const uint8_t *buf,

 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Exited(
     NativeProcessProtocol *process) {
   assert(process && "process cannot be NULL");
 
   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) {
     LLDB_LOGF(log,
               "GDBRemoteCommunicationServerLLGS::%s failed to send stop "
               "notification for PID %" PRIu64 ", state: eStateExited",
               __FUNCTION__, process->GetID());
   }
 
   if (m_current_process == process)
     m_current_process = nullptr;
   if (m_continue_process == process)
     m_continue_process = nullptr;
   auto process_it = m_debugged_processes.find(process->GetID());
   assert(process_it != m_debugged_processes.end());
   // Keep a reference to prevent the instance from being destroyed before
   // state change notification finishes.
   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();
 
     // We are ready to exit the debug monitor.
     m_exit_now = true;
     m_mainloop.RequestTermination();
   }
GDBRemoteCommunicationServerLLGS::Handle_k(StringExtractorGDBRemote &packet) {
   }
 
   // No OK response for kill packet.
   // return SendOKResponse ();
   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) {
   packet.SetFilePos(::strlen("QSetDisableASLR:"));
   if (packet.GetU32(0))
     m_process_launch_info.GetFlags().Set(eLaunchFlagDisableASLR);
   else
     m_process_launch_info.GetFlags().Clear(eLaunchFlagDisableASLR);
   return SendOKResponse();

lldb/source/Utility/StringExtractorGDBRemote.cpp

if (PACKET_STARTS_WITH("vFile:")) {
       if (PACKET_STARTS_WITH("vAttachOrWait;"))
         return eServerPacketType_vAttachOrWait;
       if (PACKET_STARTS_WITH("vAttachName;"))
         return eServerPacketType_vAttachName;
       if (PACKET_STARTS_WITH("vCont;"))
         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;
     }
     break;
   case '_':
     switch (packet_cstr[1]) {
     case 'M':
       return eServerPacketType__M;

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

def test_kill_all(self):
             {"direction": "send", "regex": exit_regex,
              "capture": {1: "pid1"}},
             {"direction": "send", "regex": exit_regex,
              "capture": {1: "pid2"}},
         ], True)
         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)