Crash reporting overhaul: Difference between revisions

Minor revisions and link to the crash ping lifecycle.
(Added information about the minidump writers)
(Minor revisions and link to the crash ping lifecycle.)
 
(15 intermediate revisions by 2 users not shown)
Line 8: Line 8:


== Exception handlers ==
== Exception handlers ==
Status: not started<br>
Developer(s): gsvelto<br>
Source code:<br>
* https://github.com/EmbarkStudios/crash-handling
Original source code:<br>
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/breakpad-client
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/profile/nsProfileLock.cpp
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/xre/nsSigHandlers.cpp
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/js/src/ds/MemoryProtectionExceptionHandler.cpp
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/js/src/wasm/WasmSignalHandlers.cpp
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/mozglue/android/APKOpen.cpp
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/mozglue/linker/ElfLoader.cpp
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/accessible/windows/msaa/Compatibility.cpp
Bugs:<br>
* {{bug|1620989}} - Rewrite the Linux exception-handler in Rust
* {{bug|1620990}} - Rewrite the Windows exception-handler in Rust
* {{bug|1620991}} - Rewrite the macOS exception-handler in Rust
=== Description ===
Exception handlers and signal handlers are used to intercept abnormal conditions
conditions within Firefox and respond to them. Depending on the affected code
and the type of exception the handlers will either ignore it, conditionally
process it or commence crash reporting in case of fatal ones.
=== Rationale ===
We have several exception and signal handlers scattered through the code. The
main one is provided by Breakpad and is used to catch fatal exceptions. In
addition to it the JIT sets several handlers to process benign ones and we have
a few more ranging from swallowing exceptions in certain processes to detecting
and working around Windows bugs. This proliferation of handlers causes has
several downsides:
* There are no abstractions whatsoever. Every piece of code that needs to install a handler does so by using low-level platform-specific code (sometimes doing bare syscalls!)
* Several handlers must be called in a specific sequence in order to work, with every step deciding if the exception should go further or not. On Windows this happens naturally as exception handling is structured, however the order is implicit and depends on the startup sequence. On macOS where the handling is non-hierarchical not only we have an implicit ordering but it relies on tricks to set mach message handlers at different levels (process VS threads) in order for them to be called in the desired sequence. Finally on Linux/Android there is no in-built ordering as only one signal handler can be active at the same time, this means that every handler has to take care of others that were installed before it and manually forward signals when necessary.
* The combination of the lack of abstraction and implicit ordering means that their use is brittle. Coders are wary of touching them and sometimes scenarios like early crashes yield unpredictable results due to not all the handlers being in place yet.
* Some handlers are covered by tests but not all of them nor is the sequence in which some must be called.
=== Plan ===
We should rewrite the exception handlers starting with a crate that would
provide proper abstractions and explicit ordering. The code relying on existing
handlers would then need to be updated to use the crate instead, registering a
callback, the conditions for it to be called and the order in which it needs to
appear with regards to the other callbacks. The overall goal is to remove
platform-specific code from the existing handler and shrink them down to just
their core functionality, moving all the platform code into the crate. With the
ordering explicitly set we'd also remove all sorts of ambiguity. Once all the
handlers are migrated we should hook functions used to install handlers (such
as signal(), sigaction(), SetUnhandledExceptionHandler(),
task_set_exception_ports(), etc...) to prevent library code from injecting
handlers under our nose. The hooks will either disregard the handlers or
insert them in the right places in the hierarchy on a case-by-case basis.


== Minidump writers ==
== Minidump writers ==
Line 16: Line 70:
* https://github.com/rust-minidump/minidump-writer
* https://github.com/rust-minidump/minidump-writer
Original source code:<br>
Original source code:<br>
* https://searchfox.org/mozilla-central/source/toolkit/crashreporter/breakpad-client/
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/breakpad-client
Bugs:<br>
Bugs:<br>
* {{bug|1620998}}
* {{bug|1620993}} - Rewrite the Linux-specific minidump writer code in Rust
* {{bug|1689358}} - Add ARM/AArch64 support to the oxidized minidump rust writer
* {{bug|1620995}} - Rewrite the macOS-specific minidump writer code in Rust
* {{bug|1620994}} - Rewrite the Windows-specific minidump writer code in Rust


=== Description ===
=== Description ===
Line 55: Line 112:
== Crash monitor ==
== Crash monitor ==


Status: not started<br>
Status: in progress<br>
Developer(s): gsvelto<br>
Developer(s): gsvelto<br>
Source code:<br>
Source code:<br>
Original source code: N/A<br>
Original source code: N/A<br>
Bugs:<br>
Bugs:<br>
* {{bug|1620998}}
* {{bug|1620998}} - Write a crash monitor program to handle annotations and minidump writing


=== Description ===
=== Description ===
Line 108: Line 165:
== Crash reporter client ==
== Crash reporter client ==


Status: not started<br>
Status: completed<br>
Developer(s):<br>
Developer(s): afranchuk<br>
Source code:<br>
Source code: https://searchfox.org/mozilla-central/source/toolkit/crashreporter/client
Original source code:<br>
Original source code:<br>
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/client
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/client
Bugs:<br>
Bugs:<br>
* {{bug|1759175}}
* {{bug|1759175}} - Rewrite the crash reporter client in Rust


=== Description ===
=== Description ===
Line 141: Line 198:
* Localization is done via an INI file and cannot use Fluent
* Localization is done via an INI file and cannot use Fluent
* Given the hard-coded nature of the UI I don't know how it behave with RTL language, probably very poorly
* Given the hard-coded nature of the UI I don't know how it behave with RTL language, probably very poorly
* It does not support Glean-based telemetry


=== Plan ===
=== Plan ===
Line 151: Line 209:
* We should leverage Rust's asynchronous facility to make processing non-blocking and the UI responsive
* We should leverage Rust's asynchronous facility to make processing non-blocking and the UI responsive
* We should add platform-independent tests for the parts of the code that do not require UI interaction
* We should add platform-independent tests for the parts of the code that do not require UI interaction
=== Results ===
The new crash reporter client is built around a simple platform-indepented UI
Rust library which wraps the individual platform-specific widgets. This made
the UI code almost completely platform-independent as opposed to the old
implementation which was entirely platform dependent, effectively requiring
three different indepented implementations. Additionally the new client is
extensively covered in tests and has more robust networking. Integration of
more features such as Glean-based crash pings and Necko-based networking is
now possible.
== Glean-based crash pings ==
Status: complete<br>
Developer(s): afranchuk<br>
Source code: https://github.com/mozilla/glean/<br>
Original source code:<br>
* https://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/client/ping.cpp
* https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/crashes/CrashManager.in.jsm#l711
* https://github.com/mozilla-mobile/android-components/tree/main/components/lib/crash
'''New source code and details are available at https://firefox-source-docs.mozilla.org/toolkit/components/crashes/crash-manager/crash-ping-lifecycle.html'''. <br>
Bugs:<br>
* {{bug|1784069}} - [meta] Migrate crash pings to Glean
=== Description ===
For every crash we detect Firefox Desktop sends a [https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/crash-ping.html crash ping] holding information that
would help us detect issues and prioritize which ones should be fixed. This
information is highly structured, resembles the JSON output of Socorro's
stackwalker, and unfortunately has seen relatively little use in the last few
years mostly because it's hard to process.
=== Rationale ===
The [https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/crash-ping.html crash ping]
uses legacy telemetry. This is problematic for a number of reasons: legacy telemetry is exclusively available in Firefox Desktop ('''not in GeckoView'''), we don't
have good interfaces to process these pings, the tools we use to extract information
can be complicated and mobile products are using a more modern data collection system,
[https://docs.telemetry.mozilla.org/concepts/glean/glean.html Glean].
Firefox for Android (aka Fenix) does not submit the crash ping that Firefox Desktop submits.
It instead records a [https://dictionary.telemetry.mozilla.org/apps/fenix/metrics/crash_metrics_crash_count crash_count metric] that's submitted in the metrics ping (via AC-'s [https://github.com/mozilla-mobile/android-components/tree/main/components/lib/crash lib-crash]). Fenix does collect crash-related information through Sentry. The old Firefox for Android (Fennec) had
full-featured crash pings but Fenix doesn't have any at all, leading to a rather
large blind spot in our telemetry.
The best way to address all of the above is to migrate the crash ping to Glean: once this is implemented, all Mozilla products using Glean will be able to benefit from this improvement (Firefox Desktop, Firefox Android, Focus, etc.)
=== Plan ===
This migration requires several steps with changes happening in different parts
of the codebase:
* Prepare the design of a minimal crash ping that can be implemented using existing client- and server-side machinery.
* Add support for this minimal Glean-based crash ping to Firefox desktop (inside the CrashManager) and Fenix where it needs to be done from scratch.
* Once the new ping's functionality has been validated, broaden the design to include parts that might require adding a new metric type (such as stack traces) and include all of the legacy crash ping information.
* Modify the code previously introduced to fully populate the Glean-based ping and make its payload match the legacy one. This might need extra work on the Fenix side, especially to capture stack traces.
* Last but not least the crash reporter client needs to be instructed to send Glean crash pings in addition to legacy telemetry pings. Currently Glean doesn't support C++ so this work will need to happen after we rewrite the crash reporter client.
* Decommission the legacy telemetry crash ping and remove the relevant code from Firefox desktop.
=== Results ===
Work on fully-featured Glean-based crash pings was completed in 2025H1 and
legacy crash pings were removed in 2025H2. This work built upon several
previous projects such as the Rust-based client-side minidump-analyzer which
provided high-quality stacks on Android machines, the new Rust-based crash
reporter client which allowed the use of Glean libraries (the previous C++
based crash reporter client could not send Glean telemetry) and several
additions to Glean itself to enable carrying structured data such as full stack
traces and module lists. Additionally a review of the data carried by the
legacy crash ping was done to assess which parts would be included in the Glean
pings and which would be dropped. The resulting data carried by the ping is
higher quality than in legacy telemetry but also significantly leaner as Glean
already provides information that we had to assemble manually. The ping also
carries the same data across different platforms, and its consistency is
guaranteed by its contents being generated from Glean metrics file instead of
being assembled manually like the legacy ping. Last but not least the new
telemetry data is used as the input of the new
[https://crash-pings.mozilla.org/ crash telemetry dashboard] which allows
visualizing crash information in a way that makes triage and analysis possible.


== minidump-analyzer ==
== minidump-analyzer ==


Status: not started<br>
Status: completed<br>
Developer(s):<br>
Developer(s):<br>
Source code: https://github.com/rust-minidump/rust-minidump/<br>
Source code: https://github.com/rust-minidump/rust-minidump/<br>
Line 161: Line 298:
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/minidump-analyzer
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/minidump-analyzer
Bugs:<br>
Bugs:<br>
* {{bug|1743983}}
* {{bug|1743983}} - Rewrite the minidump-analyzer in Rust


=== Description ===
=== Description ===
Line 208: Line 345:
Sentry is already working on integrating symbolic with rust-minidump so we're
Sentry is already working on integrating symbolic with rust-minidump so we're
currently waiting it out. This might require very little work in the end.
currently waiting it out. This might require very little work in the end.
=== Results ===
The new client-side minidump analyzer supports all our tier 1 and tier 2
architectures with full support for native deubg information, is significantly
more robust than the old one, extensively covered with tests and provide stacks
that are consistent with our server-side stack walker. Additionally the crates
used by the stack walker machinery are shared with the profiler reducing the
maintaince burden and yielding consistent results across the two projects.


= Server-side tools and components =
= Server-side tools and components =
Line 292: Line 438:
* https://hg.mozilla.org/mozilla-central/file/40bc01de5e10/toolkit/crashreporter/breakpad-patches
* https://hg.mozilla.org/mozilla-central/file/40bc01de5e10/toolkit/crashreporter/breakpad-patches
Bugs:<br>
Bugs:<br>
* {{bug|1588538}}
* {{bug|1588538}} - Use the new Windows dump_syms in Firefox local builds
* {{bug|1588534}}
* {{bug|1588534}} - Use the new Windows dump_syms to dump Microsoft libraries
* {{bug|1588739}}
* {{bug|1588739}} - Rewrite the Linux-specific implementation of dump_syms in Rust
* {{bug|1588740}}
* {{bug|1588740}} - Rewrite the macOS-specific implementation of dump_syms in Rust


=== Description ===
=== Description ===
Line 368: Line 514:
* https://hg.mozilla.org/mozilla-central/file/55f06c70f4e5/tools/rb/fix_stack_using_bpsyms.py
* https://hg.mozilla.org/mozilla-central/file/55f06c70f4e5/tools/rb/fix_stack_using_bpsyms.py
Bugs:<br>
Bugs:<br>
* {{bug|1596292}}
* {{bug|1596292}} - Replace stack-fixing scripts with a Rust-based one


=== Description ===
=== Description ===
1

edit