[llvm-branch-commits] [lldb] release/20.x: [lldb] Add support for gdb-style 'x' packet (#124733) (PR #125653)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Feb 4 01:04:44 PST 2025
https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/125653
Backport 13d0318a9848ec322ceea4f37fb6b421d70407b0
Requested by: @labath
>From bee03a956372c94d65181a0d2ddccf03fc8cb499 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Fri, 31 Jan 2025 09:07:11 +0100
Subject: [PATCH] [lldb] Add support for gdb-style 'x' packet (#124733)
See also
https://discourse.llvm.org/t/rfc-fixing-incompatibilties-of-the-x-packet-w-r-t-gdb/84288
and https://sourceware.org/pipermail/gdb/2025-January/051705.html
(cherry picked from commit 13d0318a9848ec322ceea4f37fb6b421d70407b0)
---
.../Python/lldbsuite/test/gdbclientutils.py | 6 ++
.../GDBRemoteCommunicationClient.cpp | 22 +++++---
.../gdb-remote/GDBRemoteCommunicationClient.h | 11 +++-
.../Process/gdb-remote/ProcessGDBRemote.cpp | 36 +++++++-----
.../gdb_remote_client/TestReadMemory.py | 55 +++++++++++++++++++
5 files changed, 106 insertions(+), 24 deletions(-)
create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
diff --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
index 1784487323ad6be..4b782b3b470fe22 100644
--- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
+++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
@@ -126,6 +126,9 @@ def respond(self, packet):
if packet[0] == "m":
addr, length = [int(x, 16) for x in packet[1:].split(",")]
return self.readMemory(addr, length)
+ if packet[0] == "x":
+ addr, length = [int(x, 16) for x in packet[1:].split(",")]
+ return self.x(addr, length)
if packet[0] == "M":
location, encoded_data = packet[1:].split(":")
addr, length = [int(x, 16) for x in location.split(",")]
@@ -267,6 +270,9 @@ def writeRegister(self, register, value_hex):
def readMemory(self, addr, length):
return "00" * length
+ def x(self, addr, length):
+ return ""
+
def writeMemory(self, addr, data_hex):
return "OK"
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index b3f1c6f052955b0..581dd8f8e0b6b64 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -275,7 +275,6 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
m_supports_vCont_s = eLazyBoolCalculate;
m_supports_vCont_S = eLazyBoolCalculate;
m_supports_p = eLazyBoolCalculate;
- m_supports_x = eLazyBoolCalculate;
m_supports_QSaveRegisterState = eLazyBoolCalculate;
m_qHostInfo_is_valid = eLazyBoolCalculate;
m_curr_pid_is_valid = eLazyBoolCalculate;
@@ -295,6 +294,7 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
m_supports_qXfer_siginfo_read = eLazyBoolCalculate;
m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
m_uses_native_signals = eLazyBoolCalculate;
+ m_x_packet_state.reset();
m_supports_qProcessInfoPID = true;
m_supports_qfProcessInfo = true;
m_supports_qUserName = true;
@@ -348,6 +348,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
m_supports_memory_tagging = eLazyBoolNo;
m_supports_qSaveCore = eLazyBoolNo;
m_uses_native_signals = eLazyBoolNo;
+ m_x_packet_state.reset();
m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
// not, we assume no limit
@@ -401,6 +402,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
m_supports_qSaveCore = eLazyBoolYes;
else if (x == "native-signals+")
m_uses_native_signals = eLazyBoolYes;
+ else if (x == "binary-upload+")
+ m_x_packet_state = xPacketState::Prefixed;
// Look for a list of compressions in the features list e.g.
// qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
// deflate,lzma
@@ -715,19 +718,20 @@ Status GDBRemoteCommunicationClient::WriteMemoryTags(
return status;
}
-bool GDBRemoteCommunicationClient::GetxPacketSupported() {
- if (m_supports_x == eLazyBoolCalculate) {
+GDBRemoteCommunicationClient::xPacketState
+GDBRemoteCommunicationClient::GetxPacketState() {
+ if (!m_x_packet_state)
+ GetRemoteQSupported();
+ if (!m_x_packet_state) {
StringExtractorGDBRemote response;
- m_supports_x = eLazyBoolNo;
- char packet[256];
- snprintf(packet, sizeof(packet), "x0,0");
- if (SendPacketAndWaitForResponse(packet, response) ==
+ m_x_packet_state = xPacketState::Unimplemented;
+ if (SendPacketAndWaitForResponse("x0,0", response) ==
PacketResult::Success) {
if (response.IsOKResponse())
- m_supports_x = eLazyBoolYes;
+ m_x_packet_state = xPacketState::Bare;
}
}
- return m_supports_x;
+ return *m_x_packet_state;
}
lldb::pid_t GDBRemoteCommunicationClient::GetCurrentProcessID(bool allow_lazy) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 898d176abc3465a..1118a76d7211b51 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -218,7 +218,14 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
bool GetpPacketSupported(lldb::tid_t tid);
- bool GetxPacketSupported();
+ enum class xPacketState {
+ Unimplemented,
+ Prefixed, // Successful responses start with a 'b' character. This is the
+ // style used by GDB.
+ Bare, // No prefix, packets starts with the memory being read. This is
+ // LLDB's original style.
+ };
+ xPacketState GetxPacketState();
bool GetVAttachOrWaitSupported();
@@ -541,7 +548,6 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
LazyBool m_attach_or_wait_reply = eLazyBoolCalculate;
LazyBool m_prepare_for_reg_writing_reply = eLazyBoolCalculate;
LazyBool m_supports_p = eLazyBoolCalculate;
- LazyBool m_supports_x = eLazyBoolCalculate;
LazyBool m_avoid_g_packets = eLazyBoolCalculate;
LazyBool m_supports_QSaveRegisterState = eLazyBoolCalculate;
LazyBool m_supports_qXfer_auxv_read = eLazyBoolCalculate;
@@ -561,6 +567,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
LazyBool m_supports_memory_tagging = eLazyBoolCalculate;
LazyBool m_supports_qSaveCore = eLazyBoolCalculate;
LazyBool m_uses_native_signals = eLazyBoolCalculate;
+ std::optional<xPacketState> m_x_packet_state;
bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
m_supports_qUserName : 1, m_supports_qGroupName : 1,
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 538c8680140091d..21a0fa283644d60 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2609,11 +2609,15 @@ void ProcessGDBRemote::WillPublicStop() {
// Process Memory
size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
Status &error) {
+ using xPacketState = GDBRemoteCommunicationClient::xPacketState;
+
GetMaxMemorySize();
- bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+ xPacketState x_state = m_gdb_comm.GetxPacketState();
+
// M and m packets take 2 bytes for 1 byte of memory
- size_t max_memory_size =
- binary_memory_read ? m_max_memory_size : m_max_memory_size / 2;
+ size_t max_memory_size = x_state != xPacketState::Unimplemented
+ ? m_max_memory_size
+ : m_max_memory_size / 2;
if (size > max_memory_size) {
// Keep memory read sizes down to a sane limit. This function will be
// called multiple times in order to complete the task by
@@ -2624,8 +2628,8 @@ size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
char packet[64];
int packet_len;
packet_len = ::snprintf(packet, sizeof(packet), "%c%" PRIx64 ",%" PRIx64,
- binary_memory_read ? 'x' : 'm', (uint64_t)addr,
- (uint64_t)size);
+ x_state != xPacketState::Unimplemented ? 'x' : 'm',
+ (uint64_t)addr, (uint64_t)size);
assert(packet_len + 1 < (int)sizeof(packet));
UNUSED_IF_ASSERT_DISABLED(packet_len);
StringExtractorGDBRemote response;
@@ -2634,19 +2638,25 @@ size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
GDBRemoteCommunication::PacketResult::Success) {
if (response.IsNormalResponse()) {
error.Clear();
- if (binary_memory_read) {
+ if (x_state != xPacketState::Unimplemented) {
// The lower level GDBRemoteCommunication packet receive layer has
// already de-quoted any 0x7d character escaping that was present in
// the packet
- size_t data_received_size = response.GetBytesLeft();
- if (data_received_size > size) {
- // Don't write past the end of BUF if the remote debug server gave us
- // too much data for some reason.
- data_received_size = size;
+ llvm::StringRef data_received = response.GetStringRef();
+ if (x_state == xPacketState::Prefixed &&
+ !data_received.consume_front("b")) {
+ error = Status::FromErrorStringWithFormatv(
+ "unexpected response to GDB server memory read packet '{0}': "
+ "'{1}'",
+ packet, data_received);
+ return 0;
}
- memcpy(buf, response.GetStringRef().data(), data_received_size);
- return data_received_size;
+ // Don't write past the end of BUF if the remote debug server gave us
+ // too much data for some reason.
+ size_t memcpy_size = std::min(size, data_received.size());
+ memcpy(buf, data_received.data(), memcpy_size);
+ return memcpy_size;
} else {
return response.GetHexBytes(
llvm::MutableArrayRef<uint8_t>((uint8_t *)buf, size), '\xdd');
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py b/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
new file mode 100644
index 000000000000000..81dcb54aef5d8e8
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
@@ -0,0 +1,55 @@
+import lldb
+from lldbsuite.support import seven
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestReadMemory(GDBRemoteTestBase):
+ def test_x_with_prefix(self):
+ class MyResponder(MockGDBServerResponder):
+ def qSupported(self, client_features):
+ # binary-upload+ indicates we use the gdb style of x packets
+ return super().qSupported(client_features) + ";binary-upload+"
+
+ def x(self, addr, length):
+ return "bfoobar" if addr == 0x1000 else "E01"
+
+ self.server.responder = MyResponder()
+ target = self.dbg.CreateTargetWithFileAndTargetTriple("", "x86_64-pc-linux")
+ process = self.connect(target)
+
+ error = lldb.SBError()
+ self.assertEqual(b"foobar", process.ReadMemory(0x1000, 10, error))
+
+ def test_x_bare(self):
+ class MyResponder(MockGDBServerResponder):
+ def x(self, addr, length):
+ # The OK response indicates we use the old lldb style.
+ if addr == 0 and length == 0:
+ return "OK"
+ return "foobar" if addr == 0x1000 else "E01"
+
+ self.server.responder = MyResponder()
+ target = self.dbg.CreateTargetWithFileAndTargetTriple("", "x86_64-pc-linux")
+ process = self.connect(target)
+
+ error = lldb.SBError()
+ self.assertEqual(b"foobar", process.ReadMemory(0x1000, 10, error))
+
+ def test_m_fallback(self):
+ class MyResponder(MockGDBServerResponder):
+ def x(self, addr, length):
+ # If `x` is unsupported, we should fall back to `m`.
+ return ""
+
+ def readMemory(self, addr, length):
+ return seven.hexlify("foobar") if addr == 0x1000 else "E01"
+
+ self.server.responder = MyResponder()
+ target = self.dbg.CreateTargetWithFileAndTargetTriple("", "x86_64-pc-linux")
+ process = self.connect(target)
+
+ error = lldb.SBError()
+ self.assertEqual(b"foobar", process.ReadMemory(0x1000, 10, error))
More information about the llvm-branch-commits
mailing list