[Lldb-commits] [lldb] [lldb] Fix qEcho message handling. (PR #145675)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 25 04:21:16 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: None (eleviant)
<details>
<summary>Changes</summary>
This fixes issues found in e066f35c6981c720e3a7e5883efc40c861b3b7, which was later reverted. The problem was with "k" message which was sent with sync_on_timeout flag set to true, so lldb was waiting for response, which is currently not being sent by lldb-server. Not waiting for response at all seems to be not a solution, because on MAC OS X lldb waits for response from "k" to gracefully kill inferior.
---
Full diff: https://github.com/llvm/llvm-project/pull/145675.diff
7 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/gdbclientutils.py (+10)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp (+5-4)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h (+4-2)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (+2-1)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+4-2)
- (modified) lldb/test/API/commands/command/script_alias/TestCommandScriptAlias.py (+1)
- (modified) lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py (+72)
``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
index 753de22b9cfee..b603c35c8df09 100644
--- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
+++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
@@ -92,6 +92,9 @@ class MockGDBServerResponder:
class RESPONSE_DISCONNECT:
pass
+ class RESPONSE_NONE:
+ pass
+
def __init__(self):
self.packetLog = []
@@ -181,6 +184,8 @@ def respond(self, packet):
return self.qQueryGDBServer()
if packet == "qHostInfo":
return self.qHostInfo()
+ if packet.startswith("qEcho"):
+ return self.qEcho(int(packet.split(":")[1]))
if packet == "qGetWorkingDir":
return self.qGetWorkingDir()
if packet == "qOffsets":
@@ -237,6 +242,9 @@ def qProcessInfo(self):
def qHostInfo(self):
return "ptrsize:8;endian:little;"
+ def qEcho(self):
+ return "E04"
+
def qQueryGDBServer(self):
return "E04"
@@ -655,6 +663,8 @@ def _handlePacket(self, packet):
if not isinstance(response, list):
response = [response]
for part in response:
+ if part is MockGDBServerResponder.RESPONSE_NONE:
+ continue
if part is MockGDBServerResponder.RESPONSE_DISCONNECT:
raise self.TerminateConnectionException()
self._sendPacket(part)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index 394b62559da76..406fa06ea011a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -180,7 +180,7 @@ bool GDBRemoteClientBase::Interrupt(std::chrono::seconds interrupt_timeout) {
GDBRemoteCommunication::PacketResult
GDBRemoteClientBase::SendPacketAndWaitForResponse(
llvm::StringRef payload, StringExtractorGDBRemote &response,
- std::chrono::seconds interrupt_timeout) {
+ std::chrono::seconds interrupt_timeout, bool sync_on_timeout) {
Lock lock(*this, interrupt_timeout);
if (!lock) {
if (Log *log = GetLog(GDBRLog::Process))
@@ -191,7 +191,7 @@ GDBRemoteClientBase::SendPacketAndWaitForResponse(
return PacketResult::ErrorSendFailed;
}
- return SendPacketAndWaitForResponseNoLock(payload, response);
+ return SendPacketAndWaitForResponseNoLock(payload, response, sync_on_timeout);
}
GDBRemoteCommunication::PacketResult
@@ -236,14 +236,15 @@ GDBRemoteClientBase::SendPacketAndReceiveResponseWithOutputSupport(
GDBRemoteCommunication::PacketResult
GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
- llvm::StringRef payload, StringExtractorGDBRemote &response) {
+ llvm::StringRef payload, StringExtractorGDBRemote &response,
+ bool sync_on_timeout) {
PacketResult packet_result = SendPacketNoLock(payload);
if (packet_result != PacketResult::Success)
return packet_result;
const size_t max_response_retries = 3;
for (size_t i = 0; i < max_response_retries; ++i) {
- packet_result = ReadPacket(response, GetPacketTimeout(), true);
+ packet_result = ReadPacket(response, GetPacketTimeout(), sync_on_timeout);
// Make sure we received a response
if (packet_result != PacketResult::Success)
return packet_result;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index af2abdf4da5cf..9c17a8c1de057 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -61,7 +61,8 @@ class GDBRemoteClientBase : public GDBRemoteCommunication, public Broadcaster {
// ErrorReplyTimeout.
PacketResult SendPacketAndWaitForResponse(
llvm::StringRef payload, StringExtractorGDBRemote &response,
- std::chrono::seconds interrupt_timeout = std::chrono::seconds(0));
+ std::chrono::seconds interrupt_timeout = std::chrono::seconds(0),
+ bool sync_on_timeout = true);
PacketResult ReadPacketWithOutputSupport(
StringExtractorGDBRemote &response, Timeout<std::micro> timeout,
@@ -104,7 +105,8 @@ class GDBRemoteClientBase : public GDBRemoteCommunication, public Broadcaster {
protected:
PacketResult
SendPacketAndWaitForResponseNoLock(llvm::StringRef payload,
- StringExtractorGDBRemote &response);
+ StringExtractorGDBRemote &response,
+ bool sync_on_timeout = true);
virtual void OnRunPacketSent(bool first);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 2aea7c6b781d7..f244f7abe45e3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -354,8 +354,9 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
disconnected = true;
Disconnect();
}
+ } else {
+ timed_out = true;
}
- timed_out = true;
break;
case eConnectionStatusSuccess:
// printf ("status = success but error = %s\n",
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index adbf06b9a19a0..7d2bd452acca9 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -406,7 +406,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
m_supports_qXfer_memory_map_read = eLazyBoolYes;
else if (x == "qXfer:siginfo:read+")
m_supports_qXfer_siginfo_read = eLazyBoolYes;
- else if (x == "qEcho")
+ else if (x == "qEcho+")
m_supports_qEcho = eLazyBoolYes;
else if (x == "QPassSignals+")
m_supports_QPassSignals = eLazyBoolYes;
@@ -4358,7 +4358,9 @@ llvm::Expected<int> GDBRemoteCommunicationClient::KillProcess(lldb::pid_t pid) {
StringExtractorGDBRemote response;
GDBRemoteCommunication::ScopedTimeout(*this, seconds(3));
- if (SendPacketAndWaitForResponse("k", response, GetPacketTimeout()) !=
+ // LLDB server typically sends no response for "k", so we shouldn't try
+ // to sync on timeout.
+ if (SendPacketAndWaitForResponse("k", response, GetPacketTimeout(), false) !=
PacketResult::Success)
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"failed to send k packet");
diff --git a/lldb/test/API/commands/command/script_alias/TestCommandScriptAlias.py b/lldb/test/API/commands/command/script_alias/TestCommandScriptAlias.py
index 2696f703f0e1c..09886baf5406c 100644
--- a/lldb/test/API/commands/command/script_alias/TestCommandScriptAlias.py
+++ b/lldb/test/API/commands/command/script_alias/TestCommandScriptAlias.py
@@ -11,6 +11,7 @@ class CommandScriptAliasTestCase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
def test_pycmd(self):
+ self.runCmd("log enable -f /tmp/gdb.log gdb-remote all")
self.runCmd("command script import tcsacmd.py")
self.runCmd("command script add -f tcsacmd.some_command_here attach")
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
index 08ac9290ee85a..12b464d3397eb 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -356,6 +356,78 @@ def A(self, packet):
["vRun;%s;61726731;61726732;61726733" % (exe_hex,)]
)
+ def test_launch_lengthy_vRun(self):
+ class MyResponder(MockGDBServerResponder):
+ def __init__(self, *args, **kwargs):
+ self.started = False
+ return super().__init__(*args, **kwargs)
+
+ def qC(self):
+ if self.started:
+ return "QCp10.10"
+ else:
+ return "E42"
+
+ def qfThreadInfo(self):
+ if self.started:
+ return "mp10.10"
+ else:
+ return "E42"
+
+ def qsThreadInfo(self):
+ return "l"
+
+ def qEcho(self, num):
+ resp = "qEcho:" + str(num)
+ if num >= 2:
+ # We have launched our program
+ self.started = True
+ return [resp, "T13"]
+
+ return resp
+
+ def qSupported(self, client_supported):
+ return "PacketSize=3fff;QStartNoAckMode+;qEcho+;"
+
+ def qHostInfo(self):
+ return "default_packet_timeout:1;"
+
+ def vRun(self, packet):
+ return [self.RESPONSE_NONE]
+
+ def A(self, packet):
+ return "E28"
+
+ self.server.responder = MyResponder()
+
+ target = self.createTarget("a.yaml")
+ # NB: apparently GDB packets are using "/" on Windows too
+ exe_path = self.getBuildArtifact("a").replace(os.path.sep, "/")
+ exe_hex = binascii.b2a_hex(exe_path.encode()).decode()
+ process = self.connect(target)
+ lldbutil.expect_state_changes(
+ self, self.dbg.GetListener(), process, [lldb.eStateConnected]
+ )
+
+ process = target.Launch(
+ lldb.SBListener(),
+ ["arg1", "arg2", "arg3"], # argv
+ [], # envp
+ None, # stdin_path
+ None, # stdout_path
+ None, # stderr_path
+ None, # working_directory
+ 0, # launch_flags
+ True, # stop_at_entry
+ lldb.SBError(),
+ ) # error
+ self.assertTrue(process, PROCESS_IS_VALID)
+ self.assertEqual(process.GetProcessID(), 16)
+
+ self.assertPacketLogContains(
+ ["vRun;%s;61726731;61726732;61726733" % (exe_hex,)]
+ )
+
def test_launch_QEnvironment(self):
class MyResponder(MockGDBServerResponder):
def qC(self):
``````````
</details>
https://github.com/llvm/llvm-project/pull/145675
More information about the lldb-commits
mailing list