[lldb] Serve MemoryCache::ReadRanges from the L2 cache as well as L1

lldb/include/lldb/Target/Memory.h

@@ -33,9 +33,9 @@ class MemoryCache {
 
   size_t Read(lldb::addr_t addr, void *dst, size_t dst_len, Status &error);
 
-  /// Reads multiple memory ranges, serving cache hits from L1 and batching all
-  /// misses through Process::DoReadMemoryRanges. The semantics of the return
-  /// value match Process::ReadMemoryRanges.
+  /// Reads multiple memory ranges, serving cache hits from L1 and L2 and
+  /// batching all misses through Process::DoReadMemoryRanges. The semantics of
+  /// the return value match Process::ReadMemoryRanges.
   llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
   ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
              llvm::MutableArrayRef<uint8_t> buffer);
@@ -82,6 +82,17 @@ class MemoryCache {
   // returns a pointer into that entry's data at the correct offset. Returns
   // nullptr on a miss. Caller must hold m_mutex.
   const uint8_t *FindL1CacheEntry(lldb::addr_t addr, size_t len) const;
+
+  // If the entire range [addr, addr+len) is covered by a single cache line
+  // that is already in L2, returns a pointer into that line's data at the
+  // correct offset. Never reads from the inferior. Returns nullptr on a miss.
+  // Caller must hold m_mutex.
+  const uint8_t *FindL2CacheEntry(lldb::addr_t addr, size_t len) const;
+
+  // Looks the range [addr, addr+len) up in L1 and then L2, returning a pointer
+  // into the data of whichever entry covers it. Returns nullptr on a miss.
+  // Caller must hold m_mutex.
+  const uint8_t *FindCacheEntry(lldb::addr_t addr, size_t len) const;
 };
 
 

lldb/packages/Python/lldbsuite/test/gdbclientutils.py

@@ -95,6 +95,33 @@ def parse_memory_read_packet(packet):
     return addr, length
 
 
+def parse_memory_read_ranges(packet: str) -> List[Tuple[int, int]]:
+    """
+    Parse every (addr, length) a memory-read packet asks the stub for, and return
+    an empty list if the packet isn't a memory read.  Unlike
+    parse_memory_read_packet this also covers "MultiMemRead", which carries
+    several ranges in one packet.
+    """
+    single = parse_memory_read_packet(packet)
+    if single is not None:
+        return [single]
+
+    prefix = "MultiMemRead:ranges:"
+    if not packet or not packet.startswith(prefix):
+        return []
+    body = packet[len(prefix) :]
+    end = body.find(";")
+    if end < 0:
+        return []
+    try:
+        numbers = [int(n, 16) for n in body[:end].split(",")]
+    except ValueError:
+        return []
+    if len(numbers) % 2:
+        return []
+    return list(zip(numbers[0::2], numbers[1::2]))
+
+
 class PacketDirection(Enum):
     RECV = "recv"
     SEND = "send"

lldb/source/Target/Memory.cpp

@@ -138,6 +138,27 @@ const uint8_t *MemoryCache::FindL1CacheEntry(lldb::addr_t addr,
   return pos->second->GetBytes() + (addr - chunk_range.GetRangeBase());
 }
 
+const uint8_t *MemoryCache::FindL2CacheEntry(lldb::addr_t addr,
+                                             size_t len) const {
+  if (m_L2_cache.empty())
+    return nullptr;
+  const lldb::addr_t line_offset = addr % m_L2_cache_line_byte_size;
+  BlockMap::const_iterator pos = m_L2_cache.find(addr - line_offset);
+  if (pos == m_L2_cache.end())
+    return nullptr;
+  if (line_offset + len > pos->second->GetByteSize())
+    return nullptr;
+  return pos->second->GetBytes() + line_offset;
+}
+
+const uint8_t *MemoryCache::FindCacheEntry(lldb::addr_t addr,
+                                           size_t len) const {
+  const uint8_t *cached = FindL1CacheEntry(addr, len);
+  if (!cached)
+    cached = FindL2CacheEntry(addr, len);
+  return cached;
+}
+
 lldb::DataBufferSP MemoryCache::GetL2CacheLine(lldb::addr_t line_base_addr,
                                                Status &error) {
   // This function assumes that the address given is aligned correctly.
@@ -280,7 +301,7 @@ MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
   results.reserve(ranges.size());
   llvm::SmallVector<Range<lldb::addr_t, size_t>> missed_ranges;
 
-  // Iterate once serving requests from L1.
+  // Iterate once serving requests from the caches.
   for (auto range : ranges) {
     const lldb::addr_t addr = range.GetRangeBase();
     const size_t len = range.GetByteSize();
@@ -290,10 +311,11 @@ MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
       continue;
     }
 
-    if (const uint8_t *l1_data = FindL1CacheEntry(addr, len)) {
+    const uint8_t *cached = FindCacheEntry(addr, len);
+    if (cached) {
       results.push_back(buffer.take_front(len));
       buffer = buffer.drop_front(len);
-      memcpy(results.back().data(), l1_data, len);
+      memcpy(results.back().data(), cached, len);
       continue;
     }
 

lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py

@@ -5,6 +5,11 @@
 
 import lldb
 from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import (
+    PacketDirection,
+    parse_memory_read_ranges,
+    parse_packet_log,
+)
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
@@ -67,8 +72,32 @@ def test_runtime_types_efficient_memreads(self):
         log_text = open(logfile).read()
         log_text = log_text.split("StartTesting", 1)[-1].split("EndTesting", 1)[0]
 
-        # This test is only checking that the packet it used at all (and that
-        # no errors are produced). It doesn't check that the packet is being
+        # The two assertions below only check that the packet is used at all (and
+        # that no errors are produced). They don't check that the packet is being
         # used to solve a problem in an optimal way.
         self.assertIn("MultiMemRead:", log_text)
         self.assertNotIn("MultiMemRead error", log_text)
+
+        # The memory cache serves a read out of what an earlier read fetched, so
+        # no range may be contained in one an earlier packet already read.
+        requested = []
+        for direction, body in parse_packet_log(log_text.splitlines()):
+            if direction != PacketDirection.SEND:
+                continue
+
+            # Resuming drops the whole cache, so nothing read before it is still
+            # held.
+            if body[0] in "cCsS" or body.startswith("vCont;"):
+                requested.clear()
+                continue
+
+            ranges = parse_memory_read_ranges(body)
+            for addr, length in ranges:
+                self.assertFalse(
+                    any(
+                        addr >= base and addr + length <= base + size
+                        for base, size in requested
+                    ),
+                    f"re-read {length} bytes at {addr:#x}",
+                )
+            requested.extend(ranges)

lldb/unittests/Target/MemoryTest.cpp

@@ -542,6 +542,117 @@ TEST_F(MemoryTest, TestReadMemoryRanges) {
   }
 }
 
+TEST_F(MemoryTest, TestReadMemoryRangesUsesL2Cache) {
+  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);
+
+  ProcessSP process_sp = CreateProcess(target_sp);
+  ASSERT_TRUE(process_sp);
+
+  DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
+  const uint64_t l2_cache_size = process->GetMemoryCacheLineSize();
+  Status error;
+  uint8_t header[8];
+
+  // Read the first 8 bytes of a cache line, the way a caller reads the header
+  // of an array before batching the elements that follow it. This fills the
+  // whole line and leaves the inferior unable to supply anything more.
+  const addr_t full_line = 0x1000;
+  ASSERT_EQ(full_line % l2_cache_size, 0u);
+  process->SetMaxReadSize(l2_cache_size);
+  process->SetFiller('A');
+  ASSERT_EQ(process->ReadMemory(full_line, header, sizeof(header), error),
+            sizeof(header));
+  ASSERT_EQ(process->m_bytes_left, 0u);
+
+  { // Ranges covered by that line are served from the cache. Leave the inferior
+    // able to answer, with a filler of its own, so a miss would show up both in
+    // the contents below and in the unspent budget.
+    process->SetMaxReadSize(l2_cache_size);
+    process->SetFiller('X');
+    llvm::SmallVector<uint8_t, 0> buffer(3 * 8, 0);
+    llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+        {full_line + 8, 8},
+        {full_line + 16, 8},
+        {full_line + l2_cache_size - 8, 8}};
+    llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+        process->ReadMemoryRanges(ranges, buffer);
+    ASSERT_EQ(read_results.size(), ranges.size());
+    for (llvm::MutableArrayRef<uint8_t> memory : read_results) {
+      ASSERT_EQ(memory.size(), 8u);
+      for (uint8_t byte : memory)
+        EXPECT_EQ(byte, 'A');
+    }
+    // Nothing was read from the inferior, so no packet was sent.
+    EXPECT_EQ(process->m_bytes_left, l2_cache_size);
+  }
+
+  { // A range crossing into the next, uncached line is a miss.
+    process->SetMaxReadSize(0);
+    llvm::SmallVector<uint8_t, 0> buffer(8, 0);
+    llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+        {full_line + l2_cache_size - 4, 8}};
+    llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+        process->ReadMemoryRanges(ranges, buffer);
+    ASSERT_EQ(read_results.size(), 1u);
+    EXPECT_EQ(read_results[0].size(), 0u);
+  }
+
+  { // A batch of hits and misses keeps the results in the requested order, and
+    // asks the inferior for the missed range only.
+    const addr_t uncached_line = 0x3000;
+    process->SetMaxReadSize(l2_cache_size);
+    process->SetFiller('C');
+    llvm::SmallVector<uint8_t, 0> buffer(3 * 8, 0);
+    llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+        {full_line + 8, 8}, {uncached_line, 8}, {full_line + 16, 8}};
+    llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+        process->ReadMemoryRanges(ranges, buffer);
+    ASSERT_EQ(read_results.size(), ranges.size());
+    for (llvm::MutableArrayRef<uint8_t> memory : read_results)
+      ASSERT_EQ(memory.size(), 8u);
+    for (uint8_t byte : read_results[0])
+      EXPECT_EQ(byte, 'A');
+    for (uint8_t byte : read_results[1])
+      EXPECT_EQ(byte, 'C');
+    for (uint8_t byte : read_results[2])
+      EXPECT_EQ(byte, 'A');
+    EXPECT_EQ(process->m_bytes_left, l2_cache_size - 8);
+  }
+
+  // A line the inferior could only partially supply is cached short.
+  const addr_t short_line = 0x2000;
+  ASSERT_EQ(short_line % l2_cache_size, 0u);
+  const size_t bytes_available = 64;
+  ASSERT_LT(bytes_available, l2_cache_size);
+  process->SetMaxReadSize(bytes_available);
+  process->SetFiller('D');
+  ASSERT_EQ(process->ReadMemory(short_line, header, sizeof(header), error),
+            sizeof(header));
+  ASSERT_EQ(process->m_bytes_left, 0u);
+
+  { // Only the part of the line that was actually read may be served.
+    llvm::SmallVector<uint8_t, 0> buffer(2 * 8, 0);
+    llvm::SmallVector<Range<addr_t, size_t>> ranges = {
+        {short_line + bytes_available - 8, 8},
+        {short_line + bytes_available - 4, 8}};
+    llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+        process->ReadMemoryRanges(ranges, buffer);
+    ASSERT_EQ(read_results.size(), ranges.size());
+    ASSERT_EQ(read_results[0].size(), 8u);
+    for (uint8_t byte : read_results[0])
+      EXPECT_EQ(byte, 'D');
+    EXPECT_EQ(read_results[1].size(), 0u);
+  }
+}
+
 using MemoryDeathTest = MemoryTest;
 
 TEST_F(MemoryDeathTest, TestReadMemoryRangesReturnsTooMuch) {