mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.
Issue a `vCont;t` request after getting an asynchronous stop
notification in non-stop mode in order to ensure that all threads are
stopped. This is needed for servers implementing thread-level non-stop
mode (e.g. gdbserver). Unlike the current version of LLGS, these
servers do not stop all threads whenever one of them stops.
Depending on whether any threads were running at the time, the `vCont;t`
packet may or may not yield an asynchronous stop notification. Rather
than attempting to wait on that, use a combination of `?`
and `qfThreadInfo` packets to determine whether all threads are stopped.
Issue a `?` request first in order to obtain stop reasons for all
the stopped threads. As a bonus, this request clears the asynchronous
notification queue for any notification `vCont;t` may have produced.
Afterwards, obtain the complete thread list using `qfThreadInfo`.
If all debugged threads had stop reasons, it means that they all stopped
already and we can proceed. Otherwise, try again.
This logic should also be handle to handle the corner case of a new
thread being started after the `vCont;t` request.
We limit the stop attempts to 25 to avoid hanging when communicating
with a broken server.
Sponsored by: The FreeBSD Foundation
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.2)
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
