Capture Rust panic messages in crash reports

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: njn, Assigned: ted)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
In Bug 1266309 comment 17, acrichton said:

> I'd highly recommend capturing the panic message as well as it should in
> theory contain the OS error message which would point to OOM. That'd at
> least provide another vector for tracking down panics in Rust!

We should do that.

(I'm not sure if this is the right Bugzilla component for this bug.)
Ah and to clarify how to do this, there are two methods:

std::panic::catch_unwind (http://doc.rust-lang.org/nightly/std/panic/fn.catch_unwind.html) which allows catching all panics and inspection of the payload, this is stable as of 1.9. This won't work with panic=abort, however.

std::panic::set_hook (http://doc.rust-lang.org/nightly/std/panic/fn.set_hook.html) which runs just before the panic and works with panic=abort. This will be stable in 1.10, however.
(Reporter)

Updated

2 years ago
Blocks: 1268328
No longer blocks: 1268328
Blocks: 1348896
Assignee: nobody → ted
Comment hidden (mozreview-request)
I think the code comments explain the reasoning pretty well here, but I made it so we'll capture the Rust panic reason and send it as the "MozCrashReason" annotation, which is the same place we put the existing `MOZ_CRASH` reason, so it will show up in the Socorro UI usefully. I tried to make sure we weren't going to do any extra allocation in the panic handler, since that's bad news in OOM situations.
Oops, my try builds all burned because apparently `PanicInfo` didn't implement `Debug` until Rust 1.16. I'm only using that in the "Unhandled panic payload" case, so I'll just change it to not output the payload there.
Comment hidden (mozreview-request)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8852489 [details]
bug 1275780 - capture Rust panic message in crash reports.

https://reviewboard.mozilla.org/r/124700/#review127716

Thank you!
Attachment #8852489 - Flags: review?(nfroyd) → review+

Updated

2 years ago
Blocks: 1352265
Sorry about that. I did put a manifest annotation to skip this on Win64 but apparently it didn't work:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40eb77a98bcf#l2.13

I also note that my "-b do -p all -t xpcshell" try push didn't seem to run win64 xpcshell tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bd301b2b887
Flags: needinfo?(ted)
...the xpcshell harness doesn't support `fails-if` in manifests. That's a bummer.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #10)
> ...the xpcshell harness doesn't support `fails-if` in manifests. That's a
> bummer.

No, I'm wrong, it claims to, but it didn't work in this case. I don't know why.
To close the loop here: it's `fail-if`, not `fails-if`. We should totally error on unknown keys...

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d8e59908610f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Here's an intentional Rust panic crash from today's OS X Nightly:
https://crash-stats.mozilla.com/report/index/21785f0a-ad09-4cd2-8916-cdb032170404

It shows:
MOZ_CRASH Reason	 Crashme!
\o/

Thanks for getting this working, Ted.

Updated

2 years ago
Depends on: 1352647
Depends on: 1362901
Depends on: 1373881
Weird. 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.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #17)
> Weird. 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.

For posterity: This got filed as bug 1379857.

Updated

a year ago
Blocks: 1381560
You need to log in before you can comment on or make changes to this bug.