mgorny updated this revision to Diff 439405. mgorny marked an inline comment as done. mgorny added a comment.
Limited variables passed into the lambda. Reworked starting stdio forwarding per the suggestion. TODO: rework stopping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127500/new/ https://reviews.llvm.org/D127500 Files: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.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 @@ -262,3 +262,37 @@ "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])) 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 @@ -1041,17 +1041,26 @@ __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 @@ switch (state) { case StateType::eStateRunning: - StartSTDIOForwarding(); break; case StateType::eStateStopped: @@ -1219,7 +1227,7 @@ 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,14 +1236,27 @@ // 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(); } @@ -1416,15 +1437,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); + } // 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 +1768,6 @@ 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 +1775,17 @@ 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 +1795,10 @@ 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 +4088,8 @@ GDBRemoteCommunication::PacketResult GDBRemoteCommunicationServerLLGS::SendContinueSuccessResponse() { + // TODO: how to handle forwarding in non-stop mode? + StartSTDIOForwarding(); return m_non_stop ? SendOKResponse() : PacketResult::Success; }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits