[Lldb-commits] [lldb] [lldb][minidump] Don't lose memory after an unreadable page when savi… (PR #212641)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 28 17:07:02 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: satyanarayana reddy janga (satyajanga)
<details>
<summary>Changes</summary>
**Issue**
An internal failing test found a latent bug in lldb's save-core (minidump writer). When it saved a memory range that had an unreadable page in it, it:
- stopped at that page and threw away the readable memory after it, and
- still recorded the full size for that range, which shifted and corrupted every following range in the dump.
Result: coredumps were missing/garbled memory — e.g. folly's coroutine cookie read back as 0, so co_print_suspended found "no suspended frames."
**Fix**
Rewrote ReadWriteMemoryInChunks in MinidumpFileBuilder.cpp to: save the readable bytes, zero-fill the unreadable page, keep going, and record the exact number of bytes actually written. That keeps the dump aligned and preserves memory after a hole. One file, plus a #include <cstring>.
**Test**
Added an lldb API test that builds a memory region with a readable page followed by an unreadable tail, saves a core of it, and checks the readable data comes back intact and the dump isn't corrupted. Verified it fails without the fix and passes with it.
---
Full diff: https://github.com/llvm/llvm-project/pull/212641.diff
4 Files Affected:
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp (+45-60)
- (added) lldb/test/API/functionalities/process_save_core_minidump_partial_read/Makefile (+3)
- (added) lldb/test/API/functionalities/process_save_core_minidump_partial_read/TestProcessSaveCoreMinidumpPartialRead.py (+106)
- (added) lldb/test/API/functionalities/process_save_core_minidump_partial_read/main.cpp (+33)
``````````diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 876eea28a8923..ef1a3713175fc 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -8,6 +8,8 @@
#include "MinidumpFileBuilder.h"
+#include <cstring>
+
#include "Plugins/Process/minidump/RegisterContextMinidump_ARM64.h"
#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
@@ -968,72 +970,55 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
const lldb::addr_t addr = range.range.start();
const lldb::addr_t size = range.range.size();
Log *log = GetLog(LLDBLog::Object);
- uint64_t total_bytes_read = 0;
+ void *buf = data_buffer.GetBytes();
+ const lldb::addr_t chunk_size = data_buffer.GetByteSize();
+ // The smallest page size on any supported target. Used to step over an
+ // unreadable page without zero-filling memory that may be readable.
+ const lldb::addr_t page_size = 4096;
+
+ // We must write exactly `size` bytes for this range. The Memory64List stores
+ // every range's contents in one contiguous blob indexed by the cumulative
+ // DataSize of the preceding ranges, so writing fewer bytes than DataSize
+ // corrupts every following range. On a failed read we zero-fill the
+ // unreadable page and keep going rather than truncating the range.
+ bytes_read = 0;
Status addDataError;
- Process::ReadMemoryChunkCallback callback =
- [&](Status &error, lldb::addr_t current_addr, const void *buf,
- uint64_t bytes_read) -> lldb_private::IterationAction {
- if (error.Fail() || bytes_read == 0) {
- LLDB_LOGF(log,
- "Failed to read memory region at: 0x%" PRIx64
- ". Bytes read: 0x%" PRIx64 ", error: %s",
- current_addr, bytes_read, error.AsCString());
-
- // If we failed in a memory read, we would normally want to skip
- // this entire region. If we had already written to the minidump
- // file, we can't easily rewind that state.
- //
- // So if we do encounter an error while reading, we return
- // immediately, any prior bytes read will still be included but
- // any bytes partially read before the error are ignored.
- return lldb_private::IterationAction::Stop;
- }
-
- if (current_addr != addr + total_bytes_read) {
- LLDB_LOGF(log,
- "Current addr is at unexpected address, 0x%" PRIx64
- ", expected at 0x%" PRIx64,
- current_addr, addr + total_bytes_read);
-
- // Something went wrong and the address is not where it should be
- // we'll error out of this Minidump generation.
- addDataError = Status::FromErrorStringWithFormat(
- "Unexpected address encounterd when reading memory in chunks "
- "0x%" PRIx64 " expected 0x%" PRIx64,
- current_addr, addr + total_bytes_read);
- return lldb_private::IterationAction::Stop;
+ while (bytes_read < size) {
+ const lldb::addr_t current_addr = addr + bytes_read;
+ const lldb::addr_t bytes_remaining = size - bytes_read;
+ const lldb::addr_t bytes_to_read = std::min(bytes_remaining, chunk_size);
+ Status error;
+ const lldb::addr_t bytes_read_for_chunk =
+ m_process_sp->ReadMemoryFromInferior(current_addr, buf, bytes_to_read,
+ error);
+
+ if (bytes_read_for_chunk > 0) {
+ addDataError = AddData(buf, bytes_read_for_chunk);
+ if (addDataError.Fail())
+ return addDataError;
+ bytes_read += bytes_read_for_chunk;
}
- // Write to the minidump file with the chunk potentially flushing to
- // disk.
- // This error will be captured by the outer scope and is considered fatal.
- // If we get an error writing to disk we can't easily guarauntee that we
- // won't corrupt the minidump.
- addDataError = AddData(buf, bytes_read);
- if (addDataError.Fail())
- return lldb_private::IterationAction::Stop;
-
- total_bytes_read += bytes_read;
- // If we have a partial read, report it, but only if the partial read
- // didn't finish reading the entire region.
- if (bytes_read != data_buffer.GetByteSize() && total_bytes_read != size) {
+ // The read stopped short at an unreadable page. Zero-fill up to the next
+ // page boundary and continue past it, so readable memory that follows is
+ // still captured.
+ if (bytes_read_for_chunk < bytes_to_read) {
+ const lldb::addr_t hole = addr + bytes_read;
+ lldb::addr_t fill = ((hole + page_size) & ~(page_size - 1)) - hole;
+ fill = std::min(fill, size - bytes_read);
+ fill = std::min(fill, chunk_size);
LLDB_LOGF(log,
- "Memory region at: 0x%" PRIx64 " partial read 0x%" PRIx64
- " bytes out of 0x%" PRIx64 " bytes.",
- current_addr, bytes_read,
- data_buffer.GetByteSize() - bytes_read);
-
- // If we've read some bytes, we stop trying to read more and return
- // this best effort attempt
- return lldb_private::IterationAction::Stop;
+ "Failed to read memory region at: 0x%" PRIx64
+ ". Zero-filling 0x%" PRIx64 " bytes, error: %s",
+ hole, fill, error.AsCString());
+ ::memset(buf, 0, fill);
+ addDataError = AddData(buf, fill);
+ if (addDataError.Fail())
+ return addDataError;
+ bytes_read += fill;
}
+ }
- // No problems, keep going!
- return lldb_private::IterationAction::Continue;
- };
-
- bytes_read = m_process_sp->ReadMemoryInChunks(
- addr, data_buffer.GetBytes(), data_buffer.GetByteSize(), size, callback);
return addDataError;
}
diff --git a/lldb/test/API/functionalities/process_save_core_minidump_partial_read/Makefile b/lldb/test/API/functionalities/process_save_core_minidump_partial_read/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/functionalities/process_save_core_minidump_partial_read/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/process_save_core_minidump_partial_read/TestProcessSaveCoreMinidumpPartialRead.py b/lldb/test/API/functionalities/process_save_core_minidump_partial_read/TestProcessSaveCoreMinidumpPartialRead.py
new file mode 100644
index 0000000000000..6ab140e5e4e8c
--- /dev/null
+++ b/lldb/test/API/functionalities/process_save_core_minidump_partial_read/TestProcessSaveCoreMinidumpPartialRead.py
@@ -0,0 +1,106 @@
+"""
+Regression test for saving a minidump when a saved memory range contains an
+unreadable page.
+
+The minidump Memory64List stores every range's bytes in one contiguous blob
+indexed by the cumulative DataSize of the preceding ranges. If the writer counts
+bytes it failed to read (and therefore never wrote) into a range's DataSize, the
+blob desyncs and later ranges -- and the data read back from them -- are
+corrupted. See MinidumpFileBuilder::ReadWriteMemoryInChunks.
+"""
+
+import os
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpPartialReadTestCase(TestBase):
+ def _assert_memory64_within_file(self, path):
+ """The Memory64List data must not claim more bytes than the file holds."""
+ with open(path, "rb") as f:
+ data = f.read()
+ self.assertEqual(data[:4], b"MDMP")
+ num_streams, dir_rva = struct.unpack_from("<II", data, 8)
+ found = False
+ for i in range(num_streams):
+ stream_type, _, rva = struct.unpack_from("<III", data, dir_rva + i * 12)
+ if stream_type == 9: # Memory64ListStream
+ found = True
+ num_ranges, base_rva = struct.unpack_from("<QQ", data, rva)
+ total = sum(
+ struct.unpack_from("<QQ", data, rva + 16 + j * 16)[1]
+ for j in range(num_ranges)
+ )
+ self.assertLessEqual(
+ base_rva + total,
+ len(data),
+ "Memory64List DataSize overflows the minidump file",
+ )
+ self.assertTrue(found, "minidump has no Memory64List stream")
+
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_save_core_range_with_unreadable_tail(self):
+ """A saved range whose tail is unreadable must keep its readable bytes
+ intact and must not corrupt the minidump's memory blob."""
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ target = self.dbg.CreateTarget(exe)
+ lldbutil.run_break_set_by_source_regexp(self, "Set a breakpoint here")
+ process = target.LaunchSimple(None, None, self.get_process_working_directory())
+ self.assertState(process.GetState(), lldb.eStateStopped)
+
+ frame = process.GetSelectedThread().GetFrameAtIndex(0)
+ region_addr = frame.FindVariable("region").GetValueAsUnsigned()
+ page = frame.FindVariable("page").GetValueAsUnsigned()
+ self.assertNotEqual(region_addr, 0)
+ self.assertNotEqual(page, 0)
+
+ # Sanity: the live process reads the backed first page (the sentinel),
+ # while reading into the unbacked tail fails.
+ live_error = lldb.SBError()
+ live_page = process.ReadMemory(region_addr, page, live_error)
+ self.assertSuccess(live_error)
+ self.assertEqual(live_page, b"\xab" * page)
+ tail_error = lldb.SBError()
+ process.ReadMemory(region_addr + page, page, tail_error)
+ self.assertTrue(tail_error.Fail())
+
+ core_path = self.getBuildArtifact("partial_read.dmp")
+ options = lldb.SBSaveCoreOptions()
+ options.SetOutputFile(lldb.SBFileSpec(core_path))
+ options.SetPluginName("minidump")
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
+ # Force the whole four-page mapping (readable first page + unreadable
+ # tail) to be saved as a single range.
+ rw = 0b110 # ePermissionsReadable | ePermissionsWritable
+ options.AddMemoryRegionToSave(
+ lldb.SBMemoryRegionInfo("", region_addr, region_addr + 4 * page, rw, True)
+ )
+ self.assertSuccess(process.SaveCore(options))
+
+ try:
+ core_target = self.dbg.CreateTarget(None)
+ core_process = core_target.LoadCore(core_path)
+ self.assertTrue(core_process.IsValid())
+
+ # The readable page that precedes the unreadable tail must survive
+ # byte-for-byte. Before the fix this read back as garbage/zero
+ # because the range's data was dropped and the blob desynced.
+ core_error = lldb.SBError()
+ core_page = core_process.ReadMemory(region_addr, page, core_error)
+ self.assertSuccess(core_error)
+ self.assertEqual(
+ core_page,
+ b"\xab" * page,
+ "readable page before the unreadable tail was lost or misaligned",
+ )
+
+ self._assert_memory64_within_file(core_path)
+ finally:
+ if os.path.isfile(core_path):
+ os.unlink(core_path)
diff --git a/lldb/test/API/functionalities/process_save_core_minidump_partial_read/main.cpp b/lldb/test/API/functionalities/process_save_core_minidump_partial_read/main.cpp
new file mode 100644
index 0000000000000..8e229190a1678
--- /dev/null
+++ b/lldb/test/API/functionalities/process_save_core_minidump_partial_read/main.cpp
@@ -0,0 +1,33 @@
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+// Create a single read/write memory region whose tail is unreadable: a
+// file-backed mapping that is larger than the file. The first page is backed by
+// the file (readable) and holds a sentinel; the pages that follow are past EOF
+// and fail to read via process_vm_readv. This is the shape that used to make
+// the minidump save-core writer truncate a range and desync the Memory64 data
+// blob (see MinidumpFileBuilder::ReadWriteMemoryInChunks).
+int main() {
+ const size_t page = sysconf(_SC_PAGESIZE);
+ char path[] = "/tmp/lldb_savecore_holeXXXXXX";
+ int fd = mkstemp(path);
+ if (fd < 0)
+ return 1;
+ unlink(path);
+ if (ftruncate(fd, page) != 0) // the file is exactly one page
+ return 1;
+ const size_t map_size = 4 * page; // map four pages; pages 1..3 are past EOF
+ uint8_t *region = static_cast<uint8_t *>(
+ mmap(nullptr, map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0));
+ if (region == MAP_FAILED)
+ return 1;
+ memset(region, 0xAB, page); // sentinel in the backed (readable) first page
+ printf("region = %p, page = %zu\n", (void *)region, page);
+ fflush(stdout);
+ return 0; // Set a breakpoint here
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/212641
More information about the lldb-commits
mailing list