diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp index 2cb5a920f6c66..223a21b802d94 100644 --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -295,6 +295,17 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len, llvm::SmallVector> MemoryCache::ReadRanges(llvm::ArrayRef> ranges, llvm::MutableArrayRef buffer) { + // A cache hit writes into `buffer` below, so check its size before that + // write. Fail the same way Process::DoReadMemoryRanges does. + auto total_ranges_len = llvm::sum_of( + llvm::map_range(ranges, [](auto range) { return range.size; })); + assert(buffer.size() >= total_ranges_len && + "MemoryCache::ReadRanges: provided buffer is too short"); + if (buffer.size() < total_ranges_len) { + llvm::MutableArrayRef empty; + return {ranges.size(), empty}; + } + std::lock_guard guard(m_mutex); llvm::SmallVector> results; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 1d7e413603c82..fca16b955021b 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2102,7 +2102,8 @@ Process::DoReadMemoryRanges(llvm::ArrayRef> ranges, // If the buffer is not large enough, this is a programmer error. // In production builds, gracefully fail by returning a length of 0 for all // ranges. - assert(buffer.size() >= total_ranges_len && "provided buffer is too short"); + assert(buffer.size() >= total_ranges_len && + "Process::DoReadMemoryRanges: provided buffer is too short"); if (buffer.size() < total_ranges_len) { llvm::MutableArrayRef empty; return {ranges.size(), empty}; diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index 464ae033eacee..4501de43b2d1a 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -691,6 +691,37 @@ TEST_F(MemoryDeathTest, TestReadMemoryRangesReturnsTooMuch) { #endif } +TEST_F(MemoryDeathTest, TestReadRangesWithShortBufferAndCacheHit) { + GTEST_FLAG_SET(death_test_style, "threadsafe"); + + ArchSpec arch("arm64-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()); + TestMemoryCache cache(*process); + cache.AddL1CacheData(0x1000, std::make_shared(16, 0xAA)); + ASSERT_EQ(cache.GetL1Cache().count(0x1000), 1u); + + llvm::SmallVector short_buffer(8, 0); + llvm::SmallVector> ranges = {{0x1000, 16}}; + llvm::SmallVector> read_results; + ASSERT_DEBUG_DEATH( + { read_results = cache.ReadRanges(ranges, short_buffer); }, + "MemoryCache::ReadRanges: provided buffer is too short"); +#ifdef NDEBUG + // With asserts off, the ranges come back empty instead. + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef result : read_results) + ASSERT_TRUE(result.empty()); +#endif +} + TEST_F(MemoryDeathTest, TestReadMemoryRangesWithShortBuffer) { // gtest death-tests execute in a sub-process (fork), which invalidates // any signpost handles and would cause spurious crashes if used. Use the @@ -710,13 +741,19 @@ TEST_F(MemoryDeathTest, TestReadMemoryRangesWithShortBuffer) { std::make_shared(target_sp, listener_sp); ASSERT_TRUE(process_sp); + // Memory cache has to be off to reach the one in Process::DoReadMemoryRanges. + Status set_error = process_sp->SetPropertyValue( + nullptr, eVarSetOperationAssign, "disable-memory-cache", "true"); + ASSERT_TRUE(set_error.Success()) << set_error.AsCString(); + ASSERT_TRUE(process_sp->GetDisableMemoryCache()); + llvm::SmallVector short_buffer(10, 0); llvm::SmallVector> ranges = {{0x12345, 128}, {0x11, 128}}; llvm::SmallVector> read_results; ASSERT_DEBUG_DEATH( { read_results = process_sp->ReadMemoryRanges(ranges, short_buffer); }, - "provided buffer is too short"); + "Process::DoReadMemoryRanges: provided buffer is too short"); #ifdef NDEBUG // With asserts off, the read should return empty ranges. ASSERT_EQ(read_results.size(), ranges.size());