Author: Yao Qi Date: 2026-07-30T10:10:28+01:00 New Revision: 6ddd80af0bfbc1927f8bb2935120f6f4cedadc9a
URL: https://github.com/llvm/llvm-project/commit/6ddd80af0bfbc1927f8bb2935120f6f4cedadc9a DIFF: https://github.com/llvm/llvm-project/commit/6ddd80af0bfbc1927f8bb2935120f6f4cedadc9a.diff LOG: [lldb] Fix stale L1 memory cache read after memory write (#208347) A `memory write` can leave stale bytes in the L1 memory cache, so a later `memory read` of the address that was just written returns the old value. The L1 cache (`m_L1_cache`) is a map keyed by each chunk's start address, and chunks can overlap: a read larger than an L2 cache line (`target.memory-cache-line-size`, 512 by default) bypasses L2 and is stored whole in L1, so two large reads can produce two chunks that both cover the same address. `Flush()` invalidates the L1 cache on a write. It started at the chunk at or below the flushed address and walked forward, stopping at the first chunk that did not intersect. It therefore never inspected a chunk that starts below the flushed address but is long enough to reach into it, leaving that chunk behind with the stale byte. A later read fully contained in that chunk is served from the cache and returns the old value. Fix `Flush()` to walk the whole L1 cache and erase every chunk that intersects the flushed range, so a chunk starting below the address is dropped too. Lookups (`FindL1CacheEntry()`) and inserts (`AddL1CacheData()`) are unaffected: a lookup that lands on the wrong overlapping chunk simply misses and falls through to L2 or the inferior, which is not a correctness problem. Add a unit test covering partial overlaps, containment, disjoint and adjacent chunks, and a flush that must drop a lower-starting chunk as well as several overlapping chunks. 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 f70da27ec058b..6e372fdf3fbee 100644 --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -57,18 +57,18 @@ void MemoryCache::Flush(addr_t addr, size_t size) { std::lock_guard<std::recursive_mutex> guard(m_mutex); - // Erase any blocks from the L1 cache that intersect with the flush range + // L1 chunks can overlap, and a chunk starting below addr can still reach + // into the flushed range, so scan the whole L1 cache and erase every chunk + // that intersects it. if (!m_L1_cache.empty()) { AddrRange flush_range(addr, size); - BlockMap::iterator pos = m_L1_cache.upper_bound(addr); - if (pos != m_L1_cache.begin()) { - --pos; - } + BlockMap::iterator pos = m_L1_cache.begin(); while (pos != m_L1_cache.end()) { AddrRange chunk_range(pos->first, pos->second->GetByteSize()); - if (!chunk_range.DoesIntersect(flush_range)) - break; - pos = m_L1_cache.erase(pos); + if (chunk_range.DoesIntersect(flush_range)) + pos = m_L1_cache.erase(pos); + else + ++pos; } } diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index 9d04376b4fd5b..f89e9215de713 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -120,6 +120,15 @@ class DummyProcess : public Process { void SetMaxReadSize(size_t size) { m_bytes_left = size; } void SetFiller(int filler) { m_filler = filler; } }; + +// A MemoryCache subclass that exposes the otherwise-protected L1 cache so a +// test can assert on the exact set of chunks it holds. +class TestMemoryCache : public MemoryCache { +public: + using MemoryCache::MemoryCache; + + const BlockMap &GetL1Cache() const { return m_L1_cache; } +}; } // namespace TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { @@ -283,6 +292,114 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) { // old cache } +TEST_F(MemoryTest, TestL1Cache) { + 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()); + TestMemoryCache mem_cache(*process); + + auto add = [&](lldb::addr_t addr, size_t size, uint8_t fill) { + mem_cache.AddL1CacheData(addr, + std::make_shared<DataBufferHeap>(size, fill)); + }; + + // Asserts the L1 cache holds exactly `expected` chunks, matched by start + // address, byte size, and a single repeated fill byte, in address order. + struct Chunk { + lldb::addr_t addr; + size_t size; + uint8_t fill; + }; + auto expect_l1 = [&](std::vector<Chunk> expected) { + const auto &l1 = mem_cache.GetL1Cache(); + ASSERT_EQ(l1.size(), expected.size()); + size_t i = 0; + for (const auto &[addr, data_sp] : l1) { + const Chunk &c = expected[i++]; + EXPECT_EQ(addr, c.addr); + ASSERT_EQ(data_sp->GetByteSize(), c.size); + const uint8_t *bytes = data_sp->GetBytes(); + for (size_t j = 0; j < c.size; ++j) + EXPECT_EQ(bytes[j], c.fill) + << "chunk 0x" << std::hex << addr << " byte " << std::dec << j; + } + }; + + // Partial overlap: the new chunk overhangs the existing one on the right. + mem_cache.Clear(); + add(0x1000, 0x100, 0xAA); + add(0x1080, 0x100, 0xBB); + expect_l1({{0x1000, 0x100, 0xAA}, {0x1080, 0x100, 0xBB}}); + + // Partial overlap: the new chunk overhangs the existing one on the left. + mem_cache.Clear(); + add(0x2080, 0x100, 0xAA); + add(0x2000, 0x100, 0xBB); + expect_l1({{0x2000, 0x100, 0xBB}, {0x2080, 0x100, 0xAA}}); + + // New chunk fully contains an existing one: both are kept. + mem_cache.Clear(); + add(0x3040, 0x40, 0xAA); + add(0x3000, 0x100, 0xBB); + expect_l1({{0x3000, 0x100, 0xBB}, {0x3040, 0x40, 0xAA}}); + + // New chunk is fully contained by an existing one: both are kept. + mem_cache.Clear(); + add(0x4000, 0x200, 0xAA); + add(0x4080, 0x80, 0xBB); + expect_l1({{0x4000, 0x200, 0xAA}, {0x4080, 0x80, 0xBB}}); + + // New chunk partially overlaps two existing chunks; all three are kept. + mem_cache.Clear(); + add(0x5000, 0x80, 0xAA); + add(0x5100, 0x80, 0xCC); + add(0x5040, 0x100, 0xBB); + expect_l1( + {{0x5000, 0x80, 0xAA}, {0x5040, 0x100, 0xBB}, {0x5100, 0x80, 0xCC}}); + + // Disjoint chunks stay separate. + mem_cache.Clear(); + add(0x6000, 0x80, 0xAA); + add(0x6100, 0x80, 0xBB); + expect_l1({{0x6000, 0x80, 0xAA}, {0x6100, 0x80, 0xBB}}); + + // Adjacent (touching but not overlapping) chunks stay separate. + mem_cache.Clear(); + add(0x7000, 0x80, 0xAA); + add(0x7080, 0x80, 0xBB); + expect_l1({{0x7000, 0x80, 0xAA}, {0x7080, 0x80, 0xBB}}); + + // Flush must erase every chunk intersecting the flush range, including a + // chunk that starts below the flushed address. Here 0x8140 lies only in the + // lower-starting, longer chunk; it must be dropped while the chunk that does + // not intersect survives untouched. + mem_cache.Clear(); + add(0x8000, 0x180, 0xAA); + add(0x8080, 0x40, 0xBB); + mem_cache.Flush(0x8140, 0x4); + expect_l1({{0x8080, 0x40, 0xBB}}); + + // A flush intersecting several partially overlapping chunks drops all of + // them, while a chunk it does not intersect is left in place. + mem_cache.Clear(); + add(0x9000, 0x80, 0xAA); + add(0x9040, 0x100, 0xBB); + add(0x9100, 0x80, 0xCC); + mem_cache.Flush(0x9060, 0x1); + expect_l1({{0x9100, 0x80, 0xCC}}); +} + 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
