https://github.com/eleviant created https://github.com/llvm/llvm-project/pull/145675
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. >From a70386049dc93c0c495fdc285629a1a8dbeffe8c Mon Sep 17 00:00:00 2001 From: Evgeny Leviant <elevi...@accesssoftek.com> Date: Wed, 25 Jun 2025 11:01:29 +0200 Subject: [PATCH] [lldb] Fix qEcho message handling. 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. --- .../Python/lldbsuite/test/gdbclientutils.py | 10 +++ .../gdb-remote/GDBRemoteClientBase.cpp | 9 +-- .../Process/gdb-remote/GDBRemoteClientBase.h | 6 +- .../gdb-remote/GDBRemoteCommunication.cpp | 3 +- .../GDBRemoteCommunicationClient.cpp | 6 +- .../script_alias/TestCommandScriptAlias.py | 1 + .../gdb_remote_client/TestGDBRemoteClient.py | 72 +++++++++++++++++++ 7 files changed, 98 insertions(+), 9 deletions(-) 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): _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits