bug 1275780 - capture Rust panic message in crash reports. r=froydnj
☠☠ backed out by a45d5f56491c ☠ ☠
authorTed Mielczarek <ted@mielczarek.org>
Mon, 27 Mar 2017 14:40:22 -0400
changeset 553924 23c5ecf4d92e3b2039ffbe6a9f73f56d25b57c24
parent 553923 40eb77a98bcfbf70cffac94581b0d4ad869b3ded
child 553925 23e4fd04033bb8afe7bf3e63aa5053c9c094b62f
push id51823
push userbmo:tchiovoloni@mozilla.com
push dateFri, 31 Mar 2017 00:17:03 +0000
reviewersfroydnj
bugs1275780
milestone55.0a1
bug 1275780 - capture Rust panic message in crash reports. r=froydnj MozReview-Commit-ID: IUlYqPEtkgg
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/test/unit/test_crash_rust_panic.js
toolkit/library/rust/shared/lib.rs
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -111,16 +111,23 @@ using google_breakpad::MinidumpDescripto
 #if defined(MOZ_WIDGET_ANDROID)
 using google_breakpad::auto_wasteful_vector;
 using google_breakpad::FileID;
 using google_breakpad::PageAllocator;
 #endif
 using namespace mozilla;
 using mozilla::ipc::CrashReporterClient;
 
+// From toolkit/library/rust/shared/lib.rs
+extern "C" {
+  void install_rust_panic_hook();
+  bool get_rust_panic_reason(char** reason, size_t* length);
+}
+
+
 namespace CrashReporter {
 
 #ifdef XP_WIN32
 typedef wchar_t XP_CHAR;
 typedef std::wstring xpstring;
 #define XP_TEXT(x) L##x
 #define CONVERT_XP_CHAR_TO_UTF16(x) x
 #define XP_STRLEN(x) wcslen(x)
@@ -1126,17 +1133,27 @@ bool MinidumpCallback(
     if (apiData.Valid()) {
       DllBlocklist_WriteNotes(apiData.Handle());
       DllBlocklist_WriteNotes(eventFile.Handle());
     }
 #endif
     WriteGlobalMemoryStatus(&apiData, &eventFile);
 #endif // XP_WIN
 
-    if (gMozCrashReason) {
+    char* rust_panic_reason;
+    size_t rust_panic_len;
+    if (get_rust_panic_reason(&rust_panic_reason, &rust_panic_len)) {
+      // rust_panic_reason is not null-terminated.
+      WriteLiteral(apiData, "MozCrashReason=");
+      apiData.WriteBuffer(rust_panic_reason, rust_panic_len);
+      WriteLiteral(apiData, "\n");
+      WriteLiteral(eventFile, "MozCrashReason=");
+      eventFile.WriteBuffer(rust_panic_reason, rust_panic_len);
+      WriteLiteral(eventFile, "\n");
+    } else if (gMozCrashReason) {
       WriteAnnotation(apiData, "MozCrashReason", gMozCrashReason);
       WriteAnnotation(eventFile, "MozCrashReason", gMozCrashReason);
     }
 
     if (oomAllocationSizeBuffer[0]) {
       WriteAnnotation(apiData, "OOMAllocationSize", oomAllocationSizeBuffer);
       WriteAnnotation(eventFile, "OOMAllocationSize", oomAllocationSizeBuffer);
     }
@@ -1572,16 +1589,18 @@ LocateExecutable(nsIFile* aXREDirectory,
 #endif // !defined(MOZ_WIDGET_ANDROID)
 
 nsresult SetExceptionHandler(nsIFile* aXREDirectory,
                              bool force/*=false*/)
 {
   if (gExceptionHandler)
     return NS_ERROR_ALREADY_INITIALIZED;
 
+  install_rust_panic_hook();
+
 #if !defined(DEBUG) || defined(MOZ_WIDGET_GONK)
   // In non-debug builds, enable the crash reporter by default, and allow
   // disabling it with the MOZ_CRASHREPORTER_DISABLE environment variable.
   // Also enable it by default in debug gonk builds as it is difficult to
   // set environment on startup.
   const char *envvar = PR_GetEnv("MOZ_CRASHREPORTER_DISABLE");
   if (envvar && *envvar && !force)
     return NS_OK;
--- a/toolkit/crashreporter/test/unit/test_crash_rust_panic.js
+++ b/toolkit/crashreporter/test/unit/test_crash_rust_panic.js
@@ -1,11 +1,11 @@
 function run_test() {
   // Try crashing with a Rust panic
   do_crash(function() {
              Components.classes["@mozilla.org/xpcom/debug;1"].getService(Components.interfaces.nsIDebug2).rustPanic("OH NO");
            },
            function(mdump, extra) {
-             //TODO: check some extra things?
+             do_check_eq(extra.MozCrashReason, "OH NO");
            },
           // process will exit with a zero exit status
           true);
 }
--- a/toolkit/library/rust/shared/lib.rs
+++ b/toolkit/library/rust/shared/lib.rs
@@ -6,16 +6,63 @@
 extern crate geckoservo;
 
 extern crate mp4parse_capi;
 extern crate nsstring;
 extern crate rust_url_capi;
 #[cfg(feature = "quantum_render")]
 extern crate webrender_bindings;
 
+use std::boxed::Box;
 use std::ffi::CStr;
 use std::os::raw::c_char;
+use std::panic;
 
 /// Used to implement `nsIDebug2::RustPanic` for testing purposes.
 #[no_mangle]
 pub extern "C" fn intentional_panic(message: *const c_char) {
     panic!("{}", unsafe { CStr::from_ptr(message) }.to_string_lossy());
 }
+
+/// Contains the panic message, if set.
+static mut PANIC_REASON: Option<(*const str, usize)> = None;
+
+/// Configure a panic hook to capture panic messages for crash reports.
+///
+/// We don't store this in `gMozCrashReason` because:
+/// a) Rust strings aren't null-terminated, so we'd have to allocate
+///    memory to get a null-terminated string
+/// b) The panic=abort handler is going to call `abort()` on non-Windows,
+///    which is `mozalloc_abort` for us, which will use `MOZ_CRASH` and
+///    overwrite `gMozCrashReason` with an unhelpful string.
+#[no_mangle]
+pub extern "C" fn install_rust_panic_hook() {
+    panic::set_hook(Box::new(|info| {
+        // Try to handle &str/String payloads, which should handle 99% of cases.
+        let payload = info.payload();
+        // We'll hold a raw *const str here, but it will be OK because
+        // Rust is going to abort the process before the payload could be
+        // deallocated.
+        if let Some(s) = payload.downcast_ref::<&str>() {
+            unsafe { PANIC_REASON = Some((*s as *const str, s.len())) }
+        } else if let Some(s) = payload.downcast_ref::<String>() {
+            unsafe { PANIC_REASON = Some((s.as_str() as *const str, s.len())) }
+        } else {
+            // Not the most helpful thing, but seems unlikely to happen
+            // in practice.
+            println!("Unhandled panic payload!");
+        }
+    }));
+}
+
+#[no_mangle]
+pub extern "C" fn get_rust_panic_reason(reason: *mut *const c_char, length: *mut usize) -> bool {
+    unsafe {
+        match PANIC_REASON {
+            Some((s, len)) => {
+                *reason = s as *const c_char;
+                *length = len;
+                true
+            }
+            None => false,
+        }
+    }
+}