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.
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.
While at it, fix bugs that caused lldb-server to mishandle kills
and exits when more than one process is debugged. That is:
1. Delay exiting lldb-server until all processes exit, rather than doing that
on first exited process. This requires us to actually remove exited processes
from the process list in order to know whether we have any processes left. The
last exited process instance is moved onto a dedicated variable in order to
prevent it from being destroyed while the notification is still being handled
(i.e. effectively until the next process exits).
2. Fix enabling and disabling stdio forwarding to account for multiple
processes running simultaneously. Now, it is enabled when the first process is
resumed, and remains enabled until the last one stops.
Sponsored by: The FreeBSD Foundation
https://reviews.llvm.org/D127500
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
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
@@ -263,3 +263,44 @@
"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"]
+ 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]+)#.*"
+ self.test_sequence.add_log_lines([
+ # double-check our PIDs
+ "read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+ "send packet: $OK#00",
+ "read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+ "send packet: $OK#00",
+ "read packet: $QEnableErrorStrings#00",
+ "send packet: $OK#00",
+ # kill all processes
+ "read packet: $k#00",
+ {"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid1"}},
+ {"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid2"}},
+ ], True)
+ self.expect_gdbremote_sequence()
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
@@ -95,9 +95,11 @@
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;
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;
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
@@ -1002,13 +1002,26 @@
__FUNCTION__, process->GetID());
}
- // Close the pipe to the inferior terminal i/o if we launched it and set one
- // up.
- MaybeCloseInferiorTerminalConnection();
+ 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);
- // We are ready to exit the debug monitor.
- m_exit_now = true;
- m_mainloop.RequestTermination();
+ if (m_debugged_processes.empty()) {
+ // 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();
+ }
}
void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1171,12 +1184,19 @@
}
void GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
+ Log *log = GetLog(LLDBLog::Process);
+
// Don't forward if not connected (e.g. when attaching).
if (!m_stdio_communication.IsConnected())
return;
+ // If we're not debugging multiple processes, this should be called
+ // once.
+ if (!bool(m_extensions_supported &
+ NativeProcessProtocol::Extension::multiprocess))
+ assert(!m_stdio_handle_up);
+
Status error;
- lldbassert(!m_stdio_handle_up);
m_stdio_handle_up = m_mainloop.RegisterReadObject(
m_stdio_communication.GetConnection()->GetReadObject(),
[this](MainLoopBase &) { SendProcessOutput(); }, error);
@@ -1184,15 +1204,27 @@
if (!m_stdio_handle_up) {
// 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);
}
}
void GDBRemoteCommunicationServerLLGS::StopSTDIOForwarding() {
+ Log *log = GetLog(LLDBLog::Process);
+
+ if (bool(m_extensions_supported &
+ NativeProcessProtocol::Extension::multiprocess)) {
+ // Leave stdio forwarding open if there's at least one running process
+ // still.
+ for (auto &x : m_debugged_processes) {
+ if (x.second->IsRunning()) {
+ LLDB_LOG(log,
+ "process {0} is still running, stdio forwarding stays open",
+ x.first);
+ return;
+ }
+ }
+ }
+
m_stdio_handle_up.reset();
}
@@ -1373,15 +1405,19 @@
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);
+ }
// No OK response for kill packet.
// return SendOKResponse ();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
