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 @@ -125,16 +125,31 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len, Status &error) { + if (!dst || dst_len == 0) + return 0; + size_t bytes_left = dst_len; // Check the L1 cache for a range that contain the entire memory read. If we - // find a range in the L1 cache that does, we use it. Else we fall back to - // reading memory in m_L2_cache_line_byte_size byte sized chunks. The L1 - // cache contains chunks of memory that are not required to be + // find a range in the L1 cache that does, we use it. If not: + // - dst_len > L2 cache line size: Attempt to read the entire memory chunk + // and insert whatever we get back into the L1 cache. + // - dst_len <= L2 cache line size: Attempt to read it from the L2 cache, and + // if its not there, attempt to populate the cache. + // The L1 cache contains chunks of memory that are not required to be // m_L2_cache_line_byte_size bytes in size, so we don't try anything tricky // when reading from them (no partial reads from the L1 cache). std::lock_guard 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). + if (m_invalid_ranges.FindEntryThatContains(addr)) { + error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, addr); + return 0; + } + if (!m_L1_cache.empty()) { AddrRange read_range(addr, dst_len); BlockMap::iterator pos = m_L1_cache.upper_bound(addr); @@ -149,105 +164,105 @@ } } - // 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 - // separate reads than necessary, and with a large memory read request, it is - // unlikely that the caller function will ask for the next - // 4 bytes after the large memory read - so there's little benefit to saving - // it in the cache. - if (dst && dst_len > m_L2_cache_line_byte_size) { + if (dst_len > m_L2_cache_line_byte_size) { size_t bytes_read = m_process.ReadMemoryFromInferior(addr, dst, dst_len, error); - // Add this non block sized range to the L1 cache if we actually read - // anything if (bytes_read > 0) AddL1CacheData(addr, dst, bytes_read); return bytes_read; } - if (dst && bytes_left > 0) { - const uint32_t cache_line_byte_size = m_L2_cache_line_byte_size; - uint8_t *dst_buf = (uint8_t *)dst; - addr_t curr_addr = addr - (addr % cache_line_byte_size); - addr_t cache_offset = addr - curr_addr; - - while (bytes_left > 0) { - if (m_invalid_ranges.FindEntryThatContains(curr_addr)) { - error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, - curr_addr); - return dst_len - bytes_left; - } + const uint32_t cache_line_byte_size = m_L2_cache_line_byte_size; + uint8_t *dst_buf = (uint8_t *)dst; + addr_t cache_offset = addr % cache_line_byte_size; + addr_t curr_cache_line_base_addr = addr - cache_offset; + + while (bytes_left > 0) { + // FIXME: We should be able to wrap this into the check near the beginning + // of this function but we don't check for overlapping ranges so we will + // additionally check each cache line we read. + if (m_invalid_ranges.FindEntryThatContains(curr_cache_line_base_addr)) { + error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, + curr_cache_line_base_addr); + return dst_len - bytes_left; + } - BlockMap::const_iterator pos = m_L2_cache.find(curr_addr); - BlockMap::const_iterator end = m_L2_cache.end(); + BlockMap::const_iterator pos = m_L2_cache.find(curr_cache_line_base_addr); + BlockMap::const_iterator end = m_L2_cache.end(); - if (pos != end) { - size_t curr_read_size = cache_line_byte_size - cache_offset; - if (curr_read_size > bytes_left) - curr_read_size = bytes_left; + if (pos != end) { + const size_t cached_line_size = pos->second->GetByteSize(); + + // If we didn't fill out the cache line completely and the offset is + // bigger than what we have available, we can't do anything further + // here. + if (cache_offset >= cached_line_size) + return dst_len - bytes_left; - memcpy(dst_buf + dst_len - bytes_left, - pos->second->GetBytes() + cache_offset, curr_read_size); + size_t curr_read_size = cached_line_size - cache_offset; + if (curr_read_size > bytes_left) + curr_read_size = bytes_left; - bytes_left -= curr_read_size; - curr_addr += curr_read_size + cache_offset; - cache_offset = 0; + memcpy(dst_buf + dst_len - bytes_left, + pos->second->GetBytes() + cache_offset, curr_read_size); - if (bytes_left > 0) { - // Get sequential cache page hits - for (++pos; (pos != end) && (bytes_left > 0); ++pos) { - assert((curr_addr % cache_line_byte_size) == 0); + bytes_left -= curr_read_size; + curr_cache_line_base_addr += cache_line_byte_size; + cache_offset = 0; - if (pos->first != curr_addr) - break; + // If the cache line was not filled out entirely and we still have data + // to read, we can't do much else past this. + if (cached_line_size < cache_line_byte_size && bytes_left > 0) + return dst_len - bytes_left; - curr_read_size = pos->second->GetByteSize(); - if (curr_read_size > bytes_left) - curr_read_size = bytes_left; + // If we still have data to read, attempt to get a subsequent cache + // line hit. + if (bytes_left > 0) { + if (++pos != end) { + assert((curr_cache_line_base_addr % cache_line_byte_size) == 0); + if (pos->first == curr_cache_line_base_addr) { + curr_read_size = bytes_left; + if (curr_read_size > pos->second->GetByteSize()) + curr_read_size = pos->second->GetByteSize(); memcpy(dst_buf + dst_len - bytes_left, pos->second->GetBytes(), curr_read_size); - bytes_left -= curr_read_size; - curr_addr += curr_read_size; - // We have a cache page that succeeded to read some bytes but not - // an entire page. If this happens, we must cap off how much data - // we are able to read... - if (pos->second->GetByteSize() != cache_line_byte_size) - return dst_len - bytes_left; + // We only take this code path if the amount of data we want to + // read is less than or equal to the size of an L2 cache line. + // This means that we will read at most 2 cache lines, so after + // reading the second we can return how many bytes were read. + return dst_len - bytes_left; } } } + } + + // The cache is not populated so we need to read from the process + if (bytes_left > 0) { + assert((curr_cache_line_base_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_cache_line_base_addr, data_buffer_heap_up->GetBytes(), + data_buffer_heap_up->GetByteSize(), error); + + // If we failed to read a cache line, we can't do much else. + if (process_bytes_read == 0) + return dst_len - bytes_left; - // We need to read from the process + if (process_bytes_read < cache_line_byte_size) + data_buffer_heap_up->SetByteSize(process_bytes_read); - if (bytes_left > 0) { - 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) - return dst_len - bytes_left; - - if (process_bytes_read != cache_line_byte_size) { - data_buffer_heap_up->SetByteSize(process_bytes_read); - if (process_bytes_read < data_buffer_heap_up->GetByteSize()) { - dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read; - bytes_left = process_bytes_read; - } - } - m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release()); - // We have read data and put it into the cache, continue through the - // loop again to get the data out of the cache... - } + m_L2_cache[curr_cache_line_base_addr] = + DataBufferSP(data_buffer_heap_up.release()); + // We have read data and put it into the cache, continue through the + // loop again to get the data out of the cache... } } - return dst_len - bytes_left; + return dst_len; } AllocatedBlock::AllocatedBlock(lldb::addr_t addr, uint32_t byte_size, diff --git a/lldb/unittests/Target/CMakeLists.txt b/lldb/unittests/Target/CMakeLists.txt --- a/lldb/unittests/Target/CMakeLists.txt +++ b/lldb/unittests/Target/CMakeLists.txt @@ -3,6 +3,7 @@ DynamicRegisterInfoTest.cpp ExecutionContextTest.cpp MemoryRegionInfoTest.cpp + MemoryTest.cpp MemoryTagMapTest.cpp ModuleCacheTest.cpp PathMappingListTest.cpp @@ -15,6 +16,7 @@ lldbHost lldbPluginObjectFileELF lldbPluginPlatformLinux + lldbPluginPlatformMacOSX lldbPluginSymbolFileSymtab lldbTarget lldbSymbol diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp new file mode 100644 --- /dev/null +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -0,0 +1,228 @@ +//===-- MemoryTest.cpp ----------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/Memory.h" +#include "Plugins/Platform/MacOSX/PlatformMacOSX.h" +#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/Target.h" +#include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/DataBufferHeap.h" +#include "gtest/gtest.h" + +using namespace lldb_private; +using namespace lldb_private::repro; +using namespace lldb; + +namespace { +class MemoryTest : public ::testing::Test { +public: + void SetUp() override { + FileSystem::Initialize(); + HostInfo::Initialize(); + PlatformMacOSX::Initialize(); + } + void TearDown() override { + PlatformMacOSX::Terminate(); + HostInfo::Terminate(); + FileSystem::Terminate(); + } +}; + +class DummyProcess : public Process { +public: + DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp) + : Process(target_sp, listener_sp), m_bytes_left(0) {} + + // Required overrides + bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override { + return true; + } + Status DoDestroy() override { return {}; } + void RefreshStateAfterStop() override {} + size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, + Status &error) override { + if (m_bytes_left == 0) + return 0; + + size_t num_bytes_to_write = size; + if (m_bytes_left < size) { + num_bytes_to_write = m_bytes_left; + m_bytes_left = 0; + } else { + m_bytes_left -= size; + } + + memset(buf, 'B', num_bytes_to_write); + return num_bytes_to_write; + } + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override { + return false; + } + llvm::StringRef GetPluginName() override { return "Dummy"; } + + // Test-specific additions + size_t m_bytes_left; + MemoryCache &GetMemoryCache() { return m_memory_cache; } + void SetMaxReadSize(size_t size) { m_bytes_left = size; } +}; +} // namespace + +TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { + PlatformSP platform_sp; + TargetSP target_sp; + debugger_sp->GetTargetList().CreateTarget( + *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp); + return target_sp; +} + +TEST_F(MemoryTest, TesetMemoryCacheRead) { + 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); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + DummyProcess *process = static_cast(process_sp.get()); + MemoryCache &mem_cache = process->GetMemoryCache(); + const uint64_t l2_cache_size = process->GetMemoryCacheLineSize(); + Status error; + auto data_sp = std::make_shared(l2_cache_size * 2, '\0'); + size_t bytes_read = 0; + + // Cache empty, memory read fails, size > l2 cache size + process->SetMaxReadSize(0); + bytes_read = mem_cache.Read(0x1000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == 0); + + // Cache empty, memory read fails, size <= l2 cache size + data_sp->SetByteSize(l2_cache_size); + bytes_read = mem_cache.Read(0x1000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == 0); + + // Cache empty, memory read succeeds, size > l2 cache size + process->SetMaxReadSize(l2_cache_size * 4); + data_sp->SetByteSize(l2_cache_size * 2); + bytes_read = mem_cache.Read(0x1000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == data_sp->GetByteSize()); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size * 2); + + // Reading data previously cached (not in L2 cache). + data_sp->SetByteSize(l2_cache_size + 1); + bytes_read = mem_cache.Read(0x1000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == data_sp->GetByteSize()); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size * 2); // Verify we didn't + // read from the + // inferior. + + // Read from a different address, but make the size == l2 cache size. + // This should fill in a the L2 cache. + data_sp->SetByteSize(l2_cache_size); + bytes_read = mem_cache.Read(0x2000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == data_sp->GetByteSize()); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size); + + // Read from that L2 cache entry but read less than size of the cache line. + // Additionally, read from an offset. + data_sp->SetByteSize(l2_cache_size - 5); + bytes_read = mem_cache.Read(0x2001, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == data_sp->GetByteSize()); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size); // Verify we didn't read + // from the inferior. + + // What happens if we try to populate an L2 cache line but the read gives less + // than the size of a cache line? + process->SetMaxReadSize(l2_cache_size - 10); + data_sp->SetByteSize(l2_cache_size - 5); + bytes_read = mem_cache.Read(0x3000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == l2_cache_size - 10); + ASSERT_TRUE(process->m_bytes_left == 0); + + // What happens if we have a partial L2 cache line filled in and we try to + // read the part that isn't filled in? + data_sp->SetByteSize(10); + bytes_read = mem_cache.Read(0x3000 + l2_cache_size - 10, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == 0); // The last 10 bytes from this line are + // missing and we should be reading nothing + // here. + + // What happens when we try to straddle 2 cache lines? + process->SetMaxReadSize(l2_cache_size * 2); + data_sp->SetByteSize(l2_cache_size); + bytes_read = mem_cache.Read(0x4001, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == l2_cache_size); + ASSERT_TRUE(process->m_bytes_left == 0); + + // What happens when we try to straddle 2 cache lines where the first one is + // only partially filled? + process->SetMaxReadSize(l2_cache_size - 1); + data_sp->SetByteSize(l2_cache_size); + bytes_read = mem_cache.Read(0x5005, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == l2_cache_size - 6); // Ignoring the first 5 bytes, + // missing the last byte + ASSERT_TRUE(process->m_bytes_left == 0); + + // What happens if we add an invalid range and try to do a read larger than + // a cache line? + mem_cache.AddInvalidRange(0x6000, l2_cache_size * 2); + process->SetMaxReadSize(l2_cache_size * 2); + data_sp->SetByteSize(l2_cache_size * 2); + bytes_read = mem_cache.Read(0x6000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == 0); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size * 2); + + // What happens if we add an invalid range and try to do a read lt/eq a + // cache line? + mem_cache.AddInvalidRange(0x7000, l2_cache_size); + process->SetMaxReadSize(l2_cache_size); + data_sp->SetByteSize(l2_cache_size); + bytes_read = mem_cache.Read(0x7000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == 0); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size); + + // What happens if we remove the invalid range and read again? + mem_cache.RemoveInvalidRange(0x7000, l2_cache_size); + bytes_read = mem_cache.Read(0x7000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == l2_cache_size); + ASSERT_TRUE(process->m_bytes_left == 0); + + // What happens if we flush and read again? + process->SetMaxReadSize(l2_cache_size * 2); + mem_cache.Flush(0x7000, l2_cache_size); + bytes_read = mem_cache.Read(0x7000, data_sp->GetBytes(), + data_sp->GetByteSize(), error); + ASSERT_TRUE(bytes_read == l2_cache_size); + ASSERT_TRUE(process->m_bytes_left == l2_cache_size); // Verify that we re-read + // instead of using an + // old cache +}