Index: lldb/packages/Python/lldbsuite/test/gdbclientutils.py =================================================================== --- lldb/packages/Python/lldbsuite/test/gdbclientutils.py +++ lldb/packages/Python/lldbsuite/test/gdbclientutils.py @@ -210,6 +210,8 @@ return self.vStopped() if packet == "vCtrlC": return self.vCtrlC() + if packet == "vCont;t": + return self.vContT() if packet == "k": return self.k() @@ -355,6 +357,9 @@ def vCtrlC(self): return "" + def vContT(self): + return "" + def k(self): return ["W01", self.RESPONSE_DISCONNECT] Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h @@ -37,6 +37,9 @@ bool Interrupt(std::chrono::seconds interrupt_timeout); + std::vector> + GetCurrentProcessAndThreadIDsNoLock(); + lldb::StateType SendContinuePacketAndWaitForResponse( ContinueDelegate &delegate, const UnixSignals &signals, llvm::StringRef payload, std::chrono::seconds interrupt_timeout, Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -8,6 +8,10 @@ #include "GDBRemoteClientBase.h" +#include +#include +#include + #include "llvm/ADT/StringExtras.h" #include "lldb/Target/UnixSignals.h" @@ -36,6 +40,70 @@ : GDBRemoteCommunication(comm_name, listener_name), m_async_count(0), m_is_running(false), m_should_stop(false) {} +static llvm::Optional> +GetThreadIDFromStopPacket(StringExtractor& packet) { + // Only stops are of interest to us, and only T stops carry thread id + if (packet.GetChar() != 'T') + return llvm::None; + // Skip signo + packet.GetHexU8(); + + llvm::StringRef key, value; + while (packet.GetNameColonValue(key, value)) { + if (key != "thread") + continue; + return StringExtractorGDBRemote(value).GetPidTid(LLDB_INVALID_PROCESS_ID); + } + + return llvm::None; +} + +std::vector> +GDBRemoteClientBase::GetCurrentProcessAndThreadIDsNoLock() { + std::vector> ids; + StringExtractorGDBRemote response; + + PacketResult packet_result; + for (packet_result = + SendPacketAndWaitForResponseNoLock("qfThreadInfo", response); + packet_result == PacketResult::Success && response.IsNormalResponse(); + packet_result = + SendPacketAndWaitForResponseNoLock("qsThreadInfo", response)) { + char ch = response.GetChar(); + if (ch == 'l') + break; + if (ch == 'm') { + do { + auto pid_tid = response.GetPidTid(LLDB_INVALID_PROCESS_ID); + // If we get an invalid response, break out of the loop. + // If there are valid tids, they have been added to ids. + // If there are no valid tids, we'll fall through to the + // bare-iron target handling below. + if (!pid_tid) + break; + + ids.push_back(*pid_tid); + ch = response.GetChar(); // Skip the command separator + } while (ch == ','); // Make sure we got a comma separator + } + } + + /* + * Connected bare-iron target (like YAMON gdb-stub) may not have support for + * qProcessInfo, qC and qfThreadInfo packets. The reply from '?' packet + * could + * be as simple as 'S05'. There is no packet which can give us pid and/or + * tid. + * Assume pid=tid=1 in such cases. + */ + if ((response.IsUnsupportedResponse() || response.IsNormalResponse()) && + ids.size() == 0 && IsConnected()) { + ids.emplace_back(1, 1); + } + + return ids; +} + StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse( ContinueDelegate &delegate, const UnixSignals &signals, llvm::StringRef payload, std::chrono::seconds interrupt_timeout, @@ -103,9 +171,9 @@ // We do not currently support full asynchronous communication. Instead, // when in non-stop mode, we wait for the asynchronous %Stop notification - // and then drain the notification queue - // TODO: issue vCont;t to ensure that all threads have actually stopped - // (this is not needed for LLGS but for gdbserver) + // and then drain the notification queue. Additionally, we issue + // a "vCont;t" request to stop all threads in case we were dealing + // with a true non-stop server. if (read_result == PacketResult::Notify && response.GetStringRef().startswith("Stdio:")) { @@ -132,6 +200,69 @@ if (!DrainNotificationQueue("vStopped")) return eStateInvalid; + + if (response.GetStringRef()[0] == 'T') { + auto main_pidtid = GetThreadIDFromStopPacket(response); + if (!main_pidtid.hasValue()) { + LLDB_LOG(log, "T stop reply without thread-id in non-stop mode"); + return eStateInvalid; + } + response.SetFilePos(0); + + for (int attempt = 0; attempt < 25; ++attempt) { + // Attempt to stop all remaining threads via vCont;t (if any). + StringExtractorGDBRemote stop_response; + if (SendPacketAndWaitForResponseNoLock("vCont;t", stop_response) != + PacketResult::Success || !stop_response.IsOKResponse()) { + LLDB_LOG(log, "vCont;t failed"); + return eStateInvalid; + } + + std::unordered_set all_threads; + std::unordered_set stopped_threads; + + // Issue a '?' command to get a list of all threads that have + // stopped. + // Note that '?' clears the vStopped notification queue. + const char *cmd = "?"; + for (;;) { + if (SendPacketAndWaitForResponseNoLock(cmd, stop_response) != + PacketResult::Success) { + LLDB_LOG(log, "vCont;t failed"); + return eStateInvalid; + } + + // We iterate until we get an "OK" response. + if (stop_response.IsOKResponse()) + break; + + auto stopped_pidtid = GetThreadIDFromStopPacket(stop_response); + if (stopped_pidtid.hasValue() && + stopped_pidtid->first == main_pidtid->first) + stopped_threads.insert(stopped_pidtid->second); + + cmd = "vStopped"; + } + + // Collect the list of all threads. + for (auto pidtid : GetCurrentProcessAndThreadIDsNoLock()) { + if (pidtid.first == main_pidtid->first) + all_threads.insert(pidtid.second); + } + + for (auto x : stopped_threads) + all_threads.erase(x); + // If all threads were stopped, we're done. Otherwise, try another + // iteration. + if (all_threads.empty()) + break; + + // Wait a little not to spam the server too fast. + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + + // TODO: do we fail if all attempts failed? + } } const char stop_type = response.GetChar(); Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -2864,58 +2864,17 @@ std::vector> GDBRemoteCommunicationClient::GetCurrentProcessAndThreadIDs( bool &sequence_mutex_unavailable) { - std::vector> ids; - Lock lock(*this); if (lock) { sequence_mutex_unavailable = false; - StringExtractorGDBRemote response; - - PacketResult packet_result; - for (packet_result = - SendPacketAndWaitForResponseNoLock("qfThreadInfo", response); - packet_result == PacketResult::Success && response.IsNormalResponse(); - packet_result = - SendPacketAndWaitForResponseNoLock("qsThreadInfo", response)) { - char ch = response.GetChar(); - if (ch == 'l') - break; - if (ch == 'm') { - do { - auto pid_tid = response.GetPidTid(LLDB_INVALID_PROCESS_ID); - // If we get an invalid response, break out of the loop. - // If there are valid tids, they have been added to ids. - // If there are no valid tids, we'll fall through to the - // bare-iron target handling below. - if (!pid_tid) - break; - - ids.push_back(*pid_tid); - ch = response.GetChar(); // Skip the command separator - } while (ch == ','); // Make sure we got a comma separator - } - } - - /* - * Connected bare-iron target (like YAMON gdb-stub) may not have support for - * qProcessInfo, qC and qfThreadInfo packets. The reply from '?' packet - * could - * be as simple as 'S05'. There is no packet which can give us pid and/or - * tid. - * Assume pid=tid=1 in such cases. - */ - if ((response.IsUnsupportedResponse() || response.IsNormalResponse()) && - ids.size() == 0 && IsConnected()) { - ids.emplace_back(1, 1); - } - } else { - Log *log(GetLog(GDBRLog::Process | GDBRLog::Packets)); - LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending " - "packet 'qfThreadInfo'"); - sequence_mutex_unavailable = true; + return GetCurrentProcessAndThreadIDsNoLock(); } - return ids; + Log *log(GetLog(GDBRLog::Process | GDBRLog::Packets)); + LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending " + "packet 'qfThreadInfo'"); + sequence_mutex_unavailable = true; + return {}; } size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs( Index: lldb/test/API/functionalities/gdb_remote_client/TestNonStop.py =================================================================== --- lldb/test/API/functionalities/gdb_remote_client/TestNonStop.py +++ lldb/test/API/functionalities/gdb_remote_client/TestNonStop.py @@ -33,6 +33,9 @@ self.vStopped_counter = 0 return ["OK", "%Stop:T02thread:p10.12;"] + def vContT(self): + return "OK" + self.dbg.HandleCommand( "settings set plugin.process.gdb-remote.use-non-stop-protocol true") self.addTearDownHook(lambda: @@ -74,6 +77,9 @@ return ["OK", "%Stdio:O6669727374206c696e650d0a",] + def vContT(self): + return "OK" + self.dbg.HandleCommand( "settings set plugin.process.gdb-remote.use-non-stop-protocol true") self.addTearDownHook(lambda: @@ -107,6 +113,9 @@ def vCtrlC(self): return ["OK", "%Stop:T00thread:p10.10;"] + def vContT(self): + return "OK" + self.dbg.HandleCommand( "settings set plugin.process.gdb-remote.use-non-stop-protocol true") self.addTearDownHook(lambda: @@ -130,3 +139,90 @@ self.assertPacketLogContains(["vStopped"]) self.assertEqual(process.GetSelectedThread().GetStopReason(), lldb.eStopReasonNone) + + def test_nonstop_server(self): + class MyResponder(NonStopResponder): + vCont_t_status = 0 + + def vStopped(self): + if self.vCont_t_status == 2: + self.vCont_t_status += 1 + return "T02thread:p10.12;" + return "OK" + + def cont(self): + return ["OK", "%Stop:T02thread:p10.12;"] + + def vContT(self): + self.vCont_t_status = 1 + return ["OK", "%Stop:T02thread:p10.10;"] + + def haltReason(self): + if self.vCont_t_status == 1: + self.vCont_t_status += 1 + return "T02thread:p10.10;" + return "S02" + + self.dbg.HandleCommand( + "settings set plugin.process.gdb-remote.use-non-stop-protocol true") + self.addTearDownHook(lambda: + self.runCmd( + "settings set plugin.process.gdb-remote.use-non-stop-protocol " + "false")) + self.server.responder = MyResponder() + target = self.dbg.CreateTarget("") + process = self.connect(target) + self.assertPacketLogContains(["QNonStop:1"]) + + process.Continue() + self.assertPacketLogContains(["vStopped", "vCont;t"]) + self.assertEqual(process.GetSelectedThread().GetStopReason(), + lldb.eStopReasonSignal) + self.assertEqual(process.GetSelectedThread().GetStopDescription(100), + "signal SIGINT") + self.assertEqual(self.server.responder.vCont_t_status, 3) + + def test_nonstop_server_non_immediate(self): + class MyResponder(NonStopResponder): + vCont_t_status = 0 + + def vStopped(self): + if self.vCont_t_status == 2: + return ["OK", "%Stop:T02thread:p10.10;"] + if self.vCont_t_status == 3: + self.vCont_t_status += 1 + return "T02thread:p10.10;" + return "OK" + + def cont(self): + return ["OK", "%Stop:T02thread:p10.12;"] + + def vContT(self): + if self.vCont_t_status == 0: + self.vCont_t_status = 1 + return "OK" + + def haltReason(self): + if self.vCont_t_status in (1, 2): + self.vCont_t_status += 1 + return "T02thread:p10.12;" + return "S02" + + self.dbg.HandleCommand( + "settings set plugin.process.gdb-remote.use-non-stop-protocol true") + self.addTearDownHook(lambda: + self.runCmd( + "settings set plugin.process.gdb-remote.use-non-stop-protocol " + "false")) + self.server.responder = MyResponder() + target = self.dbg.CreateTarget("") + process = self.connect(target) + self.assertPacketLogContains(["QNonStop:1"]) + + process.Continue() + self.assertPacketLogContains(["vStopped", "vCont;t"]) + self.assertEqual(process.GetSelectedThread().GetStopReason(), + lldb.eStopReasonSignal) + self.assertEqual(process.GetSelectedThread().GetStopDescription(100), + "signal SIGINT") + self.assertEqual(self.server.responder.vCont_t_status, 4)