788069 - Make mfbt's MOZ_CRASH push a stack frame before crashing the process

Status: RESOLVED WONTFIX

(Core :: MFBT, defect)

Description

[Justin Lebar (not reading bugmail)]

MOZ_CRASH() doesn't always yield clear stack traces in our crash stats, because it doesn't push a stack frame before crashing.

Although many callers of MOZ_CRASH() wrap it in a not-inlined function (see e.g. bug 787675), not all callers do.  Anyway, callers shouldn't have to jump through hoops in order to get decent crash stats.

Comment 1

[Benjamin Smedberg]

Hrm. *Not* pushing a frame was in fact the whole point of having it as a macro, because sometimes we weren't backtracing the first frame correctly and would end up skipping the important frame.

Comment 2

[Justin Lebar (not reading bugmail)]

Ah, interesting!

I guess we could wontfix this and continue to fix it on a case-by-case basis.  Or we could have a MOZ_CRASH_INLINED() and MOZ_CRASH_OUTOFLINE(), which is kind of ugly.

(Also, I'm having difficulty figuring out the build-fu required to make this bug work as-is.)

Comment 3

[Jeff Walden [:Waldo]]

MOZ_CRASH() was also made to crash completely inline so that when debugging, hitting an assertion will park the debugger on the assertion in question.  Useful crash stacks is one good, but you have to trade it off against that other good.  It certainly helps me a lot as a SpiderMonkey developer to not have to type "up N", for some N (which was non-constant over time until we made this change!), to get to where things actually went wrong.

Comment 4

[Justin Lebar (not reading bugmail)]

I would be totally fine with a solution which added a stack frame only in release builds, or builds where the crash reporter was enabled, or something.

Comment 5

[Benjamin Smedberg]

Or we could include the line number of the MOZ_CRASH in as the crash annotation (as NS_RUNTIMEABORT does still, I think), so that any inlining weirdness wouldn't be directly relevant.

Comment 6

[Simon Giesecke [:sg] [he/him]]

Bug 763070 implements what's suggested by Comment 5.

While that doesn't do what the bug description says, I assume it's an acceptable solution overall.

Please reopen if something still should be done here.