diff --git a/lldb/include/lldb/Target/Memory.h b/lldb/include/lldb/Target/Memory.h --- a/lldb/include/lldb/Target/Memory.h +++ b/lldb/include/lldb/Target/Memory.h @@ -15,6 +15,8 @@ #include #include +#include "llvm/ADT/SmallSet.h" + namespace lldb_private { // A class to track memory that was read from a live process between // runs. @@ -43,6 +45,15 @@ void AddL1CacheData(lldb::addr_t addr, const lldb::DataBufferSP &data_buffer_sp); + /// Remember that a memory address cannot be read from. + void AddUnreadableAddress(lldb::addr_t address); + + /// Test if a memory address was already recorded as unreadable. + bool IsAddressUnreadable(lldb::addr_t address); + + /// Clear unreadable address cache. + void FlushUnreadableAddresses(); + protected: typedef std::map BlockMap; typedef RangeVector InvalidRanges; @@ -57,6 +68,7 @@ InvalidRanges m_invalid_ranges; Process &m_process; uint32_t m_L2_cache_line_byte_size; + llvm::SmallSet m_unreadable_addresses; private: MemoryCache(const MemoryCache &) = delete; diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -24,7 +24,8 @@ MemoryCache::MemoryCache(Process &process) : m_mutex(), m_L1_cache(), m_L2_cache(), m_invalid_ranges(), m_process(process), - m_L2_cache_line_byte_size(process.GetMemoryCacheLineSize()) {} + m_L2_cache_line_byte_size(process.GetMemoryCacheLineSize()), + m_unreadable_addresses() {} // Destructor MemoryCache::~MemoryCache() = default; @@ -36,6 +37,7 @@ if (clear_invalid_ranges) m_invalid_ranges.Clear(); m_L2_cache_line_byte_size = m_process.GetMemoryCacheLineSize(); + FlushUnreadableAddresses(); } void MemoryCache::AddL1CacheData(lldb::addr_t addr, const void *src, @@ -51,6 +53,8 @@ } void MemoryCache::Flush(addr_t addr, size_t size) { + FlushUnreadableAddresses(); + if (size == 0) return; @@ -149,6 +153,11 @@ } } + if (IsAddressUnreadable(addr)) { + error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, addr); + return 0; + } + // If this memory read request is larger than the cache line size, then we // (1) try to read as much of it at once as possible, and (2) don't add the // data to the memory cache. We don't want to split a big read up into more @@ -163,6 +172,8 @@ // anything if (bytes_read > 0) AddL1CacheData(addr, dst, bytes_read); + else + AddUnreadableAddress(addr); return bytes_read; } @@ -224,14 +235,21 @@ // We need to read from the process if (bytes_left > 0) { + if (IsAddressUnreadable(curr_addr)) { + error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, + addr); + return dst_len - bytes_left; + } assert((curr_addr % cache_line_byte_size) == 0); std::unique_ptr data_buffer_heap_up( new DataBufferHeap(cache_line_byte_size, 0)); size_t process_bytes_read = m_process.ReadMemoryFromInferior( curr_addr, data_buffer_heap_up->GetBytes(), data_buffer_heap_up->GetByteSize(), error); - if (process_bytes_read == 0) + if (process_bytes_read == 0) { + AddUnreadableAddress(curr_addr); return dst_len - bytes_left; + } if (process_bytes_read != cache_line_byte_size) { if (process_bytes_read < data_buffer_heap_up->GetByteSize()) { @@ -250,6 +268,16 @@ return dst_len - bytes_left; } +void MemoryCache::AddUnreadableAddress(addr_t address) { + m_unreadable_addresses.insert(address); +} + +bool MemoryCache::IsAddressUnreadable(addr_t address) { + return m_unreadable_addresses.count(address) > 0; +} + +void MemoryCache::FlushUnreadableAddresses() { m_unreadable_addresses.clear(); } + AllocatedBlock::AllocatedBlock(lldb::addr_t addr, uint32_t byte_size, uint32_t permissions, uint32_t chunk_size) : m_range(addr, byte_size), m_permissions(permissions), diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2266,6 +2266,11 @@ error.SetErrorToGenericError(); return LLDB_INVALID_ADDRESS; } + if (!GetDisableMemoryCache()) { + // Flush unreadable addresses list; allocating a + // block of memory will change what addresse can be accessed. + m_memory_cache.FlushUnreadableAddresses(); + } #if defined(USE_ALLOCATE_MEMORY_CACHE) return m_allocated_memory_cache.AllocateMemory(size, permissions, error); diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestMemoryReadFailCache.py b/lldb/test/API/functionalities/gdb_remote_client/TestMemoryReadFailCache.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestMemoryReadFailCache.py @@ -0,0 +1,55 @@ +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from lldbsuite.test.gdbclientutils import * +from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase + + +class TestMemoryReadFailCache(GDBRemoteTestBase): + + @skipIfXmlSupportMissing + def test(self): + class MyResponder(MockGDBServerResponder): + first_read = True + + def qHostInfo (self): + return "cputype:16777228;cpusubtype:2;addressing_bits:47;ostype:macosx;vendor:apple;os_version:12.6.0;endian:little;ptrsize:8;" + + def readMemory(self, addr, length): + if self.first_read: + self.first_read = False + return "E05" + return "AA" * length + + self.server.responder = MyResponder() + target = self.dbg.CreateTarget('') + if self.TraceOn(): + self.runCmd("log enable gdb-remote packets") + self.addTearDownHook( + lambda: self.runCmd("log disable gdb-remote packets")) + process = self.connect(target) + + err = lldb.SBError() + uint = process.ReadUnsignedFromMemory(0x1000, 4, err) + self.assertTrue (err.Fail(), + "lldb should fail to read memory at 0x100 the first time, " + "after checking with stub") + + # Now the remote stub will return a non-zero number if + # we ask again, unlike a real stub. + # Confirm that reading from that address still fails - + # that we cached the unreadable address + uint = process.ReadUnsignedFromMemory(0x1000, 4, err) + self.assertTrue (err.Fail(), + "lldb should fail to read memory at 0x100 the second time, " + "because we did not read from the remote stub.") + + # Allocate memory in the inferior, which will flush + # the unreadable address cache. + process.AllocateMemory(0x100, 3, err) + + uint = process.ReadUnsignedFromMemory(0x1000, 4, err) + self.assertTrue (err.Success(), + "lldb should succeed to read memory at 0x100 the third time, " + "because we flushed the failed address cache, and the stub is " + "now providing data there.")