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

[David Spickett via lldb-commits]

================
@@ -225,3 +226,129 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
                                                        // instead of using an
                                                        // old cache
 }
+
+/// 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:
+  // 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;
+  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.
+    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);
+
+  {
+    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);
+
+    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));
+    }
+  }
+}
+
+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(
----------------
DavidSpickett wrote:

Though I would like to see the non-assert path anyway. So you'll need an ifdef for that.

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