[Lldb-commits] [lldb] e89dc21 - [lldb] Stop a cached read at an invalid range (#218997)

[via lldb-commits]

Author: Yao Qi
Date: 2026-08-27T09:06:03+01:00
New Revision: e89dc2172f0da02216fa3caa9d7a1acbd4d381de

URL: https://github.com/llvm/llvm-project/commit/e89dc2172f0da02216fa3caa9d7a1acbd4d381de
DIFF: https://github.com/llvm/llvm-project/commit/e89dc2172f0da02216fa3caa9d7a1acbd4d381de.diff

LOG: [lldb] Stop a cached read at an invalid range (#218997)

`MemoryCache::Read` refused a read only when its first byte sat in a
range
the process had recorded as invalid. A read whose interior met such a
range
was served in full, out of a cache line fetched straight across it,
which is
what the `FIXME` in that function described:

```
  // FIXME: We should do a more thorough check to make sure that we're not
  // overlapping with any invalid ranges (e.g. Read 0x100 - 0x200 but there's an
  // invalid range 0x180 - 0x280).
```

With an invalid range at `[0xE010, 0xE020)`, a 64-byte read at `0xE000`
returned 64 bytes and a success status.

Test the whole requested range for an intersection instead.  Report the
failure, and shorten the read to the bytes below the range, so a caller
gets
the readable prefix and an error rather than data the process said is
not
there.  A read starting inside a range still serves nothing.

Added: 
    

Modified: 
    lldb/source/Target/Memory.cpp
    lldb/unittests/Target/MemoryTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp
index 223a21b802d94..5782be0d92f85 100644
--- a/lldb/source/Target/Memory.cpp
+++ b/lldb/source/Target/Memory.cpp
@@ -193,16 +193,16 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len,
     return 0;
 
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  // FIXME: We should do a more thorough check to make sure that we're not
-  // overlapping with any invalid ranges (e.g. Read 0x100 - 0x200 but there's an
-  // invalid range 0x180 - 0x280). `FindEntryThatContains` has an implementation
-  // that takes a range, but it only checks to see if the argument is contained
-  // by an existing invalid range. It cannot check if the argument contains
-  // invalid ranges and cannot check for overlaps.
-  if (m_invalid_ranges.FindEntryThatContains(addr)) {
+
+  if (const InvalidRanges::Entry *invalid =
+          m_invalid_ranges.FindEntryThatIntersects(
+              InvalidRanges::Entry(addr, dst_len))) {
+    const addr_t invalid_addr = invalid->GetRangeBase();
     error = Status::FromErrorStringWithFormat(
-        "memory read failed for 0x%" PRIx64, addr);
-    return 0;
+        "memory read failed for 0x%" PRIx64, invalid_addr);
+    if (invalid_addr <= addr)
+      return 0;
+    dst_len = invalid_addr - addr;
   }
 
   // Check the L1 cache for a range that contains the entire memory read.

diff  --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index 4501de43b2d1a..5679d863f62d4 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -406,6 +406,45 @@ TEST_F(MemoryTest, TestL1Cache) {
   expect_l1({{0x9100, 0x80, 0xCC}});
 }
 
+TEST_F(MemoryTest, TestReadStopsAtAnInvalidRange) {
+  ArchSpec arch("arm64-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);
+
+  ProcessSP process_sp = CreateProcess(target_sp);
+  ASSERT_TRUE(process_sp);
+
+  DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
+  MemoryCache &cache = process->GetMemoryCache();
+  const lldb::addr_t base = 0xE000;
+
+  cache.AddInvalidRange(base + 16, 16);
+  process->SetMaxReadSize(4096);
+  process->SetFiller(0xBB);
+
+  // Only the bytes below the invalid range are served, and the read reports
+  // the failure.
+  Status error;
+  std::vector<uint8_t> buf(64, 0);
+  EXPECT_EQ(cache.Read(base, buf.data(), buf.size(), error), 16u);
+  EXPECT_TRUE(error.Fail());
+  for (size_t i = 0; i < 16; ++i)
+    EXPECT_EQ(buf[i], 0xBB) << "byte " << i;
+
+  // A read starting inside the range has nothing to serve.
+  Status inside_error;
+  std::vector<uint8_t> inside(8, 0);
+  EXPECT_EQ(cache.Read(base + 20, inside.data(), inside.size(), inside_error),
+            0u);
+  EXPECT_TRUE(inside_error.Fail());
+}
+
 TEST_F(MemoryTest, TestReadInteger) {
   ArchSpec arch("x86_64-apple-macosx-");