On Wed, Mar 7, 2018 at 12:36 AM, Mark Lam <mark....@apple.com> wrote:
On Darwin, we currently only use WTFCrash() on Debug builds (see definition of the CRASH() macro). Feel free to make linux do the same. FWIW, I use this crash trace a lot when debugging crashes when I don’t already have a debugger attached yet. Of course, with a debugger attached, it is of less value.


Unfortunately that somehow breaks the top frames in the backtrace on Linux, so we can't do that.

The reason for release builds using WTFBreakpointTrap() is so that we can get a crash with minimal perturbation to the register state, to help with post-mortem crash analysis. Debug builds are only used during development and internal testing. So, we take the opportunity to get more crash info there (hence, the dumping of the crash trace). The reason that ASan builds use __builtin_trap() is because ASan does not like the memory access of 0xbadbbeef (if I remember correctly).

In addition, we also have infrastructure in place that looks for these crash patterns. So, changing to use SIGABRT (even if it generates a crash log on Darwin) would mean additional cost and churn to update that infrastructure, with not much gain to show for it. Hence, I object to the change.

OK, interesting. I won't change the behavior for Darwin, then.

Our crash telemetry does not care about the crash signal that's used; we'll get a nice backtrace regardless. Linux developers kinda expect SIGABRT to be used for intentional crashes like WTFCrash(), though, so it would be (very) slightly more useful for us to use abort().

Feel free to change the linux implementation of CRASH() to use abort() if that works better for linux folks. BTW, CRASH() is what you want to define/redirect. WTFCrash() is only one implementation of CRASH(). No client should be calling WTFCrash() directly.

CRASH() always calls WTFCrash(), except in Darwin release builds. So one option is to change only WTFCrash(). I experimented with changing CRASH() to call abort() directly and decided it doesn't really matter either way. The advantage of changing CRASH() is that there is one fewer uninteresting frame at the top of the backtrace, since WTFCrash will not be present. The cost is it will be very slightly harder to notice that CRASH() was called, since WTFCrash() won't be present in the backtrace.

Michael

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to