[Lldb-commits] [lldb] e8fe7e9 - [lldb] [llgs] Make `k` kill all processes, and fix multiple exits
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Fri Jun 24 08:20:32 PDT 2022
Author: Michał Górny
Date: 2022-06-24T17:20:23+02:00
New Revision: e8fe7e930a45764cbb88d9c3fa91ef7dc1ebcc97
URL: https://github.com/llvm/llvm-project/commit/e8fe7e930a45764cbb88d9c3fa91ef7dc1ebcc97
DIFF: https://github.com/llvm/llvm-project/commit/e8fe7e930a45764cbb88d9c3fa91ef7dc1ebcc97.diff
LOG: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits
Modify the behavior of the `k` packet to kill all inferiors rather than
just the current one. The specification leaves the exact behavior
of this packet up to the implementation but since vKill is specifically
meant to be used to kill a single process, it seems logical to use `k`
to provide the alternate function of killing all of them.
Move starting stdio forwarding from the "running" response
to the packet handlers that trigger the process to start. This avoids
attempting to start it multiple times when multiple processes are killed
on Linux which implicitly causes LLGS to receive "started" events
for all of them. This is probably also more correct as the ability
to send "O" packets is implied by the continue-like command being issued
(and therefore the client waiting for responses) rather than the start
notification.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D127500
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 1cddb5b29c595..434fb50862f8b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1041,17 +1041,26 @@ void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Exited(
__FUNCTION__, process->GetID());
}
- // Close the pipe to the inferior terminal i/o if we launched it and set one
- // up.
- MaybeCloseInferiorTerminalConnection();
-
- // When running in non-stop mode, wait for the vStopped to clear
- // the notification queue.
- if (!m_non_stop) {
- // We are ready to exit the debug monitor.
- m_exit_now = true;
- m_mainloop.RequestTermination();
- }
+ if (m_current_process == process)
+ m_current_process = nullptr;
+ if (m_continue_process == process)
+ m_continue_process = nullptr;
+
+ lldb::pid_t pid = process->GetID();
+ m_mainloop.AddPendingCallback([this, pid](MainLoopBase &loop) {
+ m_debugged_processes.erase(pid);
+ // When running in non-stop mode, wait for the vStopped to clear
+ // the notification queue.
+ if (m_debugged_processes.empty() && !m_non_stop) {
+ // 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;
+ loop.RequestTermination();
+ }
+ });
}
void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1094,7 +1103,6 @@ void GDBRemoteCommunicationServerLLGS::ProcessStateChanged(
switch (state) {
case StateType::eStateRunning:
- StartSTDIOForwarding();
break;
case StateType::eStateStopped:
@@ -1219,7 +1227,7 @@ void GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
return;
Status error;
- lldbassert(!m_stdio_handle_up);
+ assert(!m_stdio_handle_up);
m_stdio_handle_up = m_mainloop.RegisterReadObject(
m_stdio_communication.GetConnection()->GetReadObject(),
[this](MainLoopBase &) { SendProcessOutput(); }, error);
@@ -1228,10 +1236,7 @@ void GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
// Not much we can do about the failure. Log it and continue without
// forwarding.
if (Log *log = GetLog(LLDBLog::Process))
- LLDB_LOGF(log,
- "GDBRemoteCommunicationServerLLGS::%s Failed to set up stdio "
- "forwarding: %s",
- __FUNCTION__, error.AsCString());
+ LLDB_LOG(log, "Failed to set up stdio forwarding: {0}", error);
}
}
@@ -1416,15 +1421,19 @@ GDBRemoteCommunicationServerLLGS::Handle_k(StringExtractorGDBRemote &packet) {
StopSTDIOForwarding();
- if (!m_current_process) {
+ if (m_debugged_processes.empty()) {
LLDB_LOG(log, "No debugged process found.");
return PacketResult::Success;
}
- Status error = m_current_process->Kill();
- if (error.Fail())
- LLDB_LOG(log, "Failed to kill debugged process {0}: {1}",
- m_current_process->GetID(), error);
+ for (auto it = m_debugged_processes.begin(); it != m_debugged_processes.end();
+ ++it) {
+ LLDB_LOG(log, "Killing process {0}", it->first);
+ Status error = it->second->Kill();
+ if (error.Fail())
+ LLDB_LOG(log, "Failed to kill debugged process {0}: {1}", it->first,
+ error);
+ }
// The response to kill packet is undefined per the spec. LLDB
// follows the same rules as for continue packets, i.e. no response
@@ -1743,10 +1752,6 @@ GDBRemoteCommunicationServerLLGS::Handle_stop_reason(
StringExtractorGDBRemote &packet) {
// Handle the $? gdbremote command.
- // If no process, indicate error
- if (!m_current_process)
- return SendErrorResponse(02);
-
if (m_non_stop) {
// Clear the notification queue first, except for pending exit
// notifications.
@@ -1754,13 +1759,17 @@ GDBRemoteCommunicationServerLLGS::Handle_stop_reason(
return x.front() != 'W' && x.front() != 'X';
});
- // Queue stop reply packets for all active threads. Start with the current
- // thread (for clients that don't actually support multiple stop reasons).
- NativeThreadProtocol *thread = m_current_process->GetCurrentThread();
- if (thread)
- m_stop_notification_queue.push_back(
- PrepareStopReplyPacketForThread(*thread).GetString().str());
- EnqueueStopReplyPackets(thread ? thread->GetID() : LLDB_INVALID_THREAD_ID);
+ if (m_current_process) {
+ // Queue stop reply packets for all active threads. Start with
+ // the current thread (for clients that don't actually support multiple
+ // stop reasons).
+ NativeThreadProtocol *thread = m_current_process->GetCurrentThread();
+ if (thread)
+ m_stop_notification_queue.push_back(
+ PrepareStopReplyPacketForThread(*thread).GetString().str());
+ EnqueueStopReplyPackets(thread ? thread->GetID()
+ : LLDB_INVALID_THREAD_ID);
+ }
// If the notification queue is empty (i.e. everything is running), send OK.
if (m_stop_notification_queue.empty())
@@ -1770,6 +1779,10 @@ GDBRemoteCommunicationServerLLGS::Handle_stop_reason(
return SendPacketNoLock(m_stop_notification_queue.front());
}
+ // If no process, indicate error
+ if (!m_current_process)
+ return SendErrorResponse(02);
+
return SendStopReasonForState(*m_current_process,
m_current_process->GetState(),
/*force_synchronous=*/true);
@@ -4059,6 +4072,8 @@ void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::SendContinueSuccessResponse() {
+ // TODO: how to handle forwarding in non-stop mode?
+ StartSTDIOForwarding();
return m_non_stop ? SendOKResponse() : PacketResult::Success;
}
diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index bbd5912edf7d1..8a3474ca78df1 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -262,3 +262,37 @@ def test_detach_all(self):
"send packet: $Eff#00",
], True)
self.expect_gdbremote_sequence()
+
+ @add_test_categories(["fork"])
+ def test_kill_all(self):
+ 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"]
+ child_pid = ret["child_pid"]
+ self.reset_test_sequence()
+
+ exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+ self.test_sequence.add_log_lines([
+ # kill all processes
+ "read packet: $k#00",
+ {"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]))
More information about the lldb-commits
mailing list