Re: [webkit-dev] WTFCrash()

[Michael Catanzaro]

On Wed, Mar 7, 2018 at 10:37 AM, Michael Catanzaro 
 wrote:
Unfortunately that somehow breaks the top frames in the backtrace on 
Linux, so we can't do that.


I mean, we can't do it in exactly the same way that Darwin does. Yes, 
of course we can change CRASH() to call abort() or do whatever we want.


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

Re: [webkit-dev] WTFCrash()

[Michael Catanzaro]

On Wed, Mar 7, 2018 at 12:36 AM, Mark Lam  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