[Lldb-commits] [lldb] [lldb] Ignore notification packets (PR #204788)
Tim Hutt via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 22 03:03:13 PDT 2026
https://github.com/Timmmm updated https://github.com/llvm/llvm-project/pull/204788
>From 8b753f09668bf7935755806f2717f52d84d08e50 Mon Sep 17 00:00:00 2001
From: Tim Hutt <tdhutt at gmail.com>
Date: Mon, 18 May 2026 10:22:54 +0100
Subject: [PATCH] [lldb] Ignore notification packets
The gdb-protocol spec says
> Recipients should silently ignore corrupted notifications and notifications they do not understand.
This changes `WaitForPacketNoLock` so that it ignores all notifications.
An example of a notification that causes issues without this change is that OpenOCD sends `%ookeepalive:00` notifications during memory accesses.
---
.../Python/lldbsuite/test/gdbclientutils.py | 15 ++--
.../gdb-remote/GDBRemoteCommunication.cpp | 43 +++++++++++-
.../gdb-remote/GDBRemoteCommunication.h | 3 +
.../TestIgnoringNotifications.py | 68 +++++++++++++++++++
4 files changed, 121 insertions(+), 8 deletions(-)
create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestIgnoringNotifications.py
diff --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
index 4c40299f3256d..b789452bac174 100644
--- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
+++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
@@ -22,15 +22,18 @@ def checksum(message):
return check % 256
-def frame_packet(message):
+def frame_packet(message, prefix):
"""
Create a framed packet that's ready to send over the GDB connection
channel.
- Framing includes surrounding the message between $ and #, and appending
- a two character hex checksum.
+ Framing means:
+ * Attaching the prefix. Which is usually '$' but for notifications will be
+ '%'.
+ * Appending a '#'.
+ * Adding a two character hex checksum.
"""
- return "$%s#%02x" % (message, checksum(message))
+ return "%s%s#%02x" % (prefix, message, checksum(message))
def escape_binary(message):
@@ -694,9 +697,9 @@ def _parsePacket(self):
self._receivedDataOffset = 0
return packet
- def _sendPacket(self, packet: str):
+ def _sendPacket(self, packet: str, prefix="$"):
assert self._socket is not None
- framed_packet = seven.bitcast_to_bytes(frame_packet(packet))
+ framed_packet = seven.bitcast_to_bytes(frame_packet(packet, prefix))
self._socket.sendall(framed_packet)
def _handlePacket(self, packet):
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 04f486882e2c2..80dc4fab51605 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -242,8 +242,15 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
Log *log = GetLog(GDBRLog::Packets);
// Check for a packet from our cache first without trying any reading...
- if (CheckForPacket(nullptr, 0, packet) != PacketType::Invalid)
+ switch (CheckForPacketIgnoreNotifications(nullptr, 0, packet)) {
+ case PacketType::Standard:
return PacketResult::Success;
+ case PacketType::Invalid:
+ break;
+ case PacketType::Notify:
+ // These are ignored by CheckForPacketIgnoreNotifications.
+ llvm_unreachable("unreachable");
+ }
bool timed_out = false;
bool disconnected = false;
@@ -258,8 +265,15 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
error, bytes_read);
if (bytes_read > 0) {
- if (CheckForPacket(buffer, bytes_read, packet) != PacketType::Invalid)
+ switch (CheckForPacketIgnoreNotifications(buffer, bytes_read, packet)) {
+ case PacketType::Standard:
return PacketResult::Success;
+ case PacketType::Invalid:
+ break;
+ case PacketType::Notify:
+ // These are ignored by CheckForPacketIgnoreNotifications.
+ llvm_unreachable("unreachable");
+ }
} else {
switch (status) {
case eConnectionStatusTimedOut:
@@ -839,6 +853,31 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len,
return GDBRemoteCommunication::PacketType::Invalid;
}
+GDBRemoteCommunication::PacketType
+GDBRemoteCommunication::CheckForPacketIgnoreNotifications(
+ const uint8_t *src, size_t src_len, StringExtractorGDBRemote &packet) {
+ for (;;) {
+ switch (CheckForPacket(src, src_len, packet)) {
+ case PacketType::Standard:
+ return PacketType::Standard;
+ case PacketType::Invalid:
+ // Either an incomplete packet or an invalid one that was removed.
+ // There may be more data in the buffer that contains valid
+ // packets but we can't easily tell because CheckForPacket
+ // does not distinguish these cases.
+ return PacketType::Invalid;
+ case PacketType::Notify:
+ // No notifications are currently supported so they should be ignored.
+ // There may be more packets in the buffer so go back to check.
+
+ // Also we need to set src_len=0 so that it doesn't add the
+ // data we read to the internal buffer again.
+ src_len = 0;
+ break;
+ }
+ }
+}
+
Status GDBRemoteCommunication::StartDebugserverProcess(
std::variant<llvm::StringRef, shared_fd_t> comm,
ProcessLaunchInfo &launch_info, const Args *inferior_args) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 35bf5eb2e3f0d..4b51cb3900e94 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -119,6 +119,9 @@ class GDBRemoteCommunication : public Communication {
PacketType CheckForPacket(const uint8_t *src, size_t src_len,
StringExtractorGDBRemote &packet);
+ PacketType CheckForPacketIgnoreNotifications(const uint8_t *src, size_t src_len,
+ StringExtractorGDBRemote &packet);
+
bool GetSendAcks() { return m_send_acks; }
// Set the global packet timeout.
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestIgnoringNotifications.py b/lldb/test/API/functionalities/gdb_remote_client/TestIgnoringNotifications.py
new file mode 100644
index 0000000000000..acf44f5aa967e
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestIgnoringNotifications.py
@@ -0,0 +1,68 @@
+"""
+Test that LLDB ignores notification packets (those beginning with '%')
+when waiting for a response to a '$' packet.
+
+OpenOCD is one debug server that uses notification packets to reset
+the connection timeout if a memory read takes too long. So that's what
+we test here, but it should apply to any exchange.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class NotifyingServerResponder(MockGDBServerResponder):
+ def readMemory(self, addr, length):
+ return "01" * length
+
+
+class NotifyingServer(MockGDBServer):
+ def __init__(self, socket):
+ self._socket = socket
+ self.responder = NotifyingServerResponder()
+ self.sent_notification = False
+
+ def _sendPacket(self, packet: str, prefix="$"):
+ # In theory we could add notifies before all packets, but it always
+ # goes through the same code and just makes the test take longer.
+ if packet == "01010101":
+ # Send more than one to make sure we clear them all to find
+ # the real response.
+ super()._sendPacket("this_is_a_notification", prefix="%")
+ super()._sendPacket("this_is_a_2nd_notification", prefix="%")
+ self.sent_notification = True
+
+ super()._sendPacket(packet)
+
+
+class TestIgnoringNotifications(GDBRemoteTestBase):
+ def setUp(self):
+ TestBase.setUp(self)
+ self.server = NotifyingServer(self.server_socket_class())
+ self.server.start()
+
+ @skipIfLLVMTargetMissing("AArch64")
+ def test(self):
+ target = self.createTarget("basic_eh_frame-aarch64.yaml")
+
+ if self.TraceOn():
+ self.runCmd("log enable gdb-remote packets")
+ self.runCmd("log enable gdb-remote comm")
+ self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets"))
+
+ process = self.connect(target)
+ lldbutil.expect_state_changes(
+ self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+ )
+
+ # Disabling cache is not required but makes debugging this test easier.
+ self.runCmd("settings set target.process.disable-memory-cache true")
+
+ # Should succeed despite getting a notification before the real result.
+ self.expect("memory read --format hex 0x1234 0x1238", substrs=["0x01010101"])
+
+ # Check we didn't succeed because lldb got memory in some other way.
+ self.assertTrue(self.server.sent_notification)
More information about the lldb-commits
mailing list