Backed out 4 changesets (bug 1496503) for xpcshell failures at toolkit/crashreporter/test/unit/test_crash_rust_panic.js on a CLOSED TREE
authorCoroiu Cristina <ccoroiu@mozilla.com>
Wed, 14 Nov 2018 09:00:06 +0200
changeset 505349 d8a262837cd3956ec6a6aa3082d0ff0e7352c79c
parent 505348 eed2a2c3c3d25e10d7273b76d45d76fd3e3c94e3
child 505374 073045259e75e0c8f7b8ffcd5e4bf72570f98f3e
child 505377 236f09f4709d78fe7f78afd3d65ee463b42378bc
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1496503
milestone65.0a1
backs outcfeee3d5ed6a215072a17a037c7a707bd03b909e
164a5a49fd25480b268a592c44c44bd0589208dc
d0b6c1fc149d58e8f3b177b28a3670046089f097
bfb4ee856c7115899960e3bd1fcf72b5e764314d
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 4 changesets (bug 1496503) for xpcshell failures at toolkit/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)
Cargo.lock
mfbt/Assertions.cpp
mfbt/Assertions.h
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/library/rust/shared/Cargo.toml
toolkit/library/rust/shared/lib.rs
toolkit/xre/nsAppRunner.cpp
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1030,17 +1030,16 @@ dependencies = [
  "nsstring-gtest 0.1.0",
  "xpcom-gtest 0.1.0",
 ]
 
 [[package]]
 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)",
  "cubeb-pulse 0.2.0",
  "cubeb-sys 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "encoding_c 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "encoding_glue 0.1.0",
  "env_logger 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)",
--- a/mfbt/Assertions.cpp
+++ b/mfbt/Assertions.cpp
@@ -13,16 +13,31 @@ MOZ_BEGIN_EXTERN_C
 /*
  * The crash reason is defined as a global variable here rather than in the
  * crash reporter itself to make it available to all code, even libraries like
  * JS that don't link with the crash reporter directly. This value will only
  * be consumed if the crash reporter is used by the target application.
  */
 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
 // if we crash in an area where recorded events are not allowed the true reason
 // for the crash is not obscured by a record/replay error.
 static mozilla::Atomic<bool, mozilla::SequentiallyConsistent,
                        mozilla::recordreplay::Behavior::DontPreserve> sCrashing(false);
 
@@ -42,15 +57,15 @@ MOZ_CrashPrintf(const char* aFilename, i
   va_list aArgs;
   va_start(aArgs, aFormat);
   int ret = vsnprintf(sPrintfCrashReason, sPrintfCrashReasonSize,
                       aFormat, aArgs);
   va_end(aArgs);
   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
--- a/mfbt/Assertions.h
+++ b/mfbt/Assertions.h
@@ -296,26 +296,25 @@ MOZ_NoReturn(int aLine)
  * arbitrary strings from a potentially compromised process is not without risk.
  * If the string being passed is the result of a printf-style function,
  * consider using MOZ_CRASH_UNSAFE_PRINTF instead.
  *
  * @note This macro causes data collection because crash strings are annotated
  * 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;
 
 #ifndef DEBUG
 MFBT_API MOZ_COLD MOZ_NORETURN MOZ_NEVER_INLINE MOZ_FORMAT_PRINTF(2, 3) void
 MOZ_CrashPrintf(int aLine, const char* aFormat, ...);
 #  define MOZ_CALL_CRASH_PRINTF(format, ...) \
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -115,17 +115,19 @@ using google_breakpad::FileID;
 using google_breakpad::kDefaultBuildIdSize;
 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();
   void install_rust_oom_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;
@@ -901,18 +903,22 @@ LaunchCrashHandlerService(XP_CHAR* aProg
 
 #endif
 
 void
 WriteEscapedMozCrashReason(PlatformWriter& aWriter)
 {
   const char *reason;
   size_t len;
-
-  if (gMozCrashReason != nullptr) {
+  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) {
     reason = gMozCrashReason;
     len = strlen(reason);
   } else {
     return; // No crash reason, bail out
   }
 
   WriteString(aWriter, AnnotationToString(Annotation::MozCrashReason));
   WriteLiteral(aWriter, "=");
@@ -1731,16 +1737,17 @@ nsresult SetExceptionHandler(nsIFile* aX
 #if defined(MOZ_WIDGET_ANDROID)
   AddAndroidMappingInfo();
 #endif // defined(MOZ_WIDGET_ANDROID)
 
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
   install_rust_oom_hook();
 
   InitThreadAnnotation();
 
   return NS_OK;
 }
 
 bool GetEnabled()
@@ -3595,16 +3602,18 @@ SetRemoteExceptionHandler(const nsACStri
 #ifdef _WIN64
   SetJitExceptionHandler();
 #endif
 
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
+
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
 
 //--------------------------------------------------
 #elif defined(XP_LINUX)
 
 // Parent-side API for children
@@ -3641,16 +3650,18 @@ SetRemoteExceptionHandler()
                      nullptr,    // no callback context
                      true,       // install signal handlers
                      gMagicChildCrashReportFd);
 
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
+
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
 
 //--------------------------------------------------
 #elif defined(XP_MACOSX)
 // Child-side API
 bool
@@ -3669,16 +3680,18 @@ SetRemoteExceptionHandler(const nsACStri
                      nullptr,    // no callback context
                      true,       // install signal handlers
                      crashPipe.BeginReading());
 
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
+  install_rust_panic_hook();
+
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
 #endif  // XP_WIN
 
 
 bool
 TakeMinidumpForChild(uint32_t childPid, nsIFile** dump, uint32_t* aSequence)
--- a/toolkit/library/rust/shared/Cargo.toml
+++ b/toolkit/library/rust/shared/Cargo.toml
@@ -25,17 +25,16 @@ audioipc-server = { path = "../../../../
 u2fhid = { path = "../../../../dom/webauthn/u2f-hid-rs" }
 rsdparsa_capi = { path = "../../../../media/webrtc/signaling/src/sdp/rsdparsa_capi" }
 # We have these to enforce common feature sets for said crates.
 log = {version = "0.4", features = ["release_max_level_info"]}
 env_logger = {version = "0.5", default-features = false} # disable `regex` to reduce code size
 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
 # require (0.2.2 requires 0.9)
 rustc_version = "=0.2.1"
 
 [features]
 default = []
--- a/toolkit/library/rust/shared/lib.rs
+++ b/toolkit/library/rust/shared/lib.rs
@@ -31,31 +31,27 @@ extern crate audioipc_server;
 extern crate env_logger;
 extern crate u2fhid;
 extern crate log;
 extern crate cosec;
 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);
     #[cfg(target_os = "android")]
     fn __android_log_write(prio: c_int, tag: *const c_char, text: *const c_char) -> c_int;
 }
 
 struct GeckoLogger {
@@ -150,103 +146,64 @@ pub extern "C" fn GkRust_Shutdown() {
 }
 
 /// 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());
 }
 
-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;
+/// 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>,
+        // Fall through to the default hook so we still print the reason and
+        // backtrace to the console.
+        default_hook(info);
+    }));
 }
 
-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()
+#[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
+        }
     }
 }
 
-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.
 // In 1.27, the API changed, so we'll need to adapt to those changes before
 // we can ship with 1.27. As of writing, there might still be further changes
 // to those APIs before 1.27 is released, so we wait for those.
 #[cfg(feature = "oom_with_global_alloc")]
 mod global_alloc {
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -3,17 +3,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/ContentChild.h"
 #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"
 #include "mozilla/CmdLineAndEnvUtils.h"
 #include "mozilla/IOInterposer.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MemoryChecking.h"
 #include "mozilla/Poison.h"
@@ -5327,33 +5326,16 @@ XRE_EnableSameExecutableForContentProc()
 
 // Because rust doesn't handle weak symbols, this function wraps the weak
 // malloc_handle_oom for it.
 extern "C" void
 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;
   aDir->Clone(getter_AddRefs(dir));
 
   dir->Append(NS_LITERAL_STRING("asan"));
   nsresult rv = dir->Create(nsIFile::DIRECTORY_TYPE, 0700);
   if (NS_WARN_IF(NS_FAILED(rv) && rv != NS_ERROR_FILE_ALREADY_EXISTS)) {