From 417f5dbd0af9f96ca3e5faa99f41c064b89b40fd Mon Sep 17 00:00:00 2001 From: Jacob Hinton Date: Thu, 2 May 2024 15:19:16 -0700 Subject: preserve NT_FILE note converting md to core breakpad produced Minidumps have all the information needed to preserve the NT_FILE note that linux coredumps normally have, but are not currently included when converting using minidump-2-core. This adds a step to coredump generation to add the note to the coredump. This change allows for two improvements when loading coredumps into gdb: 1. for PIE code (default compilation now for gcc) gdb is now able to properly validate the PIE displacement and apply it to the debugging session, providing better stack trace quality. 2. gdb can now pick up so symbol files automatically without relying on the scripting normally recommended by google for breakpad where add-symbol-file is used to manually load symbols. Bug: https://crbug.com/google-breakpad/766 Change-Id: I8cb25246dce0ae3492eedd6d3a4efcf1783d414d Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/5463435 Reviewed-by: Mike Frysinger --- src/tools/linux/md2core/minidump-2-core.cc | 86 +++++++++++++++++++++++++++--- 1 file changed, 79 insertions(+), 7 deletions(-) diff --git a/src/tools/linux/md2core/minidump-2-core.cc b/src/tools/linux/md2core/minidump-2-core.cc index 3e310bc7..00384c40 100644 --- a/src/tools/linux/md2core/minidump-2-core.cc +++ b/src/tools/linux/md2core/minidump-2-core.cc @@ -349,6 +349,34 @@ struct CrashedProcess { std::vector link_map; }; +/* NT_FILE note as defined by linux kernel in fs/binfmt_elf.c + * is structured as: + * long count -- how many files are mapped + * long page_size -- units for file_ofs + * array of [COUNT] elements of + * long start + * long end + * long file_ofs + * followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL... + * we can re-use the file mappings info + */ +struct NtFileNote { + NtFileNote() + // XXX: we really should source page size from the minidump itself but + // I cannot find anywhere in the minidump generation code where this + // would be stashed. + : page_sz((unsigned long)getpagesize()), + filename_count(0), + filenames_length(0) { + } + + unsigned long page_sz; + unsigned long filename_count; + std::vector file_mappings; + std::vector filenames; + size_t filenames_length; +}; + #if defined(__i386__) static uint32_t U32(const uint8_t* data) { @@ -1298,6 +1326,8 @@ AugmentMappings(const Options& options, CrashedProcess* crashinfo, } AddDataToMapping(crashinfo, crashinfo->dynamic_data, (uintptr_t)crashinfo->debug.dynamic); + } else { + fprintf(stderr, "dynamic data empty\n"); } } @@ -1417,19 +1447,40 @@ main(int argc, const char* argv[]) { if (!writea(options.out_fd, &ehdr, sizeof(Ehdr))) return 1; + struct NtFileNote nt_file; + for (auto iter = crashinfo.mappings.begin(); + iter != crashinfo.mappings.end(); iter++) { + if (iter->second.filename.empty()) + continue; + nt_file.file_mappings.push_back(iter->second.start_address); + nt_file.file_mappings.push_back(iter->second.end_address); + nt_file.file_mappings.push_back(iter->second.offset); + nt_file.filenames.push_back(iter->second.filename); + nt_file.filenames_length += iter->second.filename.length() + 1; + nt_file.filename_count += 1; + } + // implementation of NT_FILE note seems to pad alignment by 4 bytes but + // keep the header size the true size of the note. so we keep nt_file_align + // as separate field. + size_t nt_file_data_sz = (2 * sizeof(unsigned long)) + + (nt_file.file_mappings.size() * sizeof(unsigned long)) + + nt_file.filenames_length; + size_t nt_file_align = nt_file_data_sz % 4 == 0 ? 0 : 4 - + (nt_file_data_sz % 4); size_t offset = sizeof(Ehdr) + ehdr.e_phnum * sizeof(Phdr); size_t filesz = sizeof(Nhdr) + 8 + sizeof(prpsinfo) + - // sizeof(Nhdr) + 8 + sizeof(user) + - sizeof(Nhdr) + 8 + crashinfo.auxv_length + - crashinfo.threads.size() * ( - (sizeof(Nhdr) + 8 + sizeof(prstatus)) + // sizeof(Nhdr) + 8 + sizeof(user) + + sizeof(Nhdr) + 8 + crashinfo.auxv_length + + sizeof(Nhdr) + 8 + nt_file_data_sz + nt_file_align + + crashinfo.threads.size() * ( + (sizeof(Nhdr) + 8 + sizeof(prstatus)) #if defined(__i386__) || defined(__x86_64__) - + sizeof(Nhdr) + 8 + sizeof(user_fpregs_struct) + + sizeof(Nhdr) + 8 + sizeof(user_fpregs_struct) #endif #if defined(__i386__) - + sizeof(Nhdr) + 8 + sizeof(user_fpxregs_struct) + + sizeof(Nhdr) + 8 + sizeof(user_fpxregs_struct) #endif - ); + ); Phdr phdr; memset(&phdr, 0, sizeof(Phdr)); @@ -1492,6 +1543,27 @@ main(int argc, const char* argv[]) { return 1; } + nhdr.n_descsz = nt_file_data_sz; + nhdr.n_type = NT_FILE; + if (!writea(options.out_fd, &nhdr, sizeof(nhdr)) || + !writea(options.out_fd, "CORE\0\0\0\0", 8) || + !writea(options.out_fd, + &nt_file.filename_count, sizeof(nt_file.filename_count)) || + !writea(options.out_fd, &nt_file.page_sz, sizeof(nt_file.page_sz))) { + return 1; + } + for (auto iter = nt_file.file_mappings.begin(); + iter != nt_file.file_mappings.end(); iter++) { + if (!writea(options.out_fd, &*iter, sizeof(*iter))) + return 1; + } + for (auto iter = nt_file.filenames.begin(); + iter != nt_file.filenames.end(); iter++) { + if (!writea(options.out_fd, iter->c_str(), iter->length() + 1)) + return 1; + } + writea(options.out_fd, "\0\0\0\0", nt_file_align); + for (const auto& current_thread : crashinfo.threads) { if (current_thread.tid == crashinfo.exception.tid) { // Use the exception record's context for the crashed thread instead of -- cgit v1.2.3