From 9cc38fec8bbf4efc58f077c975e7f2b422a74ee3 Mon Sep 17 00:00:00 2001 From: Ben Hamilton Date: Mon, 20 Mar 2023 11:45:05 -0600 Subject: [PATCH] [dump_syms/Mac] New -n MODULE arg to Mac dump_syms Previously, dump_syms always used the basename of the on-disk file as the Breakpad module name and required that the on-disk filename of the dSYM and binary file match, or it would exit with an error. Build automation often uses filenames unrelated to the Breakpad module name, so this CL adds a new optional "-n MODULE" argument to Mac dump_syms that allows passing in the Breakpad module name from outside. In this case, the basename of the on-disk file(s) is ignored and no longer required to match. Change-Id: Ic38e8cf762c79bce61d289b397293eff6c0039ce Bug: b/273531493 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4338857 Reviewed-by: Robert Sesek --- src/common/mac/dump_syms.cc | 7 +++- src/common/mac/dump_syms.h | 17 ++++++-- src/tools/mac/dump_syms/dump_syms_tool.cc | 49 +++++++++++++++++++---- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/common/mac/dump_syms.cc b/src/common/mac/dump_syms.cc index efa60f5b..6396e97a 100644 --- a/src/common/mac/dump_syms.cc +++ b/src/common/mac/dump_syms.cc @@ -429,7 +429,12 @@ bool DumpSymbols::CreateEmptyModule(scoped_ptr& module) { } // Compute a module name, to appear in the MODULE record. - string module_name = google_breakpad::BaseName(object_filename_); + string module_name; + if (!module_name_.empty()) { + module_name = module_name_; + } else { + module_name = google_breakpad::BaseName(object_filename_); + } // Choose an identifier string, to appear in the MODULE record. string identifier = Identifier(); diff --git a/src/common/mac/dump_syms.h b/src/common/mac/dump_syms.h index c22a0575..d5aa7185 100644 --- a/src/common/mac/dump_syms.h +++ b/src/common/mac/dump_syms.h @@ -56,7 +56,8 @@ class DumpSymbols { public: DumpSymbols(SymbolData symbol_data, bool handle_inter_cu_refs, - bool enable_multiple = false) + bool enable_multiple = false, + const std::string& module_name = "") : symbol_data_(symbol_data), handle_inter_cu_refs_(handle_inter_cu_refs), object_filename_(), @@ -66,12 +67,18 @@ class DumpSymbols { object_files_(), selected_object_file_(), selected_object_name_(), - enable_multiple_(enable_multiple) {} + enable_multiple_(enable_multiple), + module_name_(module_name) {} ~DumpSymbols() = default; // Prepare to read debugging information from |filename|. |filename| may be // the name of a fat file, a Mach-O file, or a dSYM bundle containing either - // of the above. On success, return true; if there is a problem reading + // of the above. + // + // If |module_name_| is empty, uses the basename of |filename| as the module + // name. Otherwise, uses |module_name_| as the module name. + // + // On success, return true; if there is a problem reading // |filename|, report it and return false. bool Read(const std::string& filename); @@ -194,6 +201,10 @@ class DumpSymbols { // See: https://crbug.com/google-breakpad/751 and docs at // docs/symbol_files.md#records-3 bool enable_multiple_; + + // If non-empty, used as the module name. Otherwise, the basename of + // |object_filename_| is used as the module name. + const std::string module_name_; }; } // namespace google_breakpad diff --git a/src/tools/mac/dump_syms/dump_syms_tool.cc b/src/tools/mac/dump_syms/dump_syms_tool.cc index 4d6f25c9..ab36164f 100644 --- a/src/tools/mac/dump_syms/dump_syms_tool.cc +++ b/src/tools/mac/dump_syms/dump_syms_tool.cc @@ -63,7 +63,8 @@ struct Options { cfi(true), handle_inter_cu_refs(true), handle_inlines(false), - enable_multiple(false) {} + enable_multiple(false), + module_name() {} string srcPath; string dsymPath; @@ -73,6 +74,7 @@ struct Options { bool handle_inter_cu_refs; bool handle_inlines; bool enable_multiple; + string module_name; }; static bool StackFrameEntryComparator(const Module::StackFrameEntry* a, @@ -149,7 +151,7 @@ static bool Start(const Options& options) { (options.handle_inlines ? INLINES : NO_DATA) | (options.cfi ? CFI : NO_DATA) | SYMBOLS_AND_FILES; DumpSymbols dump_symbols(symbol_data, options.handle_inter_cu_refs, - options.enable_multiple); + options.enable_multiple, options.module_name); // For x86_64 binaries, the CFI data is in the __TEXT,__eh_frame of the // Mach-O file, which is not copied into the dSYM. Whereas in i386, the CFI @@ -196,13 +198,38 @@ static bool Start(const Options& options) { return false; scoped_ptr scoped_cfi_module(cfi_module); + bool name_matches; + if (!options.module_name.empty()) { + // Ignore the basename of the dSYM and binary and use the passed-in module + // name. + name_matches = true; + } else { + name_matches = cfi_module->name() == module->name(); + } + // Ensure that the modules are for the same debug code file. - if (cfi_module->name() != module->name() || - cfi_module->os() != module->os() || + if (!name_matches || cfi_module->os() != module->os() || cfi_module->architecture() != module->architecture() || cfi_module->identifier() != module->identifier()) { fprintf(stderr, "Cannot generate a symbol file from split sources that do" " not match.\n"); + if (!name_matches) { + fprintf(stderr, "Name mismatch: binary=[%s], dSYM=[%s]\n", + cfi_module->name().c_str(), module->name().c_str()); + } + if (cfi_module->os() != module->os()) { + fprintf(stderr, "OS mismatch: binary=[%s], dSYM=[%s]\n", + cfi_module->os().c_str(), module->os().c_str()); + } + if (cfi_module->architecture() != module->architecture()) { + fprintf(stderr, "Architecture mismatch: binary=[%s], dSYM=[%s]\n", + cfi_module->architecture().c_str(), + module->architecture().c_str()); + } + if (cfi_module->identifier() != module->identifier()) { + fprintf(stderr, "Identifier mismatch: binary=[%s], dSYM=[%s]\n", + cfi_module->identifier().c_str(), module->identifier().c_str()); + } return false; } @@ -215,8 +242,10 @@ static bool Start(const Options& options) { //============================================================================= static void Usage(int argc, const char *argv[]) { fprintf(stderr, "Output a Breakpad symbol file from a Mach-o file.\n"); - fprintf(stderr, "Usage: %s [-a ARCHITECTURE] [-c] [-g dSYM path] " - "\n", argv[0]); + fprintf(stderr, + "Usage: %s [-a ARCHITECTURE] [-c] [-g dSYM path] " + "[-n MODULE] \n", + argv[0]); fprintf(stderr, "\t-i: Output module header information only.\n"); fprintf(stderr, "\t-a: Architecture type [default: native, or whatever is\n"); fprintf(stderr, "\t in the file, if it contains only one architecture]\n"); @@ -228,6 +257,9 @@ static void Usage(int argc, const char *argv[]) { fprintf(stderr, "\t-m: Enable writing the optional 'm' field on FUNC " "and PUBLIC, denoting multiple symbols for the address.\n"); + fprintf(stderr, + "\t-n: Use MODULE as the name of the module rather than \n" + "the basename of the Mach-O file/dSYM.\n"); fprintf(stderr, "\t-h: Usage\n"); fprintf(stderr, "\t-?: Usage\n"); } @@ -237,7 +269,7 @@ static void SetupOptions(int argc, const char *argv[], Options *options) { extern int optind; signed char ch; - while ((ch = getopt(argc, (char* const*)argv, "ia:g:crdm?h")) != -1) { + while ((ch = getopt(argc, (char* const*)argv, "ia:g:crdm?hn:")) != -1) { switch (ch) { case 'i': options->header_only = true; @@ -267,6 +299,9 @@ static void SetupOptions(int argc, const char *argv[], Options *options) { case 'm': options->enable_multiple = true; break; + case 'n': + options->module_name = optarg; + break; case '?': case 'h': Usage(argc, argv);