Closed
Bug 1275780
Opened 8 years ago
Closed 8 years ago
Capture Rust panic messages in crash reports
Categories
(Toolkit :: Crash Reporting, defect)
Toolkit
Crash Reporting
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: ted)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
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.)
Comment 1•8 years ago
|
||
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.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ted
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•8 years ago
|
||
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.
Assignee | ||
Comment 4•8 years ago
|
||
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•8 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+
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/23c5ecf4d92e3b2039ffbe6a9f73f56d25b57c24 bug 1275780 - capture Rust panic message in crash reports. r=froydnj
Blocks: 1352265
Comment 8•8 years ago
|
||
sorry had to back this out for xpcshell perma-failing in Win8 like https://treeherder.mozilla.org/logviewer.html#?job_id=87655074&repo=mozilla-inbound&lineNumber=13675 https://hg.mozilla.org/integration/mozilla-inbound/rev/a45d5f56491c https://hg.mozilla.org/integration/mozilla-inbound/rev/f7da34043503
Flags: needinfo?(ted)
Assignee | ||
Comment 9•8 years ago
|
||
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)
Assignee | ||
Comment 10•8 years ago
|
||
...the xpcshell harness doesn't support `fails-if` in manifests. That's a bummer.
Assignee | ||
Comment 11•8 years ago
|
||
(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.
Assignee | ||
Comment 12•8 years ago
|
||
To close the loop here: it's `fail-if`, not `fails-if`. We should totally error on unknown keys...
Assignee | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e59908610f4e0c922ee01904c4817adf3934f0 bug 1275780 - capture Rust panic message in crash reports. r=froydnj
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d8e59908610f
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee | ||
Comment 15•8 years ago
|
||
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!
Comment 16•8 years ago
|
||
\o/ Thanks for getting this working, Ted.
Comment 17•7 years ago
|
||
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.
![]() |
||
Comment 18•7 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•