https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/124733
>From bf011ccc02c0122e7dfd74e089143eb833c1686e Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Tue, 28 Jan 2025 12:27:46 +0100
Subject: [PATCH 1/2] [lldb] Add support for gdb-style 'x' packet
DO NOT SUBMIT until the mechanism for detection of the packet format is
confirmed.
---
.../Python/lldbsuite/test/gdbclientutils.py | 6 ++++
.../GDBRemoteCommunicationClient.cpp | 22 +++++++-----
.../gdb-remote/GDBRemoteCommunicationClient.h | 9 +++--
.../Process/gdb-remote/ProcessGDBRemote.cpp | 36 ++++++++++++-------
.../gdb_remote_client/TestReadMemory.py | 36 +++++++++++++++++++
5 files changed, 85 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 1784487323ad6b..4b782b3b470fe2 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 b3f1c6f052955b..581dd8f8e0b6b6 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 898d176abc3465..50f11cb009b8ca 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -218,7 +218,12 @@ class GDBRemoteCommunicationClient : public
GDBRemoteClientBase {
bool GetpPacketSupported(lldb::tid_t tid);
- bool GetxPacketSupported();
+ enum class xPacketState {
+ Unimplemented,
+ Prefixed, // Successful responses start with a 'b' character.
+ Bare, // No prefix, packets starts with the memory being read.
+ };
+ xPacketState GetxPacketState();
bool GetVAttachOrWaitSupported();
@@ -541,7 +546,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 +565,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 538c8680140091..21a0fa283644d6 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 00000000000000..63c3b505358cbf
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
@@ -0,0 +1,36 @@
+import lldb
+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):
+ 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):
+ 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))
>From 4bb49db7151388a66f4f25caad81e6688a958c64 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Thu, 30 Jan 2025 13:50:10 +0100
Subject: [PATCH 2/2] add comments, test for m
---
.../gdb-remote/GDBRemoteCommunicationClient.h | 6 ++++--
.../gdb_remote_client/TestReadMemory.py | 19 +++++++++++++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 50f11cb009b8ca..1118a76d7211b5 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -220,8 +220,10 @@ class GDBRemoteCommunicationClient : public
GDBRemoteClientBase {
enum class xPacketState {
Unimplemented,
- Prefixed, // Successful responses start with a 'b' character.
- Bare, // No prefix, packets starts with the memory being read.
+ 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();
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
b/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
index 63c3b505358cbf..81dcb54aef5d8e 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py
@@ -1,4 +1,5 @@
import lldb
+from lldbsuite.support import seven
from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *
from lldbsuite.test.gdbclientutils import *
@@ -9,6 +10,7 @@ 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):
@@ -24,6 +26,7 @@ def x(self, addr, length):
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"
@@ -34,3 +37,19 @@ def x(self, addr, length):
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))
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
