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

Reply via email to