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])) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
