-
- Downloads
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 Differential Revision: https://phabricator.services.mozilla.com/D11720 --HG-- extra : moz-landing-system : lando
Showing
- Cargo.lock 1 addition, 0 deletionsCargo.lock
- toolkit/crashreporter/nsExceptionHandler.cpp 1 addition, 6 deletionstoolkit/crashreporter/nsExceptionHandler.cpp
- toolkit/library/rust/shared/Cargo.toml 1 addition, 0 deletionstoolkit/library/rust/shared/Cargo.toml
- toolkit/library/rust/shared/lib.rs 85 additions, 42 deletionstoolkit/library/rust/shared/lib.rs
- toolkit/xre/nsAppRunner.cpp 7 additions, 0 deletionstoolkit/xre/nsAppRunner.cpp
Loading
Please register or sign in to comment