Backed out 4 changesets (bug 1496503) for xpcshell failures at toolki…

…t/crashreporter/test/unit/test_crash_rust_panic.js on a CLOSED TREE

Backed out changeset cfeee3d5ed6a (bug 1496503)
Backed out changeset 164a5a49fd25 (bug 1496503)
Backed out changeset d0b6c1fc149d (bug 1496503)
Backed out changeset bfb4ee856c71 (bug 1496503)

mfbt/Assertions.cpp

@@ -18,6 +18,21 @@ MOZ_BEGIN_EXTERN_C
  */
 MFBT_DATA const char* gMozCrashReason = nullptr;
 
+#ifndef DEBUG
+MFBT_API MOZ_COLD MOZ_NORETURN MOZ_NEVER_INLINE void
+MOZ_CrashOOL(int aLine, const char* aReason)
+#else
+MFBT_API MOZ_COLD MOZ_NORETURN MOZ_NEVER_INLINE void
+MOZ_CrashOOL(const char* aFilename, int aLine, const char* aReason)
+#endif
+{
+#ifdef DEBUG
+  MOZ_ReportCrash(aReason, aFilename, aLine);
+#endif
+  gMozCrashReason = aReason;
+  MOZ_REALLY_CRASH(aLine);
+}
+
 static char sPrintfCrashReason[sPrintfCrashReasonSize] = {};
 
 // Accesses to this atomic are not included in web replay recordings, so that
@@ -47,10 +62,10 @@ MOZ_CrashPrintf(const char* aFilename, int aLine, const char* aFormat, ...)
   MOZ_RELEASE_ASSERT(ret >= 0 && size_t(ret) < sPrintfCrashReasonSize,
     "Could not write the explanation string to the supplied buffer!");
 #ifdef DEBUG
-  MOZ_CrashOOL(aFilename, aLine, sPrintfCrashReason);
-#else
-  MOZ_CrashOOL(nullptr, aLine, sPrintfCrashReason);
+  MOZ_ReportCrash(sPrintfCrashReason, aFilename, aLine);
 #endif
+  gMozCrashReason = sPrintfCrashReason;
+  MOZ_REALLY_CRASH(aLine);
 }
 
 MOZ_END_EXTERN_C

mfbt/Assertions.h

@@ -301,16 +301,15 @@ MOZ_NoReturn(int aLine)
  * to crash-stats and are publicly visible. Firefox data stewards must do data
  * review on usages of this macro.
  */
-static inline MOZ_COLD MOZ_NORETURN void
-MOZ_CrashOOL(const char* aFilename, int aLine, const char* aReason)
-{
-#ifdef DEBUG
-  MOZ_ReportCrash(aReason, aFilename, aLine);
+#ifndef DEBUG
+MFBT_API MOZ_COLD MOZ_NORETURN MOZ_NEVER_INLINE void
+MOZ_CrashOOL(int aLine, const char* aReason);
+#  define MOZ_CRASH_UNSAFE_OOL(reason) MOZ_CrashOOL(__LINE__, reason)
+#else
+MFBT_API MOZ_COLD MOZ_NORETURN MOZ_NEVER_INLINE void
+MOZ_CrashOOL(const char* aFilename, int aLine, const char* aReason);
+#  define MOZ_CRASH_UNSAFE_OOL(reason) MOZ_CrashOOL(__FILE__, __LINE__, reason)
 #endif
-  MOZ_CRASH_ANNOTATE(aReason);
-  MOZ_REALLY_CRASH(aLine);
-}
-#define MOZ_CRASH_UNSAFE_OOL(reason) MOZ_CrashOOL(__FILE__, __LINE__, reason)
 
 static const size_t sPrintfMaxArgs = 4;
 static const size_t sPrintfCrashReasonSize = 1024;

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -120,7 +120,9 @@ using mozilla::ipc::CrashReporterClient;
 
 // From toolkit/library/rust/shared/lib.rs
 extern "C" {
+  void install_rust_panic_hook();
   void install_rust_oom_hook();
+  bool get_rust_panic_reason(char** reason, size_t* length);
 }
 
 
@@ -906,8 +908,12 @@ 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 (gMozCrashReason != nullptr) {
+  if (rust_panic) {
+    reason = rust_panic_reason;
+  } else if (gMozCrashReason != nullptr) {
     reason = gMozCrashReason;
     len = strlen(reason);
   } else {
@@ -1736,6 +1742,7 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory,
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
   install_rust_oom_hook();
 
   InitThreadAnnotation();
@@ -3600,6 +3607,8 @@ SetRemoteExceptionHandler(const nsACString& crashPipe,
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
+
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
@@ -3646,6 +3655,8 @@ SetRemoteExceptionHandler()
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
+
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
@@ -3674,6 +3685,8 @@ SetRemoteExceptionHandler(const nsACString& crashPipe)
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
+
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }

toolkit/library/rust/shared/Cargo.toml

@@ -30,7 +30,6 @@ 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,21 +36,17 @@ 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);
@@ -155,93 +151,54 @@ pub extern "C" fn intentional_panic(message: *const c_char) {
     panic!("{}", unsafe { CStr::from_ptr(message) }.to_string_lossy());
 }
 
-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) -> !;
-}
-
-/// 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;
+/// 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!");
         }
-        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
-    }
+        // Fall through to the default hook so we still print the reason and
+        // backtrace to the console.
+        default_hook(info);
+    }));
 }
 
-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);
+#[no_mangle]
+pub extern "C" fn get_rust_panic_reason(reason: *mut *const c_char, length: *mut usize) -> bool {
     unsafe {
-        GeckoCrashOOL(filename.as_ptr() as *const c_char, line as c_int,
-                      message.as_ptr() as *const c_char);
+        if let Some(s) = PANIC_REASON {
+            *reason = s as *const c_char;
+            *length = (*s).len();
+            true
+        } else {
+            false
+        }
     }
 }
 
-/// 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,7 +8,6 @@
 #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"
@@ -5332,23 +5331,6 @@ 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);
-}
-
-// From toolkit/library/rust/shared/lib.rs
-extern "C" void install_rust_panic_hook();
-
-struct InstallRustPanicHook {
-  InstallRustPanicHook() {
-    install_rust_panic_hook();
-  }
-};
-
-InstallRustPanicHook sInstallRustPanicHook;
-
 #ifdef MOZ_ASAN_REPORTER
 void setASanReporterPath(nsIFile* aDir) {
   nsCOMPtr<nsIFile> dir;