https://github.com/satyajanga updated https://github.com/llvm/llvm-project/pull/212641
>From 31d5c9ad3984f84e250f9d038295d73e6a5586bb Mon Sep 17 00:00:00 2001 From: satya janga <[email protected]> Date: Tue, 28 Jul 2026 15:39:46 -0700 Subject: [PATCH] [lldb][minidump] Keep saving a memory range past an unreadable page MinidumpFileBuilder::ReadWriteMemoryInChunks stops a range at the first chunk that fails to read. An inaccessible page part way through a range therefore truncates it, and every readable page after the hole is dropped from the minidump even though the inferior can still read it. A range can contain such a page while looking entirely readable. Linux 6.13 guard regions (MADV_GUARD_INSTALL) make a page inaccessible without splitting the VMA, so /proc/pid/maps reports the whole mapping as rw- while reads of the guarded page fail. save-core also coalesces adjacent regions that share permissions into a single range, so the hole can sit far from the range's end. Replace the callback with a loop that writes exactly `size` bytes for the range: append the bytes that were read, zero-fill the page that could not be read, and continue past it so the readable memory beyond the hole is still captured. Writing the full range also keeps the range's DataSize equal to the bytes written, which the Memory64List relies on to locate each range in the shared data blob. Add an API test that saves a mapping whose tail is unreadable and checks that the readable page in front of the hole survives byte for byte and that the Memory64List does not overrun the minidump. --- .../Minidump/MinidumpFileBuilder.cpp | 108 +++++++----------- .../partial_read/Makefile | 3 + .../TestProcessSaveCoreMinidumpPartialRead.py | 101 ++++++++++++++++ .../partial_read/main.cpp | 26 +++++ 4 files changed, 170 insertions(+), 68 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 80a183785cb11..80f62d8d4e568 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,80 +970,50 @@ 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(); + // Step over an unreadable page at a time, so we never zero-fill memory that + // could be read. + const lldb::addr_t page_size = 4096; + + // Write exactly `size` bytes: Memory64List locates each range by the + // cumulative DataSize of the ranges before it. + 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) { + // Unreadable page: zero-fill it and continue past the hole. + 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; - }; - - // ReadMemoryInChunks returns the number of bytes it read from the inferior, - // which can exceed the number we actually appended: when a chunk read fails - // the callback stops without writing the bytes it had partially read. Report - // the bytes we wrote (total_bytes_read) so the range's DataSize matches the - // data in the blob. Otherwise the Memory64List, which locates each range by - // the cumulative DataSize of the preceding ranges, desyncs and every later - // range reads back corrupted. - m_process_sp->ReadMemoryInChunks(addr, data_buffer.GetBytes(), - data_buffer.GetByteSize(), size, callback); - bytes_read = total_bytes_read; 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..02c7b06c15dbc --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump/partial_read/TestProcessSaveCoreMinidumpPartialRead.py @@ -0,0 +1,101 @@ +""" +Test saving a minidump when a saved memory range contains an unreadable page: +the readable memory after the hole must still be captured. +""" + +import os +import struct + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +MEMORY64_LIST_STREAM = 9 # llvm::minidump::StreamType::Memory64List + + +class ProcessSaveCoreMinidumpPartialReadTestCase(TestBase): + def assert_memory64_datasize_within_file(self, core_path): + """The Memory64List must not claim more bytes than the file holds.""" + with open(core_path, "rb") as core_file: + core_bytes = core_file.read() + self.assertEqual(core_bytes[:4], b"MDMP") + num_streams, directory_rva = struct.unpack_from("<II", core_bytes, 8) + + memory64_list_found = False + for stream_index in range(num_streams): + stream_type, _, stream_rva = struct.unpack_from( + "<III", core_bytes, directory_rva + stream_index * 12 + ) + if stream_type != MEMORY64_LIST_STREAM: + continue + memory64_list_found = True + num_ranges, base_rva = struct.unpack_from("<QQ", core_bytes, stream_rva) + total_data_size = sum( + struct.unpack_from("<QQ", core_bytes, stream_rva + 16 + i * 16)[1] + for i in range(num_ranges) + ) + self.assertLessEqual( + base_rva + total_data_size, + len(core_bytes), + "Memory64List DataSize claims more bytes than the minidump holds", + ) + self.assertTrue(memory64_list_found, "minidump has no Memory64List stream") + + @skipUnlessPlatform(["linux"]) + def test_save_core_range_with_unreadable_tail(self): + 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 = frame.FindVariable("region").GetValueAsUnsigned() + page = frame.FindVariable("page").GetValueAsUnsigned() + self.assertNotEqual(region, 0) + self.assertNotEqual(page, 0) + + # The first page reads, the tail past the file's end does not. + live_error = lldb.SBError() + live_page = process.ReadMemory(region, page, live_error) + self.assertSuccess(live_error) + self.assertEqual(live_page, b"\xab" * page) + tail_error = lldb.SBError() + process.ReadMemory(region + 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) + rw = 0b110 # ePermissionsReadable | ePermissionsWritable + options.AddMemoryRegionToSave( + lldb.SBMemoryRegionInfo("", region, region + 4 * page, rw, True) + ) + self.assertSuccess(process.SaveCore(options)) + + core_target = None + try: + core_target = self.dbg.CreateTarget(None) + core_process = core_target.LoadCore(core_path) + self.assertTrue(core_process.IsValid()) + + core_error = lldb.SBError() + core_page = core_process.ReadMemory(region, 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_datasize_within_file(core_path) + finally: + self.dbg.DeleteTarget(target) + if core_target: + self.dbg.DeleteTarget(core_target) + 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..59180b634e3a5 --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump/partial_read/main.cpp @@ -0,0 +1,26 @@ +#include <cstdint> +#include <cstdio> +#include <cstring> +#include <sys/mman.h> +#include <sys/syscall.h> +#include <unistd.h> + +// A mapping whose tail is unreadable: a memfd one page long, mapped four pages +// long, so reads into the pages past the file's end fault. save-core used to +// truncate the range there; a memfd keeps the test off the filesystem. +int main() { + const size_t page = sysconf(_SC_PAGESIZE); + int fd = static_cast<int>(syscall(SYS_memfd_create, "lldb_hole", 0)); + if (fd < 0) + return 1; + if (ftruncate(fd, page) != 0) + return 1; + uint8_t *region = static_cast<uint8_t *>( + mmap(nullptr, 4 * page, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0)); + if (region == MAP_FAILED) + return 1; + memset(region, 0xAB, page); + printf("region = %p, page = %zu\n", (void *)region, page); + fflush(stdout); + return 0; // Set a breakpoint here +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
