diff options
Diffstat (limited to 'toolkit/crashreporter')
11 files changed, 160 insertions, 75 deletions
diff --git a/toolkit/crashreporter/breakpad-client/linux/crash_generation/client_info.h b/toolkit/crashreporter/breakpad-client/linux/crash_generation/client_info.h index 3de2606b7b..80b70b045f 100644 --- a/toolkit/crashreporter/breakpad-client/linux/crash_generation/client_info.h +++ b/toolkit/crashreporter/breakpad-client/linux/crash_generation/client_info.h @@ -30,6 +30,8 @@ #ifndef CLIENT_LINUX_CRASH_GENERATION_CLIENT_INFO_H_ #define CLIENT_LINUX_CRASH_GENERATION_CLIENT_INFO_H_ +#include <sys/types.h> + namespace google_breakpad { class CrashGenerationServer; @@ -42,13 +44,13 @@ class ClientInfo { CrashGenerationServer* crash_server() const { return crash_server_; } pid_t pid() const { return pid_; } - void set_error_msg(nsCString &error_msg) { + void set_error_msg(char *error_msg) { had_error_ = true; error_msg_ = error_msg; } - const nsCString* error_msg() const { - return &error_msg_; + const char* error_msg() const { + return error_msg_; } bool had_error() const { @@ -59,8 +61,8 @@ class ClientInfo { CrashGenerationServer* crash_server_; pid_t pid_; bool had_error_ = false; - nsCString error_msg_; // Possible error message of the minidumper in - // case there was an error during dumping + char* error_msg_ = nullptr; // Possible error message of the minidumper in + // case there was an error during dumping }; } diff --git a/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc b/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc index 58baa31d10..35e3e0f80c 100644 --- a/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc +++ b/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc @@ -41,8 +41,6 @@ #include <vector> -#include "nsThreadUtils.h" - #include "linux/crash_generation/crash_generation_server.h" #include "linux/crash_generation/client_info.h" #include "linux/handler/exception_handler.h" @@ -54,7 +52,6 @@ #if defined(MOZ_OXIDIZED_BREAKPAD) # include "mozilla/toolkit/crashreporter/rust_minidump_writer_linux_ffi_generated.h" # include <sys/signalfd.h> -# include "nsString.h" #endif static const char kCommandQuit = 'x'; @@ -275,7 +272,7 @@ CrashGenerationServer::ClientEvent(short revents) #if defined(MOZ_OXIDIZED_BREAKPAD) ExceptionHandler::CrashContext* breakpad_cc = reinterpret_cast<ExceptionHandler::CrashContext*>(crash_context); - nsCString error_msg; + char* error_msg = nullptr; siginfo_t& si = breakpad_cc->siginfo; signalfd_siginfo signalfd_si = {}; signalfd_si.ssi_signo = si.si_signo; @@ -330,6 +327,11 @@ CrashGenerationServer::ClientEvent(short revents) exit_callback_(exit_context_, info); } + info.set_error_msg(nullptr); + if (error_msg) { + free_minidump_error_msg(error_msg); + } + return true; } @@ -375,7 +377,6 @@ CrashGenerationServer::MakeMinidumpFilename(string& outFilename) void* CrashGenerationServer::ThreadMain(void *arg) { - NS_SetCurrentThreadName("Breakpad Server"); reinterpret_cast<CrashGenerationServer*>(arg)->Run(); return NULL; } diff --git a/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.cc b/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.cc index 92a0443d79..efb97c00cb 100644 --- a/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.cc +++ b/toolkit/crashreporter/breakpad-client/linux/handler/exception_handler.cc @@ -97,7 +97,6 @@ #include "common/linux/eintr_wrapper.h" #include "third_party/lss/linux_syscall_support.h" #if defined(MOZ_OXIDIZED_BREAKPAD) -#include "nsString.h" #include "mozilla/toolkit/crashreporter/rust_minidump_writer_linux_ffi_generated.h" #endif @@ -854,9 +853,10 @@ bool ExceptionHandler::WriteMinidumpForChild(pid_t child, MinidumpDescriptor descriptor(dump_path); descriptor.UpdatePath(); #if defined(MOZ_OXIDIZED_BREAKPAD) - nsCString error_msg; - if (!write_minidump_linux(descriptor.path(), child, child_blamed_thread, &error_msg)) + char* error_msg; + if (!write_minidump_linux(descriptor.path(), child, child_blamed_thread, &error_msg)) { return false; + } #else if (!google_breakpad::WriteMinidump(descriptor.path(), child, diff --git a/toolkit/crashreporter/client/app/Cargo.toml b/toolkit/crashreporter/client/app/Cargo.toml index 381ed32f97..a2ba090653 100644 --- a/toolkit/crashreporter/client/app/Cargo.toml +++ b/toolkit/crashreporter/client/app/Cargo.toml @@ -12,7 +12,7 @@ cfg-if = "1.0" env_logger = { version = "0.10", default-features = false } fluent = "0.16.0" intl-memoizer = "0.5" -libloading = "0.7" +libloading = "0.8" log = "0.4.17" mozbuild = "0.1" mozilla-central-workspace-hack = { version = "0.1", features = ["crashreporter"], optional = true } diff --git a/toolkit/crashreporter/client/app/src/ui/crashreporter.png b/toolkit/crashreporter/client/app/src/ui/crashreporter.png Binary files differindex 5e68bac17c..1ceb540ff0 100644 --- a/toolkit/crashreporter/client/app/src/ui/crashreporter.png +++ b/toolkit/crashreporter/client/app/src/ui/crashreporter.png diff --git a/toolkit/crashreporter/client/app/src/ui/windows/window.rs b/toolkit/crashreporter/client/app/src/ui/windows/window.rs index 7e5e8f3f2a..0188ca8216 100644 --- a/toolkit/crashreporter/client/app/src/ui/windows/window.rs +++ b/toolkit/crashreporter/client/app/src/ui/windows/window.rs @@ -84,6 +84,15 @@ pub trait CustomWindowClass: WindowClass { 0, win::SWP_NOMOVE | win::SWP_NOSIZE | win::SWP_NOZORDER | win::SWP_FRAMECHANGED, )); + + // For reasons that don't make much sense, icons scale poorly unless set with + // `WM_SETICON` (see bug 1891920). + let icon = W::icon(); + if icon != 0 { + // The return value doesn't indicate failures. + win::SendMessageW(hwnd, win::WM_SETICON, win::ICON_SMALL as _, icon); + win::SendMessageW(hwnd, win::WM_SETICON, win::ICON_BIG as _, icon); + } } let result = unsafe { W::get(hwnd).as_ref() } diff --git a/toolkit/crashreporter/generate_crash_reporter_sources.py b/toolkit/crashreporter/generate_crash_reporter_sources.py index b8c3333557..3b75e43bce 100644 --- a/toolkit/crashreporter/generate_crash_reporter_sources.py +++ b/toolkit/crashreporter/generate_crash_reporter_sources.py @@ -18,10 +18,16 @@ template_header = ( ) +def sort_annotations(annotations): + """Return annotations in ascending alphabetical order ignoring case""" + + return sorted(annotations.items(), key=lambda annotation: str.lower(annotation[0])) + + def validate_annotations(annotations): """Ensure that the annotations have all the required fields""" - for name, data in sorted(annotations.items()): + for name, data in annotations: if "description" not in data: print("Annotation " + name + " does not have a description\n") sys.exit(1) @@ -48,7 +54,7 @@ def read_annotations(annotations_filename): try: with open(annotations_filename, "r") as annotations_file: - annotations = yaml.safe_load(annotations_file) + annotations = sort_annotations(yaml.safe_load(annotations_file)) except (IOError, ValueError) as e: print("Error parsing " + annotations_filename + ":\n" + str(e) + "\n") sys.exit(1) @@ -76,9 +82,7 @@ def extract_crash_ping_allowedlist(annotations): """Extract an array holding the names of the annotations allowed for inclusion in the crash ping.""" - return [ - name for (name, data) in sorted(annotations.items()) if data.get("ping", False) - ] + return [name for (name, data) in annotations if data.get("ping", False)] def extract_skiplist(annotations): @@ -87,7 +91,7 @@ def extract_skiplist(annotations): return [ (name, data.get("skip_if")) - for (name, data) in sorted(annotations.items()) + for (name, data) in annotations if len(data.get("skip_if", "")) > 0 ] @@ -110,7 +114,7 @@ def type_to_enum(annotation_type): def extract_types(annotations): """Extract an array holding the type of each annotation.""" - return [type_to_enum(data.get("type")) for (_, data) in sorted(annotations.items())] + return [type_to_enum(data.get("type")) for (_, data) in annotations] ############################################################################### @@ -121,10 +125,7 @@ def extract_types(annotations): def generate_strings(annotations): """Generate strings corresponding to every annotation.""" - names = [ - ' "' + data.get("altname", name) + '"' - for (name, data) in sorted(annotations.items()) - ] + names = [' "' + data.get("altname", name) + '"' for (name, data) in annotations] return ",\n".join(names) @@ -135,7 +136,7 @@ def generate_enum(annotations): enum = "" - for i, (name, _) in enumerate(sorted(annotations.items())): + for i, (name, _) in enumerate(annotations): enum += " " + name + " = " + str(i) + ",\n" enum += " Count = " + str(len(annotations)) diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp index 868005a5c7..0f8a58aa88 100644 --- a/toolkit/crashreporter/nsExceptionHandler.cpp +++ b/toolkit/crashreporter/nsExceptionHandler.cpp @@ -96,6 +96,9 @@ using mozilla::InjectCrashRunnable; #endif +#ifdef XP_WIN +# include <filesystem> +#endif #include <fstream> #include <optional> @@ -2768,12 +2771,23 @@ static nsresult PrefSubmitReports(bool* aSubmitReports, bool writePref) { * toolkit/crashreporter/client/app/src/{logic,settings}.rs */ nsCOMPtr<nsIFile> reporterSettings; - rv = NS_GetSpecialDirectory("UAppData", getter_AddRefs(reporterSettings)); + rv = NS_GetSpecialDirectory(XRE_USER_APP_DATA_DIR, + getter_AddRefs(reporterSettings)); NS_ENSURE_SUCCESS(rv, rv); reporterSettings->AppendNative("Crash Reports"_ns); reporterSettings->AppendNative("crashreporter_settings.json"_ns); + // On e.g. Linux, std::filesystem requires sometimes linking libstdc++fs, + // and we don't do that yet, so limit the use of std::filesystem::path where + // it's really needed, which is Windows, because implicit conversions from + // wstring in fstream constructors are not supported as of + // https://cplusplus.github.io/LWG/issue3430. +# ifdef XP_WIN + std::optional<std::filesystem::path> file_path = + CreatePathFromFile(reporterSettings); +# else std::optional<xpstring> file_path = CreatePathFromFile(reporterSettings); +# endif if (!file_path) { return NS_ERROR_FAILURE; @@ -2900,7 +2914,7 @@ void UpdateCrashEventsDir() { return; } - rv = NS_GetSpecialDirectory("UAppData", getter_AddRefs(eventsDir)); + rv = NS_GetSpecialDirectory(XRE_USER_APP_DATA_DIR, getter_AddRefs(eventsDir)); if (NS_SUCCEEDED(rv)) { SetUserAppDataDirectory(eventsDir); return; @@ -2962,7 +2976,8 @@ static void FindPendingDir() { return; } nsCOMPtr<nsIFile> pendingDir; - nsresult rv = NS_GetSpecialDirectory("UAppData", getter_AddRefs(pendingDir)); + nsresult rv = + NS_GetSpecialDirectory(XRE_USER_APP_DATA_DIR, getter_AddRefs(pendingDir)); if (NS_FAILED(rv)) { NS_WARNING( "Couldn't get the user appdata directory, crash dumps will go in an " diff --git a/toolkit/crashreporter/rust_minidump_writer_linux/Cargo.toml b/toolkit/crashreporter/rust_minidump_writer_linux/Cargo.toml index b4bbd8e40b..8c414d8acf 100644 --- a/toolkit/crashreporter/rust_minidump_writer_linux/Cargo.toml +++ b/toolkit/crashreporter/rust_minidump_writer_linux/Cargo.toml @@ -9,7 +9,6 @@ license = "MPL-2.0" [dependencies] crash-context = "0.6.1" -minidump-writer = "0.8.3" +minidump-writer = "0.8.9" libc = "0.2.74" anyhow = "1.0" -nsstring = { path = "../../../xpcom/rust/nsstring/" } diff --git a/toolkit/crashreporter/rust_minidump_writer_linux/src/lib.rs b/toolkit/crashreporter/rust_minidump_writer_linux/src/lib.rs index 21ae630c5e..06ad386484 100644 --- a/toolkit/crashreporter/rust_minidump_writer_linux/src/lib.rs +++ b/toolkit/crashreporter/rust_minidump_writer_linux/src/lib.rs @@ -6,10 +6,10 @@ extern crate minidump_writer; use libc::pid_t; use minidump_writer::crash_context::CrashContext; use minidump_writer::minidump_writer::MinidumpWriter; -use nsstring::nsCString; -use std::ffi::CStr; +use std::ffi::{CStr, CString}; use std::mem::{self, MaybeUninit}; use std::os::raw::c_char; +use std::ptr::null_mut; // This function will be exposed to C++ #[no_mangle] @@ -17,17 +17,20 @@ pub unsafe extern "C" fn write_minidump_linux( dump_path: *const c_char, child: pid_t, child_blamed_thread: pid_t, - error_msg: &mut nsCString, + error_msg: *mut *mut c_char, ) -> bool { assert!(!dump_path.is_null()); let c_path = CStr::from_ptr(dump_path); let path = match c_path.to_str() { Ok(s) => s, Err(x) => { - error_msg.assign(&format!( - "Wrapper error. Path not convertable: {:#}", - anyhow::Error::new(x) - )); + error_message_to_c( + error_msg, + format!( + "Wrapper error. Path not convertable: {:#}", + anyhow::Error::new(x) + ), + ); return false; } }; @@ -39,11 +42,14 @@ pub unsafe extern "C" fn write_minidump_linux( { Ok(f) => f, Err(x) => { - error_msg.assign(&format!( - "Wrapper error when opening minidump destination at {:?}: {:#}", - path, - anyhow::Error::new(x) - )); + error_message_to_c( + error_msg, + format!( + "Wrapper error when opening minidump destination at {:?}: {:#}", + path, + anyhow::Error::new(x) + ), + ); return false; } }; @@ -53,7 +59,7 @@ pub unsafe extern "C" fn write_minidump_linux( return true; } Err(x) => { - error_msg.assign(&format!("{:#}", anyhow::Error::new(x))); + error_message_to_c(error_msg, format!("{:#}", anyhow::Error::new(x))); return false; } } @@ -75,7 +81,7 @@ pub unsafe extern "C" fn write_minidump_linux_with_context( #[allow(unused)] float_state: *const fpregset_t, siginfo: *const libc::signalfd_siginfo, child_thread: libc::pid_t, - error_msg: &mut nsCString, + error_msg: *mut *mut c_char, ) -> bool { let c_path = CStr::from_ptr(dump_path); @@ -97,10 +103,13 @@ pub unsafe extern "C" fn write_minidump_linux_with_context( let path = match c_path.to_str() { Ok(s) => s, Err(x) => { - error_msg.assign(&format!( - "Wrapper error. Path not convertable: {:#}", - anyhow::Error::new(x) - )); + error_message_to_c( + error_msg, + format!( + "Wrapper error. Path not convertable: {:#}", + anyhow::Error::new(x) + ), + ); return false; } }; @@ -112,11 +121,14 @@ pub unsafe extern "C" fn write_minidump_linux_with_context( { Ok(f) => f, Err(x) => { - error_msg.assign(&format!( - "Wrapper error when opening minidump destination at {:?}: {:#}", - path, - anyhow::Error::new(x) - )); + error_message_to_c( + error_msg, + format!( + "Wrapper error when opening minidump destination at {:?}: {:#}", + path, + anyhow::Error::new(x) + ), + ); return false; } }; @@ -129,8 +141,22 @@ pub unsafe extern "C" fn write_minidump_linux_with_context( return true; } Err(x) => { - error_msg.assign(&format!("{:#}", anyhow::Error::new(x))); + error_message_to_c(error_msg, format!("{:#}", anyhow::Error::new(x))); return false; } } } + +fn error_message_to_c(c_string_pointer: *mut *mut c_char, error_message: String) { + if c_string_pointer != null_mut() { + let c_error_message = CString::new(error_message).unwrap_or_default(); + unsafe { *c_string_pointer = c_error_message.into_raw() }; + } +} + +// This function will be exposed to C++ +#[no_mangle] +pub unsafe extern "C" fn free_minidump_error_msg(c_string: *mut c_char) { + // Unused because we just need to drop it + let _c_string = unsafe { CString::from_raw(c_string) }; +} diff --git a/toolkit/crashreporter/tools/upload_symbols.py b/toolkit/crashreporter/tools/upload_symbols.py index eff1f43b2b..a5c84980b1 100644 --- a/toolkit/crashreporter/tools/upload_symbols.py +++ b/toolkit/crashreporter/tools/upload_symbols.py @@ -27,6 +27,10 @@ log = logging.getLogger("upload-symbols") log.setLevel(logging.INFO) DEFAULT_URL = "https://symbols.mozilla.org/upload/" +TASKCLUSTER_PROXY_URL = os.environ.get("TASKCLUSTER_PROXY_URL", "http://taskcluster") +TASKCLUSTER_ROOT_URL = os.environ.get( + "TASKCLUSTER_ROOT_URL", "https://firefox-ci-tc.services.mozilla.com" +) MAX_RETRIES = 7 MAX_ZIP_SIZE = 500000000 # 500 MB @@ -43,7 +47,7 @@ def print_error(r): def get_taskcluster_secret(secret_name): - secrets_url = "http://taskcluster/secrets/v1/secret/{}".format(secret_name) + secrets_url = "{}/secrets/v1/secret/{}".format(TASKCLUSTER_PROXY_URL, secret_name) log.info( 'Using symbol upload token from the secrets service: "{}"'.format(secrets_url) ) @@ -55,17 +59,35 @@ def get_taskcluster_secret(secret_name): return auth_token +def get_taskcluster_artifact_urls(task_id): + artifacts_url = "{}/queue/v1/task/{}/artifacts".format( + TASKCLUSTER_PROXY_URL, task_id + ) + res = requests.get(artifacts_url) + res.raise_for_status() + return [ + "{}/api/queue/v1/task/{}/artifacts/{}".format( + TASKCLUSTER_ROOT_URL, task_id, artifact["name"] + ) + for artifact in res.json()["artifacts"] + if artifact["name"].startswith("public/build/target.crashreporter-symbols") + ] + + def main(): logging.basicConfig() parser = argparse.ArgumentParser( description="Upload symbols in ZIP using token from Taskcluster secrets service." ) parser.add_argument( - "archive", help="Symbols archive file - URL or path to local file" + "archive", + help="Symbols archive file - URL or path to local file", + nargs="*", ) parser.add_argument( "--ignore-missing", help="No error on missing files", action="store_true" ) + parser.add_argument("--task-id", help="Taskcluster task id to use symbols from") args = parser.parse_args() def check_file_exists(url): @@ -78,35 +100,45 @@ def main(): log.info("Retrying...") return False - if args.archive.startswith("http"): - is_existing = check_file_exists(args.archive) - else: - is_existing = os.path.isfile(args.archive) + if args.task_id: + args.archive.extend(get_taskcluster_artifact_urls(args.task_id)) - if not is_existing: - if args.ignore_missing: - log.info('Archive file "{0}" does not exist!'.format(args.archive)) - return 0 + for archive in args.archive: + error = False + if archive.startswith("http"): + is_existing = check_file_exists(archive) else: - log.error('Error: archive file "{0}" does not exist!'.format(args.archive)) + is_existing = os.path.isfile(archive) + + if not is_existing: + if args.ignore_missing: + log.info('Archive file "{0}" does not exist!'.format(args.archive)) + else: + log.error( + 'Error: archive file "{0}" does not exist!'.format(args.archive) + ) + error = True + if error: return 1 try: - tmpdir = None - if args.archive.endswith(".tar.zst"): - tmpdir = tempfile.TemporaryDirectory() - zip_paths = convert_zst_archive(args.archive, tmpdir) - else: - zip_paths = [args.archive] - + tmpdir = tempfile.TemporaryDirectory() + zip_paths = convert_zst_archives(args.archive, tmpdir) for zip_path in zip_paths: result = upload_symbols(zip_path) if result: return result return 0 finally: - if tmpdir: - tmpdir.cleanup() + tmpdir.cleanup() + + +def convert_zst_archives(archives, tmpdir): + for archive in archives: + if archive.endswith(".tar.zst"): + yield from convert_zst_archive(archive, tmpdir) + else: + yield archive def convert_zst_archive(zst_archive, tmpdir): |
