https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/162670
This commit implements, in debugserver, the packet as discussed in the RFC [1]. [1]: https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/88441 >From 86c55d2a5ac398a86a29d722a4d4e44ba783d08f Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan <[email protected]> Date: Wed, 8 Oct 2025 15:37:48 -0700 Subject: [PATCH] [debugserver] Implement MultiMemRead packet This commit implements, in debugserver, the packet as discussed in the RFC [1]. [1]: https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/88441 --- .../macosx/debugserver-multimemread/Makefile | 3 + .../TestDebugserverMultiMemRead.py | 72 +++++++++ .../macosx/debugserver-multimemread/main.c | 10 ++ lldb/tools/debugserver/source/RNBRemote.cpp | 140 ++++++++++++++++++ lldb/tools/debugserver/source/RNBRemote.h | 2 + 5 files changed, 227 insertions(+) create mode 100644 lldb/test/API/macosx/debugserver-multimemread/Makefile create mode 100644 lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py create mode 100644 lldb/test/API/macosx/debugserver-multimemread/main.c diff --git a/lldb/test/API/macosx/debugserver-multimemread/Makefile b/lldb/test/API/macosx/debugserver-multimemread/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/macosx/debugserver-multimemread/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py b/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py new file mode 100644 index 0000000000000..5c0c7a34e18bd --- /dev/null +++ b/lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py @@ -0,0 +1,72 @@ +""" +Tests the exit code/description coming from the debugserver. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +@skipUnlessDarwin +@skipIfOutOfTreeDebugserver +class TestCase(TestBase): + def send_process_packet(self, packet_str): + self.runCmd(f"proc plugin packet send {packet_str}", check=False) + # The output is of the form: + # packet: <packet_str> + # response: <response> + reply = self.res.GetOutput().split("\n") + reply[0] = reply[0].strip() + reply[1] = reply[1].strip() + + self.assertTrue(reply[0].startswith("packet: "), reply[0]) + self.assertTrue(reply[1].startswith("response: ")) + return reply[1][len("response: ") :] + + def test_packets(self): + self.build() + source_file = lldb.SBFileSpec("main.c") + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint( + self, "break here", source_file + ) + + reply = self.send_process_packet("qSupported") + self.assertIn("MultiMemRead+", reply) + + mem_address_var = thread.frames[0].FindVariable("memory") + self.assertTrue(mem_address_var) + mem_address = int(mem_address_var.GetValue(), 16) + + reply = self.send_process_packet("MultiMemRead:") + self.assertEqual(reply, "E03") + reply = self.send_process_packet("MultiMemRead:ranges:") + self.assertEqual(reply, "E03") + reply = self.send_process_packet("MultiMemRead:ranges:10") + self.assertEqual(reply, "E03") + reply = self.send_process_packet("MultiMemRead:ranges:10,") + self.assertEqual(reply, "E03") + reply = self.send_process_packet("MultiMemRead:ranges:10,2") + self.assertEqual(reply, "E03") + reply = self.send_process_packet("MultiMemRead:ranges:10,2,") + self.assertEqual(reply, "E03") + reply = self.send_process_packet("MultiMemRead:ranges:10,2,;") + self.assertEqual(reply, "0;") # Debugserver is permissive with trailing commas. + reply = self.send_process_packet("MultiMemRead:ranges:10,2;") + self.assertEqual(reply, "0;") + reply = self.send_process_packet(f"MultiMemRead:ranges:{mem_address:x},0;") + self.assertEqual(reply, "0;") + reply = self.send_process_packet( + f"MultiMemRead:ranges:{mem_address:x},0;options:;" + ) + self.assertEqual(reply, "0;") + reply = self.send_process_packet(f"MultiMemRead:ranges:{mem_address:x},2;") + self.assertEqual(reply, "2;ab") + reply = self.send_process_packet( + f"MultiMemRead:ranges:{mem_address:x},2,{mem_address+2:x},4;" + ) + self.assertEqual(reply, "2,4;abcdef") + reply = self.send_process_packet( + f"MultiMemRead:ranges:{mem_address:x},2,{mem_address+2:x},4,{mem_address+6:x},8;" + ) + self.assertEqual(reply, "2,4,8;abcdefghijklmn") diff --git a/lldb/test/API/macosx/debugserver-multimemread/main.c b/lldb/test/API/macosx/debugserver-multimemread/main.c new file mode 100644 index 0000000000000..1a756a3c4cbac --- /dev/null +++ b/lldb/test/API/macosx/debugserver-multimemread/main.c @@ -0,0 +1,10 @@ +#include <stdlib.h> +#include <string.h> + +int main(int argc, char **argv) { + char *memory = malloc(1024); + memset(memory, '-', 1024); + for (int i = 0; i < 50; i++) + memory[i] = 'a' + i; + return 0; // break here +} diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 434e9cfa40fb4..dfb0443fdd3dd 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -246,6 +246,11 @@ void RNBRemote::CreatePacketTable() { "Read memory")); t.push_back(Packet(read_register, &RNBRemote::HandlePacket_p, NULL, "p", "Read one register")); + // Careful: this *must* come before the `M` packet, as debugserver matches + // packet prefixes against known packet names. Inverting the order would match + // `MultiMemRead` as an `M` packet. + t.push_back(Packet(multi_mem_read, &RNBRemote::HandlePacket_MultiMemRead, + NULL, "MultiMemRead", "Read multiple memory addresses")); t.push_back(Packet(write_memory, &RNBRemote::HandlePacket_M, NULL, "M", "Write memory")); t.push_back(Packet(write_register, &RNBRemote::HandlePacket_P, NULL, "P", @@ -3160,6 +3165,140 @@ rnb_err_t RNBRemote::HandlePacket_m(const char *p) { return SendPacket(ostrm.str()); } +/// Returns true if `str` starts with `prefix`. +static bool starts_with(std::string_view str, std::string_view prefix) { + return str.size() >= prefix.size() && + str.compare(0, prefix.size(), prefix) == 0; +} + +/// Attempts to parse a prefix of `number_str` as a number of type `T`. If +/// successful, the number is returned and the prefix is dropped from +/// `number_str`. +template <typename T> +static std::optional<T> extract_number(std::string_view &number_str) { + static_assert(std::is_integral<T>()); + char *str_end = nullptr; + errno = 0; + nub_addr_t number = strtoull(number_str.data(), &str_end, 16); + if (errno != 0) + return std::nullopt; + assert(str_end); + number_str.remove_prefix(str_end - number_str.data()); + return number; +} + +/// Splits `list_str` into multiple string_views separated by `,`. +static std::vector<std::string_view> +parse_comma_separated_list(std::string_view list_str) { + std::vector<std::string_view> list; + while (!list_str.empty()) { + auto pos = list_str.find(','); + list.push_back(list_str.substr(0, pos)); + if (pos == list_str.npos) + break; + list_str.remove_prefix(pos + 1); + } + return list; +} + +rnb_err_t RNBRemote::HandlePacket_MultiMemRead(const char *p) { + const std::string_view packet_name("MultiMemRead:"); + std::string_view packet(p); + + if (!starts_with(packet, packet_name)) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, p, + "Invalid MultiMemRead packet prefix"); + + packet.remove_prefix(packet_name.size()); + + const std::string_view ranges_prefix("ranges:"); + if (!starts_with(packet, ranges_prefix)) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, packet.data(), + "Missing 'ranges' in MultiMemRead packet"); + packet.remove_prefix(ranges_prefix.size()); + + std::vector<std::pair<nub_addr_t, std::size_t>> ranges; + std::size_t total_length = 0; + + // Ranges should have the form: <addr>,<size>[,<addr>,<size>]*; + auto end_of_ranges_pos = packet.find(';'); + if (end_of_ranges_pos == packet.npos) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, packet.data(), + "MultiMemRead missing end of ranges marker"); + + std::vector<std::string_view> numbers_list = + parse_comma_separated_list(packet.substr(0, end_of_ranges_pos)); + packet.remove_prefix(end_of_ranges_pos + 1); + + // Ranges are pairs, so the number of elements must be even. + if (numbers_list.size() % 2 == 1) + return HandlePacket_ILLFORMED( + __FILE__, __LINE__, p, + "MultiMemRead has an odd number of numbers for the ranges"); + + for (unsigned idx = 0; idx < numbers_list.size(); idx += 2) { + std::optional<nub_addr_t> maybe_addr = + extract_number<nub_addr_t>(numbers_list[idx]); + std::optional<size_t> maybe_length = + extract_number<size_t>(numbers_list[idx + 1]); + if (!maybe_addr || !maybe_length) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, packet.data(), + "Invalid MultiMemRead range"); + // A sanity check that the packet requested is not too large or a negative + // number. + if (*maybe_length > 4 * 1024 * 1024) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, packet.data(), + "MultiMemRead length is too large"); + + ranges.emplace_back(*maybe_addr, *maybe_length); + total_length += *maybe_length; + } + + if (ranges.empty()) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, p, + "MultiMemRead has an empty range list"); + + // If 'options:' is present, ensure it's empty. + const std::string_view options_prefix("options:"); + if (starts_with(packet, options_prefix)) { + packet.remove_prefix(options_prefix.size()); + if (packet.empty()) + return HandlePacket_ILLFORMED( + __FILE__, __LINE__, packet.data(), + "Too short 'options' in MultiMemRead packet"); + if (packet[0] != ';') + return HandlePacket_ILLFORMED(__FILE__, __LINE__, packet.data(), + "Unimplemented 'options' in MultiMemRead"); + packet.remove_prefix(1); + } + + if (!packet.empty()) + return HandlePacket_ILLFORMED(__FILE__, __LINE__, p, + "Invalid MultiMemRead address_range"); + + std::vector<std::vector<uint8_t>> buffers; + buffers.reserve(ranges.size()); + for (auto [base_addr, length] : ranges) { + buffers.emplace_back(length, '\0'); + nub_size_t bytes_read = DNBProcessMemoryRead(m_ctx.ProcessID(), base_addr, + length, buffers.back().data()); + buffers.back().resize(bytes_read); + } + + std::ostringstream reply_stream; + bool first = true; + for (const std::vector<uint8_t> &buffer : buffers) { + reply_stream << (first ? "" : ",") << std::hex << buffer.size(); + first = false; + } + reply_stream << ';'; + + for (const std::vector<uint8_t> &buffer : buffers) + binary_encode_data_vector(reply_stream, buffer); + + return SendPacket(reply_stream.str()); +} + // Read memory, sent it up as binary data. // Usage: xADDR,LEN // ADDR and LEN are both base 16. @@ -3525,6 +3664,7 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) { if (supports_memory_tagging()) reply << "memory-tagging+;"; + reply << "MultiMemRead+;"; return SendPacket(reply.str().c_str()); } diff --git a/lldb/tools/debugserver/source/RNBRemote.h b/lldb/tools/debugserver/source/RNBRemote.h index cf1c978afcd23..b32c00adc8b23 100644 --- a/lldb/tools/debugserver/source/RNBRemote.h +++ b/lldb/tools/debugserver/source/RNBRemote.h @@ -136,6 +136,7 @@ class RNBRemote { query_transfer, // 'qXfer:' json_query_dyld_process_state, // 'jGetDyldProcessState' enable_error_strings, // 'QEnableErrorStrings' + multi_mem_read, // 'MultiMemRead' unknown_type }; // clang-format on @@ -216,6 +217,7 @@ class RNBRemote { rnb_err_t HandlePacket_last_signal(const char *p); rnb_err_t HandlePacket_m(const char *p); rnb_err_t HandlePacket_M(const char *p); + rnb_err_t HandlePacket_MultiMemRead(const char *p); rnb_err_t HandlePacket_x(const char *p); rnb_err_t HandlePacket_X(const char *p); rnb_err_t HandlePacket_z(const char *p); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
