Bug 1275780 comment 17 says: > Panic report like bp-2abff06f-d969-4ba5-845b-a98410170708 doesn't have the > MOZ_CRASH Reason field. And in report bp-03718a9c-9d98-4832-b8a6-026220170706, > there is such field, but it contains nothing useful. And from bug 1379101, we have https://crash-stats.mozilla.com/report/index/4f8bc612-65c6-4959-93f4-fd2ce0170708 which is an expect() failure that likewise lacks a MOZ_CRASH Reason field. Ted, I've assigned to you given that this is a follow-up to bug 1275780. Hope that's ok!

I believe I see the issue here. We appear to only watch for Rust panics in the main process at the moment, so we need to also hook child processes as well.

Here's a test report from the content process: https://crash-stats.mozilla.com/report/index/dde8784d-71cb-495e-ae20-5e0ba0170712 (No symbols uploaded, but the panic message appears.)

Great! Thanks for working on this!

Comment on attachment 8885922 [details] Bug 1379857 - Record Rust panics for child process crashes. https://reviewboard.mozilla.org/r/156710/#review161836 Thank you for taking this bug. This patch looks plausible to me but I'm not an appropriate reviewer for crash reporter changes. I would normally suggest Ted, but I believe he's on PTO this week. Maybe gsvelto could review?

:gsvelto seems to be on PTO as well. Maybe :dmajor? Let's try it...

Comment on attachment 8885922 [details] Bug 1379857 - Record Rust panics for child process crashes. https://reviewboard.mozilla.org/r/156710/#review162126 Thank you for including a test! ::: toolkit/crashreporter/nsExceptionHandler.cpp:1620 (Diff revision 1) > const char *envvar = PR_GetEnv("MOZ_CRASHREPORTER"); > if ((!envvar || !*envvar) && !force) > return NS_OK; > #endif > > + install_rust_panic_hook(); I wonder if we should move this down even more, near the end next to mozalloc_set_oom_abort_handler and oldTerminateHandler, in order to follow the same pattern as the child functions?

Comment on attachment 8885922 [details] Bug 1379857 - Record Rust panics for child process crashes. https://reviewboard.mozilla.org/r/156710/#review162126 > I wonder if we should move this down even more, near the end next to mozalloc_set_oom_abort_handler and oldTerminateHandler, in order to follow the same pattern as the child functions? Yes, seems resonable! Looks like there are a few more abort conditions in the middle, and it makes sense to only install the panic hook if we're really going to report things.

I assume we'll want this on Beta for 55 as well. Please nominate it for uplift when you're comfortable doing so.

Pushed by jryans@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b972eadf54e8 Record Rust panics for child process crashes. r=dmajor

Comment on attachment 8885922 [details] Bug 1379857 - Record Rust panics for child process crashes. Approval Request Comment [Feature/Bug causing the regression]: Bug 1275780 started to capture Rust panic messages in crash reports in 55, but it only did so for the parent process. [User impact if declined]: If declined, content process crash reports will miss Rust panic messages. For Stylo at least, we don't _have_ to have this, as it's not enabled for Beta 55, but there might be other Rust code that this code be helpful for. [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Reuses the existing panic handling from the parent process in the child. We've confirmed it's working in Nightly crash reports. [String changes made/needed]: None

Comment on attachment 8885922 [details] Bug 1379857 - Record Rust panics for child process crashes. looks harmless enough and possibly useful, let's get this in 55.0b11