mgorny updated this revision to Diff 443906.
mgorny added a comment.
Remove leftover debug hack.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129554/new/
https://reviews.llvm.org/D129554
Files:
lldb/packages/Python/lldbsuite/test/gdbclientutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/test/API/functionalities/gdb_remote_client/TestNonStop.py
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)
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<std::pair<lldb::pid_t, lldb::tid_t>>
GDBRemoteCommunicationClient::GetCurrentProcessAndThreadIDs(
bool &sequence_mutex_unavailable) {
- std::vector<std::pair<lldb::pid_t, lldb::tid_t>> 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/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<std::pair<lldb::pid_t, lldb::tid_t>>
+ 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 <chrono>
+#include <thread>
+#include <unordered_set>
+
#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<std::pair<lldb::pid_t, lldb::tid_t>>
+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<std::pair<lldb::pid_t, lldb::tid_t>>
+GDBRemoteClientBase::GetCurrentProcessAndThreadIDsNoLock() {
+ std::vector<std::pair<lldb::pid_t, lldb::tid_t>> 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<lldb::tid_t> all_threads;
+ std::unordered_set<lldb::tid_t> 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/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]
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
