Author: satyanarayana reddy janga Date: 2026-07-28T15:48:20-06:00 New Revision: 575898b60d805e6a7259a5db0a2d0ee9913f6790
URL: https://github.com/llvm/llvm-project/commit/575898b60d805e6a7259a5db0a2d0ee9913f6790 DIFF: https://github.com/llvm/llvm-project/commit/575898b60d805e6a7259a5db0a2d0ee9913f6790.diff LOG: [lldb][Minidump] Fix wrong variable in getModuleFileSize loop (#188363) Use next_sect_sp instead of sect_sp when getting the byte size inside the section-walking loop. Using the original sect_sp meant sect_size never changed between iterations, producing an incorrect SizeOfImage and risking an infinite loop when sections have different sizes. Added: lldb/test/API/functionalities/process_save_core_minidump_size_of_image/Makefile lldb/test/API/functionalities/process_save_core_minidump_size_of_image/TestMinidumpSizeOfImage.py lldb/test/API/functionalities/process_save_core_minidump_size_of_image/main.c lldb/test/API/functionalities/process_save_core_minidump_size_of_image/testlib.c Modified: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 8fca95512ace3..876eea28a8923 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -279,7 +279,7 @@ llvm::Expected<uint64_t> getModuleFileSize(Target &target, lldb::SectionSP next_sect_sp = sect_so_addr.GetSection(); while (next_sect_sp && next_sect_sp->GetLoadBaseAddress(&target) == next_sect_addr) { - sect_size = sect_sp->GetByteSize(); + sect_size = next_sect_sp->GetByteSize(); SizeOfImage += sect_size; next_sect_addr += sect_size; target.ResolveLoadAddress(next_sect_addr, sect_so_addr); diff --git a/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/Makefile b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/Makefile new file mode 100644 index 0000000000000..2f94f1a99c0cf --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/Makefile @@ -0,0 +1,4 @@ +C_SOURCES := main.c +LD_EXTRAS := -ldl + +include Makefile.rules diff --git a/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/TestMinidumpSizeOfImage.py b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/TestMinidumpSizeOfImage.py new file mode 100644 index 0000000000000..81f799dd4c655 --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/TestMinidumpSizeOfImage.py @@ -0,0 +1,155 @@ +""" +Regression test for getModuleFileSize using sect_sp instead of next_sect_sp, +producing incorrect SizeOfImage when contiguous sections have diff erent sizes. +""" + +import os +import struct +import subprocess +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class MinidumpSizeOfImageTestCase(TestBase): + def build_shared_lib(self): + """Build a shared library with two contiguous PT_LOAD segments + of diff erent sizes via a linker script.""" + testlib_src = os.path.join(self.getSourceDir(), "testlib.c") + testlib_out = self.getBuildArtifact("libtestlib.so") + lds = self.getBuildArtifact("contig.lds") + + # Pad the RO segment to a page boundary so the RW segment starts + # immediately after, creating contiguous segments of diff erent sizes. + with open(lds, "w") as f: + f.write( + "PHDRS { ro PT_LOAD FLAGS(5); rw PT_LOAD FLAGS(6);" + " dyn PT_DYNAMIC FLAGS(6); }\n" + "SECTIONS {\n" + " . = SIZEOF_HEADERS;\n" + " .hash : { *(.hash) } :ro\n" + " .gnu.hash : { *(.gnu.hash) } :ro\n" + " .dynsym : { *(.dynsym) } :ro\n" + " .dynstr : { *(.dynstr) } :ro\n" + " .rela.dyn : { *(.rela.dyn) } :ro\n" + " .rela.plt : { *(.rela.plt) } :ro\n" + " .plt : { *(.plt) } :ro\n" + " .text : { *(.text .text.*) } :ro\n" + " .rodata : { *(.rodata .rodata.*) } :ro\n" + " .eh_frame_hdr : { *(.eh_frame_hdr) } :ro\n" + " .eh_frame : { *(.eh_frame) . = ALIGN(0x1000); } :ro\n" + " .dynamic : { *(.dynamic) } :rw :dyn\n" + " .data.rel.ro : { *(.data.rel.ro .data.rel.ro.*) } :rw\n" + " .got : { *(.got) } :rw\n" + " .got.plt : { *(.got.plt) } :rw\n" + " .data : { *(.data .data.*) } :rw\n" + " .bss : { *(.bss .bss.*) } :rw\n" + " /DISCARD/ : { *(.comment) *(.note.*)" + " *(.gnu.build.attributes) }\n" + "}\n" + ) + + subprocess.check_call( + [ + self.getCompiler(), + "-shared", + "-fPIC", + "-Wl,-T," + lds, + "-o", + testlib_out, + testlib_src, + ] + ) + return testlib_out + + def get_minidump_size_of_image(self, filepath, module_name): + """Return SizeOfImage from a minidump for the module whose name + contains module_name.""" + with open(filepath, "rb") as f: + data = f.read() + + num_streams = struct.unpack_from("<I", data, 8)[0] + stream_dir_rva = struct.unpack_from("<I", data, 12)[0] + + for i in range(num_streams): + stype, _, rva = struct.unpack_from("<III", data, stream_dir_rva + i * 12) + if stype != 4: # ModuleListStream + continue + num_modules = struct.unpack_from("<I", data, rva)[0] + for j in range(num_modules): + off = rva + 4 + j * 108 + size_of_image = struct.unpack_from("<I", data, off + 8)[0] + name_rva = struct.unpack_from("<I", data, off + 20)[0] + name_len = struct.unpack_from("<I", data, name_rva)[0] + name = data[name_rva + 4 : name_rva + 4 + name_len].decode( + "utf-16-le", errors="replace" + ) + if module_name in name: + return size_of_image + return None + + def get_expected_size(self, target, module): + """Compute expected SizeOfImage the same way getModuleFileSize does: + start with the first segment's byte size, then walk contiguous leaf + (deepest child) sections.""" + first = module.GetSectionAtIndex(0) + first_addr = first.GetLoadAddress(target) + total = first.GetByteSize() + next_addr = first_addr + total + + # Build a map of leaf-section-address → size + leaves = {} + for i in range(module.GetNumSections()): + sec = module.GetSectionAtIndex(i) + if sec.GetNumSubSections() == 0: + a = sec.GetLoadAddress(target) + if a != lldb.LLDB_INVALID_ADDRESS: + leaves[a] = sec.GetByteSize() + else: + for j in range(sec.GetNumSubSections()): + c = sec.GetSubSectionAtIndex(j) + a = c.GetLoadAddress(target) + if a != lldb.LLDB_INVALID_ADDRESS: + leaves[a] = c.GetByteSize() + + while next_addr in leaves: + total += leaves[next_addr] + next_addr += leaves[next_addr] + + return total + + @skipUnlessArch("x86_64") + @skipUnlessPlatform(["linux"]) + def test_size_of_image_with_contiguous_segments(self): + """Test SizeOfImage is correct for a module with contiguous segments.""" + self.build() + testlib_path = self.build_shared_lib() + exe = self.getBuildArtifact("a.out") + core_path = self.getBuildArtifact("core.dmp") + + target = self.dbg.CreateTarget(exe) + process = target.LaunchSimple( + [testlib_path], None, self.get_process_working_directory() + ) + self.assertState(process.GetState(), lldb.eStateStopped) + + testlib = target.FindModule(lldb.SBFileSpec("libtestlib.so")) + self.assertTrue(testlib.IsValid(), "libtestlib.so not loaded") + + expected = self.get_expected_size(target, testlib) + + options = lldb.SBSaveCoreOptions() + options.SetOutputFile(lldb.SBFileSpec(core_path)) + options.SetPluginName("minidump") + options.SetStyle(lldb.eSaveCoreStackOnly) + error = process.SaveCore(options) + self.assertTrue(error.Success(), error.GetCString()) + + actual = self.get_minidump_size_of_image(core_path, "libtestlib") + self.assertIsNotNone(actual, "libtestlib not found in minidump") + self.assertEqual( + actual, expected, f"SizeOfImage: got {actual:#x}, want {expected:#x}" + ) + + self.assertSuccess(process.Kill()) + self.dbg.DeleteTarget(target) diff --git a/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/main.c b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/main.c new file mode 100644 index 0000000000000..35e05e190ea3d --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/main.c @@ -0,0 +1,27 @@ +#include <dlfcn.h> +#include <signal.h> +#include <stdio.h> + +int main(int argc, char *argv[]) { + if (argc < 2) { + fprintf(stderr, "usage: %s <path-to-libtestlib.so>\n", argv[0]); + return 1; + } + + void *handle = dlopen(argv[1], RTLD_NOW); + if (!handle) { + fprintf(stderr, "dlopen failed: %s\n", dlerror()); + return 1; + } + + int (*func)(int) = dlsym(handle, "lib_func"); + if (!func) { + fprintf(stderr, "dlsym failed: %s\n", dlerror()); + return 1; + } + + printf("result: %d\n", func(21)); + raise(SIGSTOP); // stop here for the debugger + dlclose(handle); + return 0; +} diff --git a/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/testlib.c b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/testlib.c new file mode 100644 index 0000000000000..92af3631fac87 --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump_size_of_image/testlib.c @@ -0,0 +1,9 @@ +// Padding to fill the read-only segment to exactly one page (0x1000). +// The linker script places .text and .pad_ro in the first PT_LOAD, +// and .data in the second PT_LOAD, making them contiguous at a page +// boundary with diff erent sizes. +int lib_func(int x) { return x * 2; } + +// Writable data in the second segment +int lib_data = 100; +char lib_buf[256] = {1}; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
