[lldb] Guard MemoryCache::ReadRanges against a short buffer

lldb/source/Target/Memory.cpp

@@ -295,6 +295,17 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len,
 llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
 MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
                         llvm::MutableArrayRef<uint8_t> 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<uint8_t> empty;
+    return {ranges.size(), empty};
+  }
+
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
 
   llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> results;

lldb/source/Target/Process.cpp

@@ -2102,7 +2102,8 @@ Process::DoReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> 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<uint8_t> empty;
     return {ranges.size(), empty};

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<DummyProcess *>(process_sp.get());
+  TestMemoryCache cache(*process);
+  cache.AddL1CacheData(0x1000, std::make_shared<DataBufferHeap>(16, 0xAA));
+  ASSERT_EQ(cache.GetL1Cache().count(0x1000), 1u);
+
+  llvm::SmallVector<uint8_t, 0> short_buffer(8, 0);
+  llvm::SmallVector<Range<addr_t, size_t>> ranges = {{0x1000, 16}};
+  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> 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<uint8_t> 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<DummyReaderProcess>(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<uint8_t, 0> short_buffer(10, 0);
   llvm::SmallVector<Range<addr_t, size_t>> ranges = {{0x12345, 128},
                                                      {0x11, 128}};
   llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> 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());