Backed out 4 changesets (bug 1496503) for Valgrind bustage. CLOSED TREE
authorDorel Luca <dluca@mozilla.com>
Wed, 14 Nov 2018 19:00:29 +0200
changeset 446190 38516fcc409fdc8f82ae10fa96fe474e7b4ac251
parent 446189 7aa761d6638c31c7a1db5bdbb846c802d50f0a7d
child 446191 72b0f7554eee5c2174fc4a1be8f8975f830184a6
push id35038
push userrmaries@mozilla.com
push dateWed, 14 Nov 2018 22:12:17 +0000
treeherdermozilla-central@4e1b2b7e0c37 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1496503
milestone65.0a1
backs out033a89b3e00d8210043a4696ab7ad66c2bd88fe3
a0f255b660ce2d49fc7a07abe7d546c51e12da02
963d8ac1cfeed19cc3b18ec991e01eafd4b49f33
43e44f8439eca0caa66b6da8d93d1c0a34792bdb
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 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
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
@@ -28,17 +28,17 @@
  * if present. It is declared here (and defined in Assertions.cpp) to make it
  * available to all code, even libraries that don't link with the crash reporter
  * directly.
  */
 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)
 {
   gMozCrashReason = reason;
 }
 #  define MOZ_CRASH_ANNOTATE(...) AnnotateMozCrashReason(__VA_ARGS__)
 #else
 #  define MOZ_CRASH_ANNOTATE(...) do { /* nothing */ } while (false)
@@ -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)) {