[Lldb-commits] [lldb] [lldb][minidump] Don't lose memory after an unreadable page when savi… (PR #212641)
satyanarayana reddy janga via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 28 16:46:25 PDT 2026
https://github.com/satyajanga updated https://github.com/llvm/llvm-project/pull/212641
>From 970dad3de14b5d50da319600c2b9ba5bd954e935 Mon Sep 17 00:00:00 2001
From: satya janga <satyajanga at fb.com>
Date: Tue, 28 Jul 2026 15:39:46 -0700
Subject: [PATCH] [lldb][minidump] Don't lose memory after an unreadable page
when saving a core
When save-core wrote a memory range that contained an unreadable (unmapped
or guard) page, MinidumpFileBuilder::ReadWriteMemoryInChunks stopped at the
first failed read and counted the bytes it had read-but-not-written into the
range's DataSize. Because the Memory64List stores every range's contents in a
single contiguous blob indexed by the cumulative DataSize of the preceding
ranges, over-reporting a range's size desynced the blob: every later range,
and the data read back from it, was shifted and corrupted. Ranges whose read
failed part way were also truncated, dropping readable memory that followed
the hole.
Rewrite ReadWriteMemoryInChunks to write exactly `size` bytes for each range:
append the bytes that were read, zero-fill the unreadable page, and continue
past it so readable memory that follows is still captured. This keeps each
range's DataSize equal to the bytes actually written and preserves the blob's
alignment.
Add an API test that custom-saves a file-backed mapping larger than its file
(a single read/write region whose first page is readable and whose tail is
past EOF and unreadable) and verifies the readable page survives byte-for-byte
and the Memory64List does not overflow the minidump.
---
.../Minidump/MinidumpFileBuilder.cpp | 105 ++++++++---------
.../Makefile | 3 +
.../TestProcessSaveCoreMinidumpPartialRead.py | 106 ++++++++++++++++++
.../main.cpp | 33 ++++++
4 files changed, 187 insertions(+), 60 deletions(-)
create mode 100644 lldb/test/API/functionalities/process_save_core_minidump_partial_read/Makefile
create mode 100644 lldb/test/API/functionalities/process_save_core_minidump_partial_read/TestProcessSaveCoreMinidumpPartialRead.py
create mode 100644 lldb/test/API/functionalities/process_save_core_minidump_partial_read/main.cpp
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
+}
More information about the lldb-commits
mailing list