[lldb] Fix qEcho message handling. #145675
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes issues found in e066f35, 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.
|
@llvm/pr-subscribers-lldb Author: None (eleviant) ChangesThis fixes issues found in e066f35, 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:
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):
|
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
This fixes issues found in e066f35, 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes issues found in e066f35, 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.