diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -155,6 +155,9 @@ PacketResult Handle_QListThreadsInStopReply(StringExtractorGDBRemote &packet); + PacketResult ResumeProcess(NativeProcessProtocol &process, + const ResumeActionList &actions); + PacketResult Handle_C(StringExtractorGDBRemote &packet); PacketResult Handle_c(StringExtractorGDBRemote &packet); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -1512,6 +1512,30 @@ return SendOKResponse(); } +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::ResumeProcess( + NativeProcessProtocol &process, const ResumeActionList &actions) { + Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread); + + // In non-stop protocol mode, the process could be running already. + // We do not support resuming threads independently, so just error out. + if (!process.CanResume()) { + LLDB_LOG(log, "process {0} cannot be resumed (state={1})", process.GetID(), + process.GetState()); + return SendErrorResponse(0x37); + } + + Status error = process.Resume(actions); + if (error.Fail()) { + LLDB_LOG(log, "process {0} failed to resume: {1}", process.GetID(), error); + return SendErrorResponse(GDBRemoteServerError::eErrorResume); + } + + LLDB_LOG(log, "process {0} resumed", process.GetID()); + + return PacketResult::Success; +} + GDBRemoteCommunication::PacketResult GDBRemoteCommunicationServerLLGS::Handle_C(StringExtractorGDBRemote &packet) { Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread); @@ -1547,6 +1571,14 @@ packet, "unexpected content after $C{signal-number}"); } + // In non-stop protocol mode, the process could be running already. + // We do not support resuming threads independently, so just error out. + if (!m_continue_process->CanResume()) { + LLDB_LOG(log, "process cannot be resumed (state={0})", + m_continue_process->GetState()); + return SendErrorResponse(0x37); + } + ResumeActionList resume_actions(StateType::eStateRunning, LLDB_INVALID_SIGNAL_NUMBER); Status error; @@ -1580,14 +1612,11 @@ } } - // Resume the threads. - error = m_continue_process->Resume(resume_actions); - if (error.Fail()) { - LLDB_LOG(log, "failed to resume threads for process {0}: {1}", - m_continue_process->GetID(), error); - - return SendErrorResponse(0x38); - } + // NB: this checks CanResume() twice but using a single code path for + // resuming still seems worth it. + PacketResult resume_res = ResumeProcess(*m_continue_process, resume_actions); + if (resume_res != PacketResult::Success) + return resume_res; // Don't send an "OK" packet, except in non-stop mode; // otherwise, the response is the stopped/exited message. @@ -1622,14 +1651,9 @@ ResumeActionList actions(StateType::eStateRunning, LLDB_INVALID_SIGNAL_NUMBER); - Status error = m_continue_process->Resume(actions); - if (error.Fail()) { - LLDB_LOG(log, "c failed for process {0}: {1}", m_continue_process->GetID(), - error); - return SendErrorResponse(GDBRemoteServerError::eErrorResume); - } - - LLDB_LOG(log, "continued process {0}", m_continue_process->GetID()); + PacketResult resume_res = ResumeProcess(*m_continue_process, actions); + if (resume_res != PacketResult::Success) + return resume_res; return SendContinueSuccessResponse(); } @@ -1643,6 +1667,18 @@ return SendPacketNoLock(response.GetString()); } +static bool ResumeActionListStopsAllThreads(ResumeActionList &actions) { + // We're doing a stop-all if and only if our only action is a "t" for all + // threads. + if (const ResumeAction *default_action = + actions.GetActionForThread(LLDB_INVALID_THREAD_ID, false)) { + if (default_action->state == eStateSuspended && actions.GetSize() == 1) + return true; + } + + return false; +} + GDBRemoteCommunication::PacketResult GDBRemoteCommunicationServerLLGS::Handle_vCont( StringExtractorGDBRemote &packet) { @@ -1664,9 +1700,6 @@ // Move past the ';', then do a simple 's'. packet.SetFilePos(packet.GetFilePos() + 1); return Handle_s(packet); - } else if (m_non_stop && ::strcmp(packet.Peek(), ";t") == 0) { - // TODO: add full support for "t" action - return SendOKResponse(); } std::unordered_map thread_actions; @@ -1733,6 +1766,12 @@ tid = pid_tid->second; } + if (thread_action.state == eStateSuspended && + tid != StringExtractorGDBRemote::AllThreads) { + return SendIllFormedResponse( + packet, "'t' action not supported for individual threads"); + } + if (pid == StringExtractorGDBRemote::AllProcesses) { if (m_debugged_processes.size() > 1) return SendIllFormedResponse( @@ -1765,13 +1804,43 @@ return SendErrorResponse(GDBRemoteServerError::eErrorResume); } - Status error = process_it->second.process_up->Resume(x.second); - if (error.Fail()) { - LLDB_LOG(log, "vCont failed for process {0}: {1}", x.first, error); - return SendErrorResponse(GDBRemoteServerError::eErrorResume); - } + // There are four possible scenarios here. These are: + // 1. vCont on a stopped process that resumes at least one thread. + // In this case, we call Resume(). + // 2. vCont on a stopped process that leaves all threads suspended. + // A no-op. + // 3. vCont on a running process that requests suspending all + // running threads. In this case, we call Interrupt(). + // 4. vCont on a running process that requests suspending a subset + // of running threads or resuming a subset of suspended threads. + // Since we do not support full nonstop mode, this is unsupported + // and we return an error. + + assert(process_it->second.process_up); + if (ResumeActionListStopsAllThreads(x.second)) { + if (process_it->second.process_up->IsRunning()) { + assert(m_non_stop); + + Status error = process_it->second.process_up->Interrupt(); + if (error.Fail()) { + LLDB_LOG(log, "vCont failed to halt process {0}: {1}", x.first, + error); + return SendErrorResponse(GDBRemoteServerError::eErrorResume); + } + + LLDB_LOG(log, "halted process {0}", x.first); - LLDB_LOG(log, "continued process {0}", x.first); + // hack to avoid enabling stdio forwarding after stop + // TODO: remove this when we improve stdio forwarding for nonstop + assert(thread_actions.size() == 1); + return SendOKResponse(); + } + } else { + PacketResult resume_res = + ResumeProcess(*process_it->second.process_up, x.second); + if (resume_res != PacketResult::Success) + return resume_res; + } } return SendContinueSuccessResponse(); @@ -2940,15 +3009,10 @@ // All other threads stop while we're single stepping a thread. actions.SetDefaultThreadActionIfNeeded(eStateStopped, 0); - Status error = m_continue_process->Resume(actions); - if (error.Fail()) { - LLDB_LOGF(log, - "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64 - " tid %" PRIu64 " Resume() failed with error: %s", - __FUNCTION__, m_continue_process->GetID(), tid, - error.AsCString()); - return SendErrorResponse(0x49); - } + + PacketResult resume_res = ResumeProcess(*m_continue_process, actions); + if (resume_res != PacketResult::Success) + return resume_res; // No response here, unless in non-stop mode. // Otherwise, the stop or exit will come from the resulting action. diff --git a/lldb/test/API/tools/lldb-server/TestNonStop.py b/lldb/test/API/tools/lldb-server/TestNonStop.py --- a/lldb/test/API/tools/lldb-server/TestNonStop.py +++ b/lldb/test/API/tools/lldb-server/TestNonStop.py @@ -168,3 +168,137 @@ "send packet: $OK#00", ], True) self.expect_gdbremote_sequence() + + def multiple_resume_test(self, second_command): + self.build() + self.set_inferior_startup_launch() + procs = self.prep_debug_monitor_and_inferior( + inferior_args=["sleep:15"]) + self.test_sequence.add_log_lines( + ["read packet: $QNonStop:1#00", + "send packet: $OK#00", + "read packet: $c#63", + "send packet: $OK#00", + "read packet: ${}#00".format(second_command), + "send packet: $E37#00", + ], True) + self.expect_gdbremote_sequence() + + @add_test_categories(["llgs"]) + def test_multiple_C(self): + self.multiple_resume_test("C05") + + @add_test_categories(["llgs"]) + def test_multiple_c(self): + self.multiple_resume_test("c") + + @add_test_categories(["llgs"]) + def test_multiple_s(self): + self.multiple_resume_test("s") + + @expectedFailureAll(archs=["arm"]) # TODO + @expectedFailureAll(archs=["aarch64"], + bugnumber="https://github.com/llvm/llvm-project/issues/56268") + @add_test_categories(["llgs"]) + def test_multiple_vCont(self): + self.build() + self.set_inferior_startup_launch() + procs = self.prep_debug_monitor_and_inferior( + inferior_args=["thread:new", "trap", "sleep:15"]) + self.test_sequence.add_log_lines( + ["read packet: $QNonStop:1#00", + "send packet: $OK#00", + "read packet: $c#63", + "send packet: $OK#00", + {"direction": "send", + "regex": r"^%Stop:T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);", + "capture": {1: "tid1"}, + }, + "read packet: $vStopped#63", + {"direction": "send", + "regex": r"^[$]T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);", + "capture": {1: "tid2"}, + }, + "read packet: $vStopped#63", + "send packet: $OK#00", + ], True) + ret = self.expect_gdbremote_sequence() + + self.reset_test_sequence() + self.test_sequence.add_log_lines( + ["read packet: $vCont;c:{}#00".format(ret["tid1"]), + "send packet: $OK#00", + "read packet: $vCont;c:{}#00".format(ret["tid2"]), + "send packet: $E37#00", + ], True) + self.expect_gdbremote_sequence() + + @add_test_categories(["llgs"]) + def test_vCont_then_stop(self): + self.build() + self.set_inferior_startup_launch() + procs = self.prep_debug_monitor_and_inferior( + inferior_args=["sleep:15"]) + self.test_sequence.add_log_lines( + ["read packet: $QNonStop:1#00", + "send packet: $OK#00", + "read packet: $c#63", + "send packet: $OK#00", + "read packet: $vCont;t#00", + "send packet: $OK#00", + ], True) + self.expect_gdbremote_sequence() + + def vCont_then_partial_stop_test(self, run_both): + self.build() + self.set_inferior_startup_launch() + procs = self.prep_debug_monitor_and_inferior( + inferior_args=["thread:new", "trap", "sleep:15"]) + self.test_sequence.add_log_lines( + ["read packet: $QNonStop:1#00", + "send packet: $OK#00", + "read packet: $c#63", + "send packet: $OK#00", + {"direction": "send", + "regex": r"^%Stop:T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);", + "capture": {1: "tid1"}, + }, + "read packet: $vStopped#63", + {"direction": "send", + "regex": r"^[$]T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);", + "capture": {1: "tid2"}, + }, + "read packet: $vStopped#63", + "send packet: $OK#00", + ], True) + ret = self.expect_gdbremote_sequence() + + self.reset_test_sequence() + if run_both: + self.test_sequence.add_log_lines( + ["read packet: $vCont;c#00", + ], True) + else: + self.test_sequence.add_log_lines( + ["read packet: $vCont;c:{}#00".format(ret["tid1"]), + ], True) + self.test_sequence.add_log_lines( + ["send packet: $OK#00", + "read packet: $vCont;t:{}#00".format(ret["tid2"]), + "send packet: $E03#00", + ], True) + self.expect_gdbremote_sequence() + + @expectedFailureAll(archs=["arm"]) # TODO + @expectedFailureAll(archs=["aarch64"], + bugnumber="https://github.com/llvm/llvm-project/issues/56268") + @add_test_categories(["llgs"]) + def test_vCont_then_partial_stop(self): + self.vCont_then_partial_stop_test(False) + + @expectedFailureAll(archs=["arm"]) # TODO + @expectedFailureAll(archs=["aarch64"], + bugnumber="https://github.com/llvm/llvm-project/issues/56268") + @add_test_categories(["llgs"]) + def test_vCont_then_partial_stop_run_both(self): + self.vCont_then_partial_stop_test(True) diff --git a/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py --- a/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py +++ b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py @@ -75,54 +75,3 @@ main_thread, "c:{:x};c:{:x};t".format(*all_subthreads_list[:2])), set(all_subthreads_list[:2])) - - @skipIfWindows - @add_test_categories(["llgs"]) - def test_vCont_txc(self): - main_thread, all_subthreads_list = ( - self.start_vCont_run_subset_of_threads_test()) - # stop one thread explicitly, resume others - self.assertEqual( - self.continue_and_get_threads_running( - main_thread, - "t:{:x};c".format(all_subthreads_list[-1])), - set(all_subthreads_list[:2])) - - @skipIfWindows - @add_test_categories(["llgs"]) - def test_vCont_cxtxc(self): - main_thread, all_subthreads_list = ( - self.start_vCont_run_subset_of_threads_test()) - # resume one thread explicitly, stop one explicitly, - # resume others - self.assertEqual( - self.continue_and_get_threads_running( - main_thread, - "c:{:x};t:{:x};c".format(*all_subthreads_list[-2:])), - set(all_subthreads_list[:2])) - - @skipIfWindows - @add_test_categories(["llgs"]) - def test_vCont_txcx(self): - main_thread, all_subthreads_list = ( - self.start_vCont_run_subset_of_threads_test()) - # resume one thread explicitly, stop one explicitly, - # stop others implicitly - self.assertEqual( - self.continue_and_get_threads_running( - main_thread, - "t:{:x};c:{:x}".format(*all_subthreads_list[:2])), - set(all_subthreads_list[1:2])) - - @skipIfWindows - @add_test_categories(["llgs"]) - def test_vCont_txcxt(self): - main_thread, all_subthreads_list = ( - self.start_vCont_run_subset_of_threads_test()) - # resume one thread explicitly, stop one explicitly, - # stop others explicitly - self.assertEqual( - self.continue_and_get_threads_running( - main_thread, - "t:{:x};c:{:x};t".format(*all_subthreads_list[:2])), - set(all_subthreads_list[1:2]))