Backed out 4 changesets (bug 1496503) for Valgrind bustage. CLOSED TREE

Backed out changeset 033a89b3e00d (bug 1496503)
Backed out changeset a0f255b660ce (bug 1496503)
Backed out changeset 963d8ac1cfee (bug 1496503)
Backed out changeset 43e44f8439ec (bug 1496503)

Cargo.lock

@@ -1035,7 +1035,6 @@ 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.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

@@ -33,7 +33,7 @@ MOZ_BEGIN_EXTERN_C
 extern MFBT_DATA const char* gMozCrashReason;
 MOZ_END_EXTERN_C
 
-#if defined(MOZ_HAS_MOZGLUE) || defined(MOZILLA_INTERNAL_API)
+#if !defined(DEBUG) && (defined(MOZ_HAS_MOZGLUE) || defined(MOZILLA_INTERNAL_API))
 static inline void
 AnnotateMozCrashReason(const char* reason)
 {
@@ -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,91 +151,52 @@ 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) -> !;
-}
+/// Contains the panic message, if set.
+static mut PANIC_REASON: Option<*const str> = None;
 
-/// 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.
+/// 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(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);
+    }));
+}
+
+#[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
+        }
+    }
 }
 
 // Wrap the rust system allocator to override the OOM handler, redirecting

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;