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 @@ -16,16 +16,52 @@ #include namespace lldb_private { + +// Abstraction of a reader from inferior's memory. +class MemoryFromInferiorReader { +public: + /// Read of memory from a process. + /// + /// This function has the same semantics of ReadMemory except that it + /// bypasses caching. + /// + /// \param[in] vm_addr + /// A virtual load address that indicates where to start reading + /// memory from. + /// + /// \param[out] buf + /// A byte buffer that is at least \a size bytes long that + /// will receive the memory bytes. + /// + /// \param[in] size + /// The number of bytes to read. + /// + /// \param[out] error + /// An error that indicates the success or failure of this + /// operation. If error indicates success (error.Success()), + /// then the value returned can be trusted, otherwise zero + /// will be returned. + /// + /// \return + /// The number of bytes that were actually read into \a buf. If + /// the returned number is greater than zero, yet less than \a + /// size, then this function will get called again with \a + /// vm_addr, \a buf, and \a size updated appropriately. Zero is + /// returned in the case of an error. + virtual size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, + size_t size, Status &error) = 0; +}; + // A class to track memory that was read from a live process between // runs. class MemoryCache { public: // Constructors and Destructors - MemoryCache(Process &process); + MemoryCache(MemoryFromInferiorReader &reader, uint64_t cache_line_size); ~MemoryCache(); - void Clear(bool clear_invalid_ranges = false); + void Clear(uint64_t cache_line_size, bool clear_invalid_ranges = false); void Flush(lldb::addr_t addr, size_t size); @@ -52,10 +88,10 @@ BlockMap m_L1_cache; // A first level memory cache whose chunk sizes vary that // will be used only if the memory read fits entirely in // a chunk - BlockMap m_L2_cache; // A memory cache of fixed size chinks + BlockMap m_L2_cache; // A memory cache of fixed size chunks // (m_L2_cache_line_byte_size bytes in size each) InvalidRanges m_invalid_ranges; - Process &m_process; + MemoryFromInferiorReader &m_reader; uint32_t m_L2_cache_line_byte_size; private: diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -356,7 +356,8 @@ public UserID, public Broadcaster, public ExecutionContextScope, - public PluginInterface { + public PluginInterface, + public MemoryFromInferiorReader { friend class FunctionCaller; // For WaitForStateChangeEventsPrivate friend class Debugger; // For PopProcessIOHandler and ProcessIOHandlerIsActive friend class DynamicLoader; // For LoadOperatingSystemPlugin @@ -1481,7 +1482,7 @@ /// vm_addr, \a buf, and \a size updated appropriately. Zero is /// returned in the case of an error. size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size, - Status &error); + Status &error) override; /// Read a NULL terminated string from memory /// 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 @@ -20,21 +20,21 @@ using namespace lldb_private; // MemoryCache constructor -MemoryCache::MemoryCache(Process &process) +MemoryCache::MemoryCache(MemoryFromInferiorReader &reader, + uint64_t cache_line_size) : m_mutex(), m_L1_cache(), m_L2_cache(), m_invalid_ranges(), - m_process(process), - m_L2_cache_line_byte_size(process.GetMemoryCacheLineSize()) {} + m_reader(reader), m_L2_cache_line_byte_size(cache_line_size) {} // Destructor MemoryCache::~MemoryCache() {} -void MemoryCache::Clear(bool clear_invalid_ranges) { +void MemoryCache::Clear(uint64_t cache_line_size, bool clear_invalid_ranges) { std::lock_guard guard(m_mutex); m_L1_cache.clear(); m_L2_cache.clear(); if (clear_invalid_ranges) m_invalid_ranges.Clear(); - m_L2_cache_line_byte_size = m_process.GetMemoryCacheLineSize(); + m_L2_cache_line_byte_size = cache_line_size; } void MemoryCache::AddL1CacheData(lldb::addr_t addr, const void *src, @@ -157,7 +157,7 @@ // it in the cache. if (dst && dst_len > m_L2_cache_line_byte_size) { size_t bytes_read = - m_process.ReadMemoryFromInferior(addr, dst, dst_len, error); + m_reader.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) @@ -220,24 +220,24 @@ } } - // We need to read from the process + // We need to read from the inferior 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( + size_t inferior_bytes_read = m_reader.ReadMemoryFromInferior( curr_addr, data_buffer_heap_up->GetBytes(), data_buffer_heap_up->GetByteSize(), error); - if (process_bytes_read == 0) + if (inferior_bytes_read == 0) return dst_len - bytes_left; - if (process_bytes_read != cache_line_byte_size) { - if (process_bytes_read < data_buffer_heap_up->GetByteSize()) { - dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read; - bytes_left = process_bytes_read; + if (inferior_bytes_read != cache_line_byte_size) { + if (inferior_bytes_read < data_buffer_heap_up->GetByteSize()) { + dst_len -= data_buffer_heap_up->GetByteSize() - inferior_bytes_read; + bytes_left = inferior_bytes_read; } - data_buffer_heap_up->SetByteSize(process_bytes_read); + data_buffer_heap_up->SetByteSize(inferior_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 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 @@ -486,9 +486,10 @@ m_stdio_communication("process.stdio"), m_stdio_communication_mutex(), m_stdin_forward(false), m_stdout_data(), m_stderr_data(), m_profile_data_comm_mutex(), m_profile_data(), m_iohandler_sync(0), - m_memory_cache(*this), m_allocated_memory_cache(*this), - m_should_detach(false), m_next_event_action_up(), m_public_run_lock(), - m_private_run_lock(), m_finalizing(false), m_finalize_called(false), + m_memory_cache(*this, GetMemoryCacheLineSize()), + m_allocated_memory_cache(*this), m_should_detach(false), + m_next_event_action_up(), m_public_run_lock(), m_private_run_lock(), + m_finalizing(false), m_finalize_called(false), m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false), m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false), m_can_interpret_function_calls(false), m_warnings_issued(), @@ -610,7 +611,7 @@ std::vector empty_notifications; m_notifications.swap(empty_notifications); m_image_tokens.clear(); - m_memory_cache.Clear(); + m_memory_cache.Clear(GetMemoryCacheLineSize()); m_allocated_memory_cache.Clear(); { std::lock_guard guard(m_language_runtimes_mutex); @@ -1493,7 +1494,7 @@ m_mod_id.BumpStopID(); if (!m_mod_id.IsLastResumeForUserExpression()) m_mod_id.SetStopEventForLastNaturalStopID(event_sp); - m_memory_cache.Clear(); + m_memory_cache.Clear(GetMemoryCacheLineSize()); LLDB_LOGF(log, "Process::SetPrivateState (%s) stop_id = %u", StateAsCString(new_state), m_mod_id.GetStopID()); } @@ -5611,7 +5612,7 @@ } m_instrumentation_runtimes.clear(); m_thread_list.DiscardThreadPlans(); - m_memory_cache.Clear(true); + m_memory_cache.Clear(GetMemoryCacheLineSize(), true); DoDidExec(); CompleteAttach(); // Flush the process (threads and all stack frames) after running 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 @@ -1,6 +1,7 @@ add_lldb_unittest(TargetTests ABITest.cpp ExecutionContextTest.cpp + MemoryCacheTest.cpp MemoryRegionInfoTest.cpp ModuleCacheTest.cpp PathMappingListTest.cpp diff --git a/lldb/unittests/Target/MemoryCacheTest.cpp b/lldb/unittests/Target/MemoryCacheTest.cpp new file mode 100644 --- /dev/null +++ b/lldb/unittests/Target/MemoryCacheTest.cpp @@ -0,0 +1,125 @@ +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "lldb/Target/Memory.h" +#include "lldb/Utility/Status.h" + +using namespace lldb_private; +using namespace lldb; + +namespace { + +class MemoryCacheTest : public MemoryFromInferiorReader, public testing::Test { + static const size_t k_memory_size = 256; + static const uint64_t k_cache_line_size = 16; + + size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size, + Status &error) override { + ++m_inferior_read_count; + + if (vm_addr >= m_memory.size()) + return 0; + + size = std::min(size, m_memory.size() - vm_addr); + memcpy(buf, m_memory.data() + vm_addr, size); + return size; + } + +public: + MemoryCacheTest() : m_cache(*this, k_cache_line_size) { + // Fill memory from [0x0 - 0x256) with byte values that match the index. We + // will use this memory in each test and each test will start with a fresh + // copy. + for (size_t i = 0; i < k_memory_size; ++i) + m_memory.push_back(static_cast(i)); + } + + void AddRangeToL1Cache(lldb::addr_t addr, size_t len) { + m_cache.AddL1CacheData(addr, m_memory.data() + addr, len); + } + + void IncrementAndFlushRange(lldb::addr_t addr, size_t len) { + for (size_t i = 0; i < len; ++i) + ++m_memory[addr + i]; + m_cache.Flush(addr, len); + } + + uint8_t ReadByte(lldb::addr_t addr) { + uint8_t result; + Status error; + m_cache.Read(addr, &result, 1, error); + EXPECT_TRUE(error.Success()); + return result; + } + +protected: + MemoryCache m_cache; + std::vector m_memory; + size_t m_inferior_read_count = 0; +}; + +TEST_F(MemoryCacheTest, L1FlushSimple) { + // Add a byte to the cache. + AddRangeToL1Cache(10, 1); + + // Sanity check - memory should agree with what ReadByte returns. + EXPECT_EQ(10, m_memory[10]); + EXPECT_EQ(m_memory[10], ReadByte(10)); + // Check that we have hit the cache and not the inferior's memory. + EXPECT_EQ(0u, m_inferior_read_count); + + IncrementAndFlushRange(10, 1); + + // Check the memory is incremented and the real memory agrees with ReadByte's + // result. + EXPECT_EQ(11, m_memory[11]); + EXPECT_EQ(m_memory[10], ReadByte(10)); +} + +// Let us do a parametrized test that caches two ranges. In the test, we then +// try to modify (and flush) various regions of memory and check that the cache +// still returns the correct values from reads. +using AddrRange = Range; +class MemoryCacheParametrizedTest + : public MemoryCacheTest, + public testing::WithParamInterface {}; + +TEST_P(MemoryCacheParametrizedTest, L1FlushWithTwoRegions) { + // Add two ranges to the cache. + std::vector cached = {{10, 10}, {30, 10}}; + for (AddrRange cached_range : cached) + AddRangeToL1Cache(cached_range.base, cached_range.size); + + // Flush the range given by the parameter. + AddrRange flush_range = GetParam(); + IncrementAndFlushRange(flush_range.base, flush_range.size); + + // Check that reads from the cached ranges read the inferior's memory + // if and only if the flush range intersects with one of the cached ranges. + bool has_intersection = false; + for (AddrRange cached_range : cached) { + if (flush_range.DoesIntersect(cached_range)) + has_intersection = true; + + for (size_t i = 0; i < cached_range.size; ++i) + ReadByte(cached_range.base + i); + } + EXPECT_EQ(has_intersection, m_inferior_read_count > 0); + + // Sanity check: check the memory contents. + for (size_t i = 0; i < 50; ++i) { + EXPECT_EQ(m_memory[i], ReadByte(i)) << " (element at addr=" << i << ")"; + } +} + +INSTANTIATE_TEST_CASE_P( + AllMemoryCacheParametrizedTest, MemoryCacheParametrizedTest, + ::testing::Values(AddrRange(5, 6), AddrRange(5, 10), AddrRange(5, 20), + AddrRange(5, 30), AddrRange(5, 40), AddrRange(10, 1), + AddrRange(10, 21), AddrRange(19, 1), AddrRange(19, 11), + AddrRange(19, 12), AddrRange(30, 1), AddrRange(39, 1), + AddrRange(39, 5), AddrRange(5, 1), AddrRange(5, 5), + AddrRange(9, 1), AddrRange(20, 1), AddrRange(20, 10), + AddrRange(29, 1), AddrRange(40, 1), AddrRange(40, 10))); + +} // namespace