Bug 1496503 - Change the rust panic hook to delegate to Gecko's crash code. r=froydnj

The current rust panic hook keeps a string for the crash reporter, and
goes on calling the default rust panic hook, which prints out a crash
stack...  when RUST_BOOTSTRAP is set *and* when that works. Notably, on
both mac and Windows, it only really works for local builds, but fails
for debug builds from automation, although on automation itself, we also
do stackwalk from crash minidumps, which alleviates the problem.
Artifact debug builds are affected, though.

More importantly, C++ calls to e.g. MOZ_CRASH have a similar but
different behavior, in that they dump a stack trace on debug builds, by
default (with exceptions, see below for one). The format of those stack
traces is understood by the various fix*stack*py scripts under
tools/rb/, that are used by the various test harnesses both on
automation and locally.

Additionally, the current rust panic hook, as it calls the default rust
panic hook, ends up calling abort() on non-Windows platforms, which ends
up being verbosely redirected to mozalloc_abort per
https://dxr.mozilla.org/mozilla-central/rev/237e4c0633fda8e227b2ab3ab57e417c980a2811/memory/mozalloc/mozalloc_abort.cpp#79
which then calls MOZ_CRASH. Theoretically, /that/ would also print a
stack trace, but doesn't because currently the stack trace printing code
lives in libxul, and MOZ_CRASH only calls it when compiled from
libxul-code, which mozalloc_abort is not part of.

With this change, we make the rust panic handler call back into
MOZ_CRASH directly. This has multiple advantages:
- This is more consistent cross-platforms (Windows is not special
anymore).
- This is more consistent between C++ and rust (stack traces all look
the same, and can all be post-processed by fix*stack*py if need be)
- This is more consistent in behavior, where debug builds will show
those stack traces without caring about environment variables.
- It demangles C++ symbols in rust-initiated stack traces (for some
reason that didn't happen with the rust panic handler)

A few downsides:
- the loss of demangling for some rust symbols.
- the loss of addresses in the stacks, although they're not entirely
useful
- extra empty lines.

The first should be fixable later one. The latter two are arguably
something that should be consistent across C++ and rust, and should be
changed if necessary, independently of this patch.

Depends on D11719

Depends on D11719

Differential Revision: https://phabricator.services.mozilla.com/D11720

--HG--
extra : moz-landing-system : lando

Cargo.lock

@@ -1035,6 +1035,7 @@ dependencies = [
 name = "gkrust-shared"
 version = "0.1.0"
 dependencies = [
+ "arrayvec 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "audioipc-client 0.4.0",
  "audioipc-server 0.2.3",
  "cose-c 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",

mfbt/Assertions.h

@@ -33,7 +33,7 @@ MOZ_BEGIN_EXTERN_C
 extern MFBT_DATA const char* gMozCrashReason;
 MOZ_END_EXTERN_C
 
-#if !defined(DEBUG) && (defined(MOZ_HAS_MOZGLUE) || defined(MOZILLA_INTERNAL_API))
+#if defined(MOZ_HAS_MOZGLUE) || defined(MOZILLA_INTERNAL_API)
 static inline void
 AnnotateMozCrashReason(const char* reason)
 {

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -122,7 +122,6 @@ using mozilla::ipc::CrashReporterClient;
 extern "C" {
   void install_rust_panic_hook();
   void install_rust_oom_hook();
-  bool get_rust_panic_reason(char** reason, size_t* length);
 }
 
 
@@ -908,12 +907,8 @@ WriteEscapedMozCrashReason(PlatformWriter& aWriter)
 {
   const char *reason;
   size_t len;
-  char *rust_panic_reason;
-  bool rust_panic = get_rust_panic_reason(&rust_panic_reason, &len);
 
-  if (rust_panic) {
-    reason = rust_panic_reason;
-  } else if (gMozCrashReason != nullptr) {
+  if (gMozCrashReason != nullptr) {
     reason = gMozCrashReason;
     len = strlen(reason);
   } else {

toolkit/library/rust/shared/Cargo.toml

@@ -30,6 +30,7 @@ env_logger = {version = "0.5", default-features = false} # disable `regex` to re
 cose-c = { version = "0.1.5" }
 rkv = "0.5"
 jsrust_shared = { path = "../../../../js/src/rust/shared", optional = true }
+arrayvec = "0.4"
 
 [build-dependencies]
 # Use exactly version 0.2.1, which uses semver 0.6, which other crates

toolkit/library/rust/shared/lib.rs

@@ -36,17 +36,21 @@ extern crate rsdparsa_capi;
 #[cfg(feature = "spidermonkey_rust")]
 extern crate jsrust_shared;
 
+extern crate arrayvec;
+
 use std::boxed::Box;
 use std::env;
 use std::ffi::{CStr, CString};
 use std::os::raw::c_char;
-#[cfg(target_os = "android")]
 use std::os::raw::c_int;
 #[cfg(target_os = "android")]
 use log::Level;
 #[cfg(not(target_os = "android"))]
 use log::Log;
+use std::cmp;
 use std::panic;
+use std::ops::Deref;
+use arrayvec::{Array, ArrayString};
 
 extern "C" {
     fn gfx_critical_note(msg: *const c_char);
@@ -151,54 +155,93 @@ 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> = 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() {
-    let default_hook = panic::take_hook();
-    panic::set_hook(Box::new(move |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); }
-        } else if let Some(s) = payload.downcast_ref::<String>() {
-            unsafe { PANIC_REASON = Some(s.as_str() as *const str); }
-        } else {
-            // Not the most helpful thing, but seems unlikely to happen
-            // in practice.
-            println!("Unhandled panic payload!");
-        }
-        // Fall through to the default hook so we still print the reason and
-        // backtrace to the console.
-        default_hook(info);
-    }));
+extern "C" {
+    // We can't use MOZ_CrashOOL directly because it may be weakly linked
+    // to libxul, and rust can't handle that.
+    fn GeckoCrashOOL(filename: *const c_char, line: c_int, reason: *const c_char) -> !;
 }
 
-#[no_mangle]
-pub extern "C" fn get_rust_panic_reason(reason: *mut *const c_char, length: *mut usize) -> bool {
-    unsafe {
-        if let Some(s) = PANIC_REASON {
-            *reason = s as *const c_char;
-            *length = (*s).len();
-            true
-        } else {
-            false
+/// Truncate a string at the closest unicode character boundary
+/// ```
+/// assert_eq!(str_truncate_valid("éà", 3), "é");
+/// assert_eq!(str_truncate_valid("éà", 4), "éè");
+/// ```
+fn str_truncate_valid(s: &str, mut mid: usize) -> &str {
+    loop {
+        if let Some(res) = s.get(..mid) {
+            return res;
         }
+        mid -= 1;
     }
 }
 
+/// Similar to ArrayString, but with terminating nul character.
+#[derive(Debug, PartialEq)]
+struct ArrayCString<A: Array<Item = u8>> {
+    inner: ArrayString<A>,
+}
+
+impl<S: AsRef<str>, A: Array<Item = u8>> From<S> for ArrayCString<A> {
+    /// Contrary to ArrayString::from, truncates at the closest unicode
+    /// character boundary.
+    /// ```
+    /// assert_eq!(ArrayCString::<[_; 4]>::from("éà"),
+    ///            ArrayCString::<[_; 4]>::from("é"));
+    /// assert_eq!(&*ArrayCString::<[_; 4]>::from("éà"), "é\0");
+    /// ```
+    fn from(s: S) -> Self {
+        let s = s.as_ref();
+        let len = cmp::min(s.len(), A::capacity() - 1);
+        let mut result = Self {
+            inner: ArrayString::from(str_truncate_valid(s, len)).unwrap(),
+        };
+        result.inner.push('\0');
+        result
+    }
+}
+
+impl<A: Array<Item = u8>> Deref for ArrayCString<A> {
+    type Target = str;
+
+    fn deref(&self) -> &str {
+        self.inner.as_str()
+    }
+}
+
+fn panic_hook(info: &panic::PanicInfo) {
+    // Try to handle &str/String payloads, which should handle 99% of cases.
+    let payload = info.payload();
+    let message = if let Some(s) = payload.downcast_ref::<&str>() {
+        s
+    } else if let Some(s) = payload.downcast_ref::<String>() {
+        s.as_str()
+    } else {
+        // Not the most helpful thing, but seems unlikely to happen
+        // in practice.
+        "Unhandled rust panic payload!"
+    };
+    let (filename, line) = if let Some(loc) = info.location() {
+        (loc.file(), loc.line())
+    } else {
+        ("unknown.rs", 0)
+    };
+    // Copy the message and filename to the stack in order to safely add
+    // a terminating nul character (since rust strings don't come with one
+    // and GeckoCrashOOL wants one).
+    let message = ArrayCString::<[_; 512]>::from(message);
+    let filename = ArrayCString::<[_; 512]>::from(filename);
+    unsafe {
+        GeckoCrashOOL(filename.as_ptr() as *const c_char, line as c_int,
+                      message.as_ptr() as *const c_char);
+    }
+}
+
+/// Configure a panic hook to redirect rust panics to Gecko's MOZ_CrashOOL.
+#[no_mangle]
+pub extern "C" fn install_rust_panic_hook() {
+    panic::set_hook(Box::new(panic_hook));
+}
+
 // Wrap the rust system allocator to override the OOM handler, redirecting
 // to Gecko's, which interacts with the crash reporter.
 // This relies on unstable APIs that have not changed between 1.24 and 1.27.

toolkit/xre/nsAppRunner.cpp

@@ -8,6 +8,7 @@
 #include "mozilla/ipc/GeckoChildProcessHost.h"
 
 #include "mozilla/ArrayUtils.h"
+#include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/FilePreferences.h"
 #include "mozilla/ChaosMode.h"
@@ -5343,6 +5344,12 @@ GeckoHandleOOM(size_t size) {
   mozalloc_handle_oom(size);
 }
 
+// Similarly, this wraps MOZ_CrashOOL
+extern "C" void
+GeckoCrashOOL(const char* aFilename, int aLine, const char* aReason) {
+  MOZ_CrashOOL(aFilename, aLine, aReason);
+}
+
 #ifdef MOZ_ASAN_REPORTER
 void setASanReporterPath(nsIFile* aDir) {
   nsCOMPtr<nsIFile> dir;