From c7522272ffafa9b162f135aaee5d02a8895fcb0b Mon Sep 17 00:00:00 2001 From: Nelson Billing Date: Thu, 27 Feb 2020 14:04:01 -0800 Subject: [PATCH] Add "type" option to sym_upload sym-upload-v2 mode. - "sym-upload-v2" protocol now supports specifying a symbol file "type". - Known types are "breakpad" (default option, previously this was only effectively the only option), "elf", "pe", "macho", "debug_only", "dwp", "pdb", and "dsym". - When type other than breakpad is specified, sym_upload tool requires the code_file and debug_id value (that it otherwise would have gotten from the text of the Breakpad symbol file). - This ultimately means that sym_upload can be used to upload native symbol files now, in addition to Breakpad symbol files. Change-Id: I3a331ba16f199d1d0025df735716ba5de298f522 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2078670 Reviewed-by: Mark Mentovai --- src/common/linux/symbol_collector_client.cc | 6 +- src/common/linux/symbol_collector_client.h | 3 +- src/common/linux/symbol_upload.cc | 60 +++++++++++------ src/common/linux/symbol_upload.h | 7 ++ src/tools/linux/symupload/sym_upload.cc | 73 +++++++++++++++++++-- 5 files changed, 120 insertions(+), 29 deletions(-) diff --git a/src/common/linux/symbol_collector_client.cc b/src/common/linux/symbol_collector_client.cc index ea995c4b..92b25ddb 100644 --- a/src/common/linux/symbol_collector_client.cc +++ b/src/common/linux/symbol_collector_client.cc @@ -102,7 +102,8 @@ CompleteUploadResult SymbolCollectorClient::CompleteUpload( const string& api_key, const string& upload_key, const string& debug_file, - const string& debug_id) { + const string& debug_id, + const string& type) { string header, response; long response_code; @@ -113,7 +114,8 @@ CompleteUploadResult SymbolCollectorClient::CompleteUpload( string body = "{ symbol_id: {" "debug_file: \"" + debug_file + "\", " - "debug_id: \"" + debug_id + "\" } }"; + "debug_id: \"" + debug_id + "\" }, " + "symbol_upload_type: \"" + type + "\" }"; if (!libcurl_wrapper->SendSimplePostRequest(url, body, diff --git a/src/common/linux/symbol_collector_client.h b/src/common/linux/symbol_collector_client.h index 5f811de4..0e23242a 100644 --- a/src/common/linux/symbol_collector_client.h +++ b/src/common/linux/symbol_collector_client.h @@ -71,7 +71,8 @@ class SymbolCollectorClient { const string& api_key, const string& upload_key, const string& debug_file, - const string& debug_id); + const string& debug_id, + const string& type); static SymbolStatus CheckSymbolStatus( LibcurlWrapper* libcurl_wrapper, diff --git a/src/common/linux/symbol_upload.cc b/src/common/linux/symbol_upload.cc index 99750fd1..87741a0a 100644 --- a/src/common/linux/symbol_upload.cc +++ b/src/common/linux/symbol_upload.cc @@ -155,17 +155,15 @@ bool SymUploadV1Start( } // |options| describes the current sym_upload options. -// |module_parts| contains the strings parsed from the MODULE entry of the -// Breakpad symbol file being uploaded. -// |compacted_id| is the debug_id from the MODULE entry of the Breakpad symbol -// file being uploaded, with all hyphens removed. +// |code_id| is the basename of the module for which symbols are being +// uploaded. +// |debug_id| is the debug_id of the module for which symbols are being +// uploaded. bool SymUploadV2Start( const Options& options, - std::vector module_parts, - const string& compacted_id) { - string debug_file = module_parts[4]; - string debug_id = compacted_id; - + const string& code_file, + const string& debug_id, + const string& type) { google_breakpad::LibcurlWrapper libcurl_wrapper; if (!libcurl_wrapper.Init()) { printf("Failed to init google_breakpad::LibcurlWrapper.\n"); @@ -177,7 +175,7 @@ bool SymUploadV2Start( &libcurl_wrapper, options.uploadURLStr, options.api_key, - debug_file, + code_file, debug_id); if (symbolStatus == SymbolStatus::Found) { printf("Symbol file already exists, upload aborted." @@ -230,8 +228,9 @@ bool SymUploadV2Start( options.uploadURLStr, options.api_key, upload_key, - debug_file, - debug_id); + code_file, + debug_id, + type); if (completeUploadResult == CompleteUploadResult::Error) { printf("Failed to complete upload.\n"); return false; @@ -247,17 +246,36 @@ bool SymUploadV2Start( //============================================================================= void Start(Options* options) { - std::vector module_parts; - if (!ModuleDataForSymbolFile(options->symbolsPath, &module_parts)) { - fprintf(stderr, "Failed to parse symbol file!\n"); - return; - } - - const string compacted_id = CompactIdentifier(module_parts[3]); - if (options->upload_protocol == UploadProtocol::SYM_UPLOAD_V2) { - options->success = SymUploadV2Start(*options, module_parts, compacted_id); + string code_file; + string debug_id; + string type; + + if (options->type.empty() || options->type == kBreakpadSymbolType) { + // Breakpad upload so read these from input file. + std::vector module_parts; + if (!ModuleDataForSymbolFile(options->symbolsPath, &module_parts)) { + fprintf(stderr, "Failed to parse symbol file!\n"); + return; + } + code_file = module_parts[4]; + debug_id = CompactIdentifier(module_parts[3]); + type = kBreakpadSymbolType; + } else { + // Native upload so these must be explicitly set. + code_file = options->code_file; + debug_id = options->debug_id; + type = options->type; + } + + options->success = SymUploadV2Start(*options, code_file, debug_id, type); } else { + std::vector module_parts; + if (!ModuleDataForSymbolFile(options->symbolsPath, &module_parts)) { + fprintf(stderr, "Failed to parse symbol file!\n"); + return; + } + const string compacted_id = CompactIdentifier(module_parts[3]); options->success = SymUploadV1Start(*options, module_parts, compacted_id); } } diff --git a/src/common/linux/symbol_upload.h b/src/common/linux/symbol_upload.h index 040e980f..9033152b 100644 --- a/src/common/linux/symbol_upload.h +++ b/src/common/linux/symbol_upload.h @@ -46,6 +46,8 @@ enum class UploadProtocol { SYM_UPLOAD_V2, }; +constexpr char kBreakpadSymbolType[] = "BREAKPAD"; + struct Options { Options() : upload_protocol(UploadProtocol::SYM_UPLOAD_V1), force(false) {} @@ -58,6 +60,11 @@ struct Options { UploadProtocol upload_protocol; bool force; string api_key; + + // These only need to be set for native symbol uploads. + string code_file; + string debug_id; + string type; }; // Starts upload to symbol server with options. diff --git a/src/tools/linux/symupload/sym_upload.cc b/src/tools/linux/symupload/sym_upload.cc index a9b9175b..f155eb95 100644 --- a/src/tools/linux/symupload/sym_upload.cc +++ b/src/tools/linux/symupload/sym_upload.cc @@ -44,11 +44,23 @@ #include #include +#include + #include "common/linux/symbol_upload.h" using google_breakpad::sym_upload::UploadProtocol; using google_breakpad::sym_upload::Options; +static void StrToUpper(std::string* str) { + if (str == nullptr) { + fprintf(stderr, "nullptr passed to StrToUpper.\n"); + exit(1); + } + for (size_t i = 0; i < str->length(); i++) { + (*str)[i] = std::toupper((*str)[i], std::locale::classic()); + } +} + //============================================================================= static void Usage(int argc, const char *argv[]) { @@ -61,23 +73,39 @@ Usage(int argc, const char *argv[]) { fprintf(stderr, " is the destination for the upload\n"); fprintf(stderr, "-p:\t One of ['sym-upload-v1'," " 'sym-upload-v2'], defaults to 'sym-upload-v1'.\n"); - fprintf(stderr, "-k:\t A secret used to authenticate with the" - " API [Only supported when using 'sym-upload-v2' protocol].\n"); - fprintf(stderr, "-f:\t Force symbol upload if already exists [Only" - " supported when using 'sym-upload-v2' protocol].\n"); fprintf(stderr, "-v:\t Version information (e.g., 1.2.3.4)\n"); fprintf(stderr, "-x:\t Use HTTP proxy on given port\n"); fprintf(stderr, "-u:\t Set proxy user and password\n"); fprintf(stderr, "-h:\t Usage\n"); fprintf(stderr, "-?:\t Usage\n"); fprintf(stderr, "\n"); + fprintf(stderr, "These options only work with 'sym-upload-v2' protocol:\n"); + fprintf(stderr, "-k:\t A secret used to authenticate with the" + " API.\n"); + fprintf(stderr, "-f:\t Force symbol upload if already exists.\n"); + fprintf(stderr, "-t:\t Explicitly set symbol upload type (" + "default is 'breakpad').\n" + "\t One of ['breakpad', 'elf', 'pe', 'macho', 'debug_only', 'dwp', " + "'dsym', 'pdb'].\n" + "\t Note: When this flag is set to anything other than 'breakpad', then " + "the '-c' and '-i' flags must also be set.\n"); + fprintf(stderr, "-c:\t Explicitly set 'code_file' for symbol " + "upload (basename of executable).\n"); + fprintf(stderr, "-i:\t Explicitly set 'debug_id' for symbol " + "upload (typically build ID of executable).\n"); + fprintf(stderr, "\n"); fprintf(stderr, "Examples:\n"); fprintf(stderr, " With 'sym-upload-v1':\n"); fprintf(stderr, " %s path/to/symbol_file http://myuploadserver\n", argv[0]); fprintf(stderr, " With 'sym-upload-v2':\n"); + fprintf(stderr, " [Defaulting to symbol type 'BREAKPAD']\n"); fprintf(stderr, " %s -p sym-upload-v2 -k mysecret123! " "path/to/symbol_file http://myuploadserver\n", argv[0]); + fprintf(stderr, " [Explicitly set symbol type to 'elf']\n"); + fprintf(stderr, " %s -p sym-upload-v2 -k mysecret123! -t elf " + "-c app -i 11111111BBBB3333DDDD555555555555F " + "path/to/symbol_file http://myuploadserver\n", argv[0]); } //============================================================================= @@ -85,8 +113,9 @@ static void SetupOptions(int argc, const char *argv[], Options *options) { extern int optind; int ch; + constexpr char flag_pattern[] = "u:v:x:p:k:t:c:i:hf?"; - while ((ch = getopt(argc, (char * const *)argv, "u:v:x:p:k:hf?")) != -1) { + while ((ch = getopt(argc, (char * const *)argv, flag_pattern)) != -1) { switch (ch) { case 'h': case '?': @@ -116,6 +145,19 @@ SetupOptions(int argc, const char *argv[], Options *options) { case 'k': options->api_key = optarg; break; + case 't': { + // This is really an enum, so treat as upper-case for consistency with + // enum naming convention on server-side. + options->type = optarg; + StrToUpper(&(options->type)); + break; + } + case 'c': + options->code_file = optarg; + break; + case 'i': + options->debug_id = optarg; + break; case 'f': options->force = true; break; @@ -134,6 +176,27 @@ SetupOptions(int argc, const char *argv[], Options *options) { exit(1); } + bool is_breakpad_upload = options->type.empty() || + options->type == google_breakpad::sym_upload::kBreakpadSymbolType; + bool has_code_file = !options->code_file.empty(); + bool has_debug_id = !options->debug_id.empty(); + if (is_breakpad_upload && (has_code_file || has_debug_id)) { + fprintf(stderr, "\n"); + fprintf(stderr, "%s: -c and -i should only be specified for non-breakpad " + "symbol upload types.\n", argv[0]); + fprintf(stderr, "\n"); + Usage(argc, argv); + exit(1); + } + if (!is_breakpad_upload && (!has_code_file || !has_debug_id)) { + fprintf(stderr, "\n"); + fprintf(stderr, "%s: -c and -i must be specified for non-breakpad " + "symbol upload types.\n", argv[0]); + fprintf(stderr, "\n"); + Usage(argc, argv); + exit(1); + } + options->symbolsPath = argv[optind]; options->uploadURLStr = argv[optind + 1]; }