[debugserver] Implement MultiMemRead packet (#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

Author: felipepiovezan

lldb/test/API/macosx/debugserver-multimemread/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/macosx/debugserver-multimemread/TestDebugserverMultiMemRead.py

@@ -0,0 +1,102 @@
+"""
+Tests debugserver support for MultiMemRead.
+"""
+
+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")
+        packet = reply[0].strip()
+        response = reply[1].strip()
+
+        self.assertTrue(packet.startswith("packet: "))
+        self.assertTrue(response.startswith("response: "))
+        return response[len("response: ") :]
+
+    def check_invalid_packet(self, packet_str):
+        reply = self.send_process_packet("packet_str")
+        self.assertEqual(reply, "E03")
+
+    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 = mem_address_var.GetValueAsUnsigned() + 42
+
+        # no ":"
+        self.check_invalid_packet("MultiMemRead")
+        # missing ranges
+        self.check_invalid_packet("MultiMemRead:")
+        # needs at least one range
+        self.check_invalid_packet("MultiMemRead:ranges:")
+        # needs at least one range
+        self.check_invalid_packet("MultiMemRead:ranges:,")
+        # a range is a pair of numbers
+        self.check_invalid_packet("MultiMemRead:ranges:10")
+        # a range is a pair of numbers
+        self.check_invalid_packet("MultiMemRead:ranges:10,")
+        # range list must end with ;
+        self.check_invalid_packet("MultiMemRead:ranges:10,2")
+        self.check_invalid_packet("MultiMemRead:ranges:10,2,")
+        self.check_invalid_packet("MultiMemRead:ranges:10,2,3")
+        # ranges are pairs of numbers.
+        self.check_invalid_packet("MultiMemRead:ranges:10,2,3;")
+        # unrecognized field
+        self.check_invalid_packet("MultiMemRead:ranges:10,2;blah:;")
+        # unrecognized field
+        self.check_invalid_packet("MultiMemRead:blah:;ranges:10,2;")
+
+        # Zero-length reads are ok.
+        reply = self.send_process_packet("MultiMemRead:ranges:0,0;")
+        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},2,;")
+        self.assertEqual(reply, "2;ab")
+
+        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},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")
+
+        # Test zero length in the middle.
+        reply = self.send_process_packet(
+            f"MultiMemRead:ranges:{mem_address:x},2,{mem_address+2:x},0,{mem_address+6:x},8;"
+        )
+        self.assertEqual(reply, "2,0,8;abghijklmn")
+        # Test zero length in the end.
+        reply = self.send_process_packet(
+            f"MultiMemRead:ranges:{mem_address:x},2,{mem_address+2:x},4,{mem_address+6:x},0;"
+        )
+        self.assertEqual(reply, "2,4,0;abcdef")

lldb/test/API/macosx/debugserver-multimemread/main.c

@@ -0,0 +1,14 @@
+#include <stdlib.h>
+#include <string.h>
+
+int main(int argc, char **argv) {
+  char *memory = malloc(1024);
+  memset(memory, '-', 1024);
+  // Write "interesting" characters at an offset from the memory filled with
+  // `-`. This way, if we read outside the range in either direction, we should
+  // find `-`s`.
+  int offset = 42;
+  for (int i = offset; i < offset + 14; i++)
+    memory[i] = 'a' + (i - offset);
+  return 0; // break here
+}

lldb/tools/debugserver/source/RNBRemote.cpp

@@ -170,6 +170,20 @@ static uint64_t decode_uint64(const char *p, int base, char **end = nullptr,
   return addr;
 }
 
+/// Attempts to parse a prefix of `number_str` as a uint64_t. If
+/// successful, the number is returned and the prefix is dropped from
+/// `number_str`.
+static std::optional<uint64_t> extract_u64(std::string_view &number_str) {
+  char *str_end = nullptr;
+  errno = 0;
+  uint64_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;
+}
+
 static void append_hex_value(std::ostream &ostrm, const void *buf,
                              size_t buf_size, bool swap) {
   int i;
@@ -204,6 +218,25 @@ static void append_hexified_string(std::ostream &ostrm,
   }
 }
 
+/// Returns true if `str` starts with `prefix`.
+static bool starts_with(std::string_view str, std::string_view prefix) {
+  return str.substr(0, prefix.size()) == prefix;
+}
+
+/// 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;
+}
+
 // from System.framework/Versions/B/PrivateHeaders/sys/codesign.h
 extern "C" {
 #define CS_OPS_STATUS 0       /* return status */
@@ -270,6 +303,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",
@@ -3150,6 +3188,88 @@ rnb_err_t RNBRemote::HandlePacket_m(const char *p) {
   return SendPacket(ostrm.str());
 }
 
+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<uint64_t> maybe_addr = extract_u64(numbers_list[idx]);
+    std::optional<uint64_t> maybe_length = extract_u64(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 (!packet.empty())
+    return HandlePacket_ILLFORMED(
+        __FILE__, __LINE__, p, "MultiMemRead packet has unrecognized fields");
+
+  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.
@@ -3503,6 +3623,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());
 }
 

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);