[Lldb-commits] [lldb] [lldb] Implement Process::ReadMemoryRanges (PR #163651)

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Tue Oct 21 08:12:04 PDT 2025


https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/163651

>From 5b143ddaad4a8369bd31b78b0a5fbf41140f0288 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 14 Oct 2025 08:07:51 -0700
Subject: [PATCH 1/6] [lldb] Implement Process::ReadMemoryRanges

This commit introduces a base-class implementation for a method that
reads memory from multiple ranges at once. This implementation simply
calls the underlying `ReadMemoryFromInferior` method on each requested
range, intentionally bypassing the memory caching mechanism (though this
may be easily changed in the future).

`Process` implementations that can be perform this operation more
efficiently - e.g. with the MultiMemPacket described in [1] - are
expected to override this method.

As an example, this commit changes AppleObjCClassDescriptorV2 to use the
new API.

Note about the API
------------------

In the RFC, we discussed having the API return some kind of class
`ReadMemoryRangesResult`. However, while writing such a class, it became
clear that it was merely wrapping a vector, without providing anything
useful. For example, this class:

```
struct ReadMemoryRangesResult {
  ReadMemoryRangesResult(
      llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges)
      : ranges(std::move(ranges)) {}

  llvm::ArrayRef<llvm::MutableArrayRef<uint8_t>> getRanges() const {
    return ranges;
  }

private:
  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges;
};
```

As can be seen in the added test and in the added use-case
(AppleObjCClassDescriptorV2), users of this API will just iterate over
the vector of memory buffers. So they want a return type that can be
iterated over, and the vector seems more natural than creating a new
class and defining iterators for it.

Likewise, in the RFC, we discussed wrapping the result into an
`Expected`. Upon experimenting with the code, this feels like it limits
what the API is able to do as the base class implementation never needs
to fail the entire result, it's the individual reads that may fail and
this is expressed through a zero-length result. Any derived classes
overriding `ReadMemoryRanges` should also never produce a top level
failure: if they did, they can just fall back to the base class
implementation, which would produce a better result.

The choice of having the caller allocate a buffer and pass it to
`Process::ReadMemoryRanges` is done mostly to follow conventions already
done in the Process class.

[1]: https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/
---
 lldb/include/lldb/Target/Process.h            | 19 ++++++
 .../AppleObjCClassDescriptorV2.cpp            | 23 +++----
 lldb/source/Target/Process.cpp                | 28 +++++++++
 lldb/unittests/Target/MemoryTest.cpp          | 62 +++++++++++++++++++
 4 files changed, 121 insertions(+), 11 deletions(-)

diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index dc75d98acea70..7a260323b5a3d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1571,6 +1571,25 @@ class Process : public std::enable_shared_from_this<Process>,
   virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
                             Status &error);
 
+  /// Read from multiple memory ranges and write the results into buffer.
+  ///
+  /// \param[in] ranges
+  ///     A collection of ranges (base address + size) to read from.
+  ///
+  /// \param[out] buffer
+  ///     A buffer where the read memory will be written to. It must be at least
+  ///     as long as the sum of the sizes of each range.
+  ///
+  /// \return
+  ///     A vector of MutableArrayRef, where each MutableArrayRef is a slice of
+  ///     the input buffer into which the memory contents were copied. The size
+  ///     of the slice indicates how many bytes were read successfully. Partial
+  ///     reads are always performed from the start of the requested range,
+  ///     never from the middle or end.
+  virtual llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
+  ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
+                   llvm::MutableArrayRef<uint8_t> buffer);
+
   /// Read of memory from a process.
   ///
   /// This function has the same semantics of ReadMemory except that it
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
index 6d8f41aef1ffc..7ce4cbf4c61a4 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -279,22 +279,23 @@ ClassDescriptorV2::ReadMethods(llvm::ArrayRef<lldb::addr_t> addresses,
   const size_t num_methods = addresses.size();
 
   llvm::SmallVector<uint8_t, 0> buffer(num_methods * size, 0);
-  llvm::DenseSet<uint32_t> failed_indices;
 
-  for (auto [idx, addr] : llvm::enumerate(addresses)) {
-    Status error;
-    process->ReadMemory(addr, buffer.data() + idx * size, size, error);
-    if (error.Fail())
-      failed_indices.insert(idx);
-  }
+  llvm::SmallVector<Range<addr_t, size_t>> mem_ranges =
+      llvm::to_vector(llvm::map_range(llvm::seq(num_methods), [&](size_t idx) {
+        return Range<addr_t, size_t>(addresses[idx], size);
+      }));
+
+  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+      process->ReadMemoryRanges(mem_ranges, buffer);
 
   llvm::SmallVector<method_t, 0> methods;
   methods.reserve(num_methods);
-  for (auto [idx, addr] : llvm::enumerate(addresses)) {
-    if (failed_indices.contains(idx))
+  for (auto [addr, memory] : llvm::zip(addresses, read_results)) {
+    // Ignore partial reads.
+    if (memory.size() != size)
       continue;
-    DataExtractor extractor(buffer.data() + idx * size, size,
-                            process->GetByteOrder(),
+
+    DataExtractor extractor(memory.data(), size, process->GetByteOrder(),
                             process->GetAddressByteSize());
     methods.push_back(method_t());
     methods.back().Read(extractor, process, addr, relative_selector_base_addr,
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 3176852f0b724..9692a7fece7e5 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1971,6 +1971,34 @@ size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) {
   }
 }
 
+llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
+Process::ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
+                          llvm::MutableArrayRef<uint8_t> buffer) {
+  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> results;
+
+  for (auto [addr, len] : ranges) {
+    // This is either a programmer error, or a protocol violation.
+    // In production builds, gracefully fail.
+    assert(buffer.size() >= len);
+    if (buffer.size() < len) {
+      results.push_back(buffer.take_front(0));
+      continue;
+    }
+
+    Status status;
+    size_t num_bytes_read =
+        ReadMemoryFromInferior(addr, buffer.data(), len, status);
+    // FIXME: ReadMemoryFromInferior promises to return 0 in case of errors, but
+    // it doesn't; it never checks for errors.
+    if (status.Fail())
+      num_bytes_read = 0;
+    results.push_back(buffer.take_front(num_bytes_read));
+    buffer = buffer.drop_front(num_bytes_read);
+  }
+
+  return results;
+}
+
 void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
                              const uint8_t *buf, size_t size,
                              AddressRanges &matches, size_t alignment,
diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index 4a96730e00464..3bf7b75728c65 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -17,6 +17,8 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "gtest/gtest.h"
+#include <cstdint>
+#include <random>
 
 using namespace lldb_private;
 using namespace lldb;
@@ -225,3 +227,63 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
                                                        // instead of using an
                                                        // old cache
 }
+
+/// A process class that reads `lower_byte(address)` for each `address` it
+/// reads.
+class DummyReaderProcess : public Process {
+public:
+  size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
+                      Status &error) override {
+    uint8_t *buffer = static_cast<uint8_t *>(buf);
+    for (size_t addr = vm_addr; addr < vm_addr + size; addr++)
+      buffer[addr - vm_addr] = addr;
+    return size;
+  }
+  // Boilerplate, nothing interesting below.
+  DummyReaderProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+      : Process(target_sp, listener_sp) {}
+  bool CanDebug(lldb::TargetSP, bool) override { return true; }
+  Status DoDestroy() override { return {}; }
+  void RefreshStateAfterStop() override {}
+  bool DoUpdateThreadList(ThreadList &, ThreadList &) override { return false; }
+  llvm::StringRef GetPluginName() override { return "Dummy"; }
+};
+
+TEST_F(MemoryTest, TestReadMemoryRanges) {
+  ArchSpec arch("x86_64-apple-macosx-");
+
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+
+  ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  ProcessSP process_sp =
+      std::make_shared<DummyReaderProcess>(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+
+  DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
+  process->SetMaxReadSize(1024);
+
+  llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
+
+  // Read 8 ranges of 128 bytes, starting at random addresses
+  std::mt19937 rng(42);
+  std::uniform_int_distribution<addr_t> distribution(1, 100000);
+  llvm::SmallVector<Range<addr_t, size_t>> ranges;
+  for (unsigned i = 0; i < 1024; i += 128)
+    ranges.emplace_back(distribution(rng), 128);
+
+  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+      process->ReadMemoryRanges(ranges, buffer);
+
+  for (auto [range, memory] : llvm::zip(ranges, read_results)) {
+    ASSERT_EQ(memory.size(), 128u);
+    addr_t range_base = range.GetRangeBase();
+    for (auto [idx, byte] : llvm::enumerate(memory))
+      ASSERT_EQ(byte, static_cast<uint8_t>(range_base + idx));
+  }
+}

>From 58d7eda862c0219e4798a00f4834ad9ce6e5eb2c Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Mon, 20 Oct 2025 09:52:06 -0700
Subject: [PATCH 2/6] fixup! Address review comments

---
 lldb/include/lldb/Target/Process.h   |  3 +++
 lldb/source/Target/Process.cpp       | 31 +++++++++++++++++++---------
 lldb/unittests/Target/MemoryTest.cpp | 18 +++++++---------
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 7a260323b5a3d..8f5892e16cedf 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1572,6 +1572,9 @@ class Process : public std::enable_shared_from_this<Process>,
                             Status &error);
 
   /// Read from multiple memory ranges and write the results into buffer.
+  /// This calls ReadMemoryFromInferior multiple times, once per range,
+  /// bypassing the read cache. Process implementations that can perform this
+  /// operation more efficiently should override this.
   ///
   /// \param[in] ranges
   ///     A collection of ranges (base address + size) to read from.
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 9692a7fece7e5..e4375d4d2b50e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1974,25 +1974,36 @@ size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) {
 llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
 Process::ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
                           llvm::MutableArrayRef<uint8_t> buffer) {
-  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> results;
+  auto total_ranges_len = llvm::sum_of(
+      llvm::map_range(ranges, [](auto range) { return range.size; }));
+  // If the buffer is not large enough, this is a programmer error.
+  // In production builds, gracefully fail by returning empty chunks.
+  assert(buffer.size() >= total_ranges_len);
+  if (buffer.size() < total_ranges_len)
+    return llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>(ranges.size());
 
-  for (auto [addr, len] : ranges) {
-    // This is either a programmer error, or a protocol violation.
-    // In production builds, gracefully fail.
-    assert(buffer.size() >= len);
-    if (buffer.size() < len) {
-      results.push_back(buffer.take_front(0));
-      continue;
-    }
+  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> results;
 
+  // While `buffer` has space, take the next requested range and read
+  // memory into a `buffer` chunk, then slice it to remove the used chunk.
+  for (auto [addr, range_len] : ranges) {
     Status status;
     size_t num_bytes_read =
-        ReadMemoryFromInferior(addr, buffer.data(), len, status);
+        ReadMemoryFromInferior(addr, buffer.data(), range_len, status);
     // FIXME: ReadMemoryFromInferior promises to return 0 in case of errors, but
     // it doesn't; it never checks for errors.
     if (status.Fail())
       num_bytes_read = 0;
+
+    assert(num_bytes_read <= range_len && "read more than requested bytes");
+    if (num_bytes_read > range_len) {
+      // In production builds, gracefully fail by returning an empty chunk.
+      results.emplace_back();
+      continue;
+    }
+
     results.push_back(buffer.take_front(num_bytes_read));
+    // Slice buffer to remove the used chunk.
     buffer = buffer.drop_front(num_bytes_read);
   }
 
diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index 3bf7b75728c65..2543376897fc4 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -18,7 +18,6 @@
 #include "lldb/Utility/DataBufferHeap.h"
 #include "gtest/gtest.h"
 #include <cstdint>
-#include <random>
 
 using namespace lldb_private;
 using namespace lldb;
@@ -228,15 +227,15 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
                                                        // old cache
 }
 
-/// A process class that reads `lower_byte(address)` for each `address` it
-/// reads.
+/// A process class that, when asked to read memory from some address X, returns
+/// the least significant byte of X.
 class DummyReaderProcess : public Process {
 public:
   size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
                       Status &error) override {
     uint8_t *buffer = static_cast<uint8_t *>(buf);
     for (size_t addr = vm_addr; addr < vm_addr + size; addr++)
-      buffer[addr - vm_addr] = addr;
+      buffer[addr - vm_addr] = static_cast<uint8_t>(addr); // LSB of addr.
     return size;
   }
   // Boilerplate, nothing interesting below.
@@ -270,12 +269,11 @@ TEST_F(MemoryTest, TestReadMemoryRanges) {
 
   llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
 
-  // Read 8 ranges of 128 bytes, starting at random addresses
-  std::mt19937 rng(42);
-  std::uniform_int_distribution<addr_t> distribution(1, 100000);
-  llvm::SmallVector<Range<addr_t, size_t>> ranges;
-  for (unsigned i = 0; i < 1024; i += 128)
-    ranges.emplace_back(distribution(rng), 128);
+  // Read 8 ranges of 128 bytes with arbitrary base addresses.
+  llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+      {0x12345, 128},      {0x11112222, 128}, {0x77777777, 128},
+      {0xffaabbccdd, 128}, {0x0, 128},        {0x4242424242, 128},
+      {0x17171717, 128},   {0x99999, 128}};
 
   llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
       process->ReadMemoryRanges(ranges, buffer);

>From 1bcac3440f8f31f1e9dd4e0d6801dc485204e535 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 21 Oct 2025 07:19:16 -0700
Subject: [PATCH 3/6] fixup! Remove dead code from test

---
 lldb/unittests/Target/MemoryTest.cpp | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index 2543376897fc4..390b07d718fab 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -264,9 +264,6 @@ TEST_F(MemoryTest, TestReadMemoryRanges) {
       std::make_shared<DummyReaderProcess>(target_sp, listener_sp);
   ASSERT_TRUE(process_sp);
 
-  DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
-  process->SetMaxReadSize(1024);
-
   llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
 
   // Read 8 ranges of 128 bytes with arbitrary base addresses.
@@ -276,7 +273,7 @@ TEST_F(MemoryTest, TestReadMemoryRanges) {
       {0x17171717, 128},   {0x99999, 128}};
 
   llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
-      process->ReadMemoryRanges(ranges, buffer);
+      process_sp->ReadMemoryRanges(ranges, buffer);
 
   for (auto [range, memory] : llvm::zip(ranges, read_results)) {
     ASSERT_EQ(memory.size(), 128u);

>From 52aae17338cd97d094cb7885b0ba92dc102737ee Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 21 Oct 2025 07:41:48 -0700
Subject: [PATCH 4/6] fixup! Add test for reading fewer bytes than requested

---
 lldb/unittests/Target/MemoryTest.cpp | 52 +++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index 390b07d718fab..f4d132dba2978 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -231,8 +231,16 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
 /// the least significant byte of X.
 class DummyReaderProcess : public Process {
 public:
+  // If true, `DoReadMemory` will not return all requested bytes.
+  // It's not possible to control exactly how many bytes will be read, because
+  // Process::ReadMemoryFromInferior tries to fulfill the entire request by
+  // reading smaller chunks until it gets nothing back.
+  bool read_less_than_requested = false;
+
   size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
                       Status &error) override {
+    if (read_less_than_requested && size > 0)
+      size--;
     uint8_t *buffer = static_cast<uint8_t *>(buf);
     for (size_t addr = vm_addr; addr < vm_addr + size; addr++)
       buffer[addr - vm_addr] = static_cast<uint8_t>(addr); // LSB of addr.
@@ -264,21 +272,39 @@ TEST_F(MemoryTest, TestReadMemoryRanges) {
       std::make_shared<DummyReaderProcess>(target_sp, listener_sp);
   ASSERT_TRUE(process_sp);
 
-  llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
 
-  // Read 8 ranges of 128 bytes with arbitrary base addresses.
-  llvm::SmallVector<Range<addr_t, size_t>> ranges = {
-      {0x12345, 128},      {0x11112222, 128}, {0x77777777, 128},
-      {0xffaabbccdd, 128}, {0x0, 128},        {0x4242424242, 128},
-      {0x17171717, 128},   {0x99999, 128}};
+  {
+    llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
+    // Read 8 ranges of 128 bytes with arbitrary base addresses.
+    llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+        {0x12345, 128},      {0x11112222, 128}, {0x77777777, 128},
+        {0xffaabbccdd, 128}, {0x0, 128},        {0x4242424242, 128},
+        {0x17171717, 128},   {0x99999, 128}};
 
-  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
-      process_sp->ReadMemoryRanges(ranges, buffer);
+    llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+        process_sp->ReadMemoryRanges(ranges, buffer);
 
-  for (auto [range, memory] : llvm::zip(ranges, read_results)) {
-    ASSERT_EQ(memory.size(), 128u);
-    addr_t range_base = range.GetRangeBase();
-    for (auto [idx, byte] : llvm::enumerate(memory))
-      ASSERT_EQ(byte, static_cast<uint8_t>(range_base + idx));
+    for (auto [range, memory] : llvm::zip(ranges, read_results)) {
+      ASSERT_EQ(memory.size(), 128u);
+      addr_t range_base = range.GetRangeBase();
+      for (auto [idx, byte] : llvm::enumerate(memory))
+        ASSERT_EQ(byte, static_cast<uint8_t>(range_base + idx));
+    }
+  }
+
+  auto &dummy_process = static_cast<DummyReaderProcess&>(*process_sp);
+  dummy_process.read_less_than_requested = true;
+  {
+    llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
+    llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+        {0x12345, 128}, {0x11112222, 128}, {0x77777777, 128}};
+    llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+        dummy_process.ReadMemoryRanges(ranges, buffer);
+    for (auto [range, memory] : llvm::zip(ranges, read_results)) {
+      ASSERT_LT(memory.size(), 128u);
+      addr_t range_base = range.GetRangeBase();
+      for (auto [idx, byte] : llvm::enumerate(memory))
+        ASSERT_EQ(byte, static_cast<uint8_t>(range_base + idx));
+    }
   }
 }

>From d498717af6913812e30051f9b6eebf47b9330782 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 21 Oct 2025 07:53:30 -0700
Subject: [PATCH 5/6] fixup! rewrite empty response for clarity

---
 lldb/source/Target/Process.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index e4375d4d2b50e..bb0b553a9d898 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1979,8 +1979,10 @@ Process::ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
   // If the buffer is not large enough, this is a programmer error.
   // In production builds, gracefully fail by returning empty chunks.
   assert(buffer.size() >= total_ranges_len);
-  if (buffer.size() < total_ranges_len)
-    return llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>(ranges.size());
+  if (buffer.size() < total_ranges_len) {
+    llvm::MutableArrayRef<uint8_t> empty;
+    return {ranges.size(), empty};
+  }
 
   llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> results;
 

>From 1b470fbba124383337b33d007e20cfe25e67e35d Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 21 Oct 2025 08:09:12 -0700
Subject: [PATCH 6/6] fixup! add death tests

---
 lldb/source/Target/Process.cpp       |  2 +-
 lldb/unittests/Target/MemoryTest.cpp | 45 ++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index bb0b553a9d898..b749601552cc2 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1978,7 +1978,7 @@ Process::ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
       llvm::map_range(ranges, [](auto range) { return range.size; }));
   // If the buffer is not large enough, this is a programmer error.
   // In production builds, gracefully fail by returning empty chunks.
-  assert(buffer.size() >= total_ranges_len);
+  assert(buffer.size() >= total_ranges_len && "provided buffer is too short");
   if (buffer.size() < total_ranges_len) {
     llvm::MutableArrayRef<uint8_t> empty;
     return {ranges.size(), empty};
diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index f4d132dba2978..1380503362a5d 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -236,11 +236,14 @@ class DummyReaderProcess : public Process {
   // Process::ReadMemoryFromInferior tries to fulfill the entire request by
   // reading smaller chunks until it gets nothing back.
   bool read_less_than_requested = false;
+  bool read_more_than_requested = false;
 
   size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
                       Status &error) override {
     if (read_less_than_requested && size > 0)
       size--;
+    if (read_more_than_requested)
+      size *= 2;
     uint8_t *buffer = static_cast<uint8_t *>(buf);
     for (size_t addr = vm_addr; addr < vm_addr + size; addr++)
       buffer[addr - vm_addr] = static_cast<uint8_t>(addr); // LSB of addr.
@@ -308,3 +311,45 @@ TEST_F(MemoryTest, TestReadMemoryRanges) {
     }
   }
 }
+
+using MemoryDeathTest = MemoryTest;
+
+TEST_F(MemoryDeathTest, TestReadMemoryRangesReturnsTooMuch) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+  TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+  ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  ProcessSP process_sp =
+      std::make_shared<DummyReaderProcess>(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+
+  auto &dummy_process = static_cast<DummyReaderProcess &>(*process_sp);
+  dummy_process.read_more_than_requested = true;
+  llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
+  llvm::SmallVector<Range<addr_t, size_t>> ranges = {{0x12345, 128}};
+  ASSERT_DEATH(
+      { process_sp->ReadMemoryRanges(ranges, buffer); },
+      "read more than requested bytes");
+}
+
+TEST_F(MemoryDeathTest, TestReadMemoryRangesWithShortBuffer) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+  TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+  ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  ProcessSP process_sp =
+      std::make_shared<DummyReaderProcess>(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+
+  llvm::SmallVector<uint8_t, 0> too_short_buffer(10, 0);
+  llvm::SmallVector<Range<addr_t, size_t>> ranges = {{0x12345, 128}};
+  ASSERT_DEATH(
+      { process_sp->ReadMemoryRanges(ranges, too_short_buffer); },
+      "provided buffer is too short");
+}



More information about the lldb-commits mailing list