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-"); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
