1275780 - Capture Rust panic messages in crash reports

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Nicholas Nethercote [inactive]]

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

[Alex Crichton [:acrichto]]

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.

Comment 2

[(not currently active) Ted Mielczarek]

Attachment: bug 1275780 - capture Rust panic message in crash reports.

Review commit: https://reviewboard.mozilla.org/r/124700/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124700/

Comment 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[(not currently active) Ted Mielczarek]

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 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8852489 [details]
bug 1275780 - capture Rust panic message in crash reports.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124700/diff/1-2/

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8852489 [details]
bug 1275780 - capture Rust panic message in crash reports.

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

Thank you!

Comment 7

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/23c5ecf4d92e3b2039ffbe6a9f73f56d25b57c24
bug 1275780 - capture Rust panic message in crash reports. r=froydnj

Comment 8

[Iris Hsiao [:ihsiao]]

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

Comment 9

[(not currently active) Ted Mielczarek]

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

Comment 10

[(not currently active) Ted Mielczarek]

...the xpcshell harness doesn't support `fails-if` in manifests. That's a bummer.

Comment 11

[(not currently active) Ted Mielczarek]

(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.

Comment 12

[(not currently active) Ted Mielczarek]

To close the loop here: it's `fail-if`, not `fails-if`. We should totally error on unknown keys...

Comment 13

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e59908610f4e0c922ee01904c4817adf3934f0
bug 1275780 - capture Rust panic message in crash reports. r=froydnj

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d8e59908610f

Comment 15

[(not currently active) Ted Mielczarek]

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

[Ralph Giles (:rillian)]

\o/

Thanks for getting this working, Ted.

Comment 17

[Xidorn Quan [:xidorn] UTC+10]

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

[(Away)]

(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.