diff --git a/lldb/include/lldb/Target/Memory.h b/lldb/include/lldb/Target/Memory.h index 2b8655e277a29..ad902ea584449 100644 --- a/lldb/include/lldb/Target/Memory.h +++ b/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> ReadRanges(llvm::ArrayRef> ranges, llvm::MutableArrayRef 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; }; diff --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py index afd0b12b20379..abe1da925cb40 100644 --- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py +++ b/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" diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp index 6e372fdf3fbee..2cb5a920f6c66 100644 --- a/lldb/source/Target/Memory.cpp +++ b/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> ranges, results.reserve(ranges.size()); llvm::SmallVector> 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> 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; } diff --git a/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py b/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py index a9fbe54e074ff..a72b04850d045 100644 --- a/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py +++ b/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) diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index f9d3dc1fee72f..8035b4b4d5c82 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/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(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 buffer(3 * 8, 0); + llvm::SmallVector> ranges = { + {full_line + 8, 8}, + {full_line + 16, 8}, + {full_line + l2_cache_size - 8, 8}}; + llvm::SmallVector> read_results = + process->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef 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 buffer(8, 0); + llvm::SmallVector> ranges = { + {full_line + l2_cache_size - 4, 8}}; + llvm::SmallVector> 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 buffer(3 * 8, 0); + llvm::SmallVector> ranges = { + {full_line + 8, 8}, {uncached_line, 8}, {full_line + 16, 8}}; + llvm::SmallVector> read_results = + process->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef 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 buffer(2 * 8, 0); + llvm::SmallVector> ranges = { + {short_line + bytes_available - 8, 8}, + {short_line + bytes_available - 4, 8}}; + llvm::SmallVector> 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) {