Bug 174028

Summary: Force crashWithInfo to be out of line.
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, commit-queue, dbates, fpizlo, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Description Keith Miller 2017-06-30 10:45:23 PDT
Force crashWithInfo to be out of line.
Comment 1 Keith Miller 2017-06-30 10:57:48 PDT
Created attachment 314275 [details]
Patch
Comment 2 Keith Miller 2017-06-30 11:02:54 PDT
Created attachment 314276 [details]
Patch
Comment 3 Filip Pizlo 2017-06-30 11:07:19 PDT
Comment on attachment 314275 [details]
Patch

This will still lead to coalescing.  You need to feed reason, __FILE__, __LINE__, and probably WTF_PRETTY_FUNCTION for good measure.

Remember, any operation of the form op(a, b, c) in tail position will get coalesced with any other op(a, b, c) in tail position.  A crash macro is in tail position.  I don't think you're passing enough interesting things in __VA_ARGS__ to deduplicate this.
Comment 4 Filip Pizlo 2017-06-30 11:08:27 PDT
Comment on attachment 314276 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314276&action=review

> Source/JavaScriptCore/dfg/DFGGraph.h:101
> -        (graph).logAssertionFailure(                                 \
> +        (graph).logAssertionFailure(                                    \
>              (node), __FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \
> -        CRASH_WITH_INFO(__VA_ARGS__);                                       \
> +        CRASH_WITH_SECURITY_IMPLICATION_AND_INFO(__VA_ARGS__);          \

This doesn't fix coalescing.  Please pass #assertion, __FILE__, __LINE__, and WTF_PRETTY_FUNCTION to CRASH_WITH_SECURITY_IMPLICATION, so that it gets deduplicated appropriately.

Ideally, you would have this functionality inside logAssertionFailure, so that you don't have to pass the same things twice.
Comment 5 Keith Miller 2017-06-30 13:29:52 PDT
Created attachment 314283 [details]
Patch
Comment 6 Keith Miller 2017-06-30 13:50:45 PDT
Created attachment 314284 [details]
Patch
Comment 7 Keith Miller 2017-06-30 14:09:47 PDT
Created attachment 314286 [details]
Patch
Comment 8 Keith Miller 2017-06-30 14:12:49 PDT
(In reply to Filip Pizlo from comment #3)
> Comment on attachment 314275 [details]
> Patch
> 
> This will still lead to coalescing.  You need to feed reason, __FILE__,
> __LINE__, and probably WTF_PRETTY_FUNCTION for good measure.
> 
> Remember, any operation of the form op(a, b, c) in tail position will get
> coalesced with any other op(a, b, c) in tail position.  A crash macro is in
> tail position.  I don't think you're passing enough interesting things in
> __VA_ARGS__ to deduplicate this.

I ended up passing __FILE__, __LINE__, WTF_PRETTY_FUNCTION, and __COUNTER__. Hopefully, that's enough junk. I wish there was an attribute for this.
Comment 9 Keith Miller 2017-06-30 14:31:24 PDT
Created attachment 314293 [details]
Patch for landing
Comment 10 Keith Miller 2017-06-30 14:49:19 PDT
Created attachment 314296 [details]
Patch for landing
Comment 11 Keith Miller 2017-06-30 16:56:19 PDT
Created attachment 314325 [details]
Patch for landing
Comment 12 Keith Miller 2017-06-30 17:44:05 PDT
Created attachment 314336 [details]
Patch for landing
Comment 13 Keith Miller 2017-06-30 18:07:51 PDT
Created attachment 314341 [details]
Patch for landing
Comment 14 WebKit Commit Bot 2017-06-30 19:42:50 PDT
The commit-queue encountered the following flaky tests while processing attachment 314341 [details]:

fast/workers/worker-exception-during-navigation.html bug 174058
The commit-queue is continuing to process your patch.
Comment 15 WebKit Commit Bot 2017-06-30 19:42:51 PDT
The commit-queue encountered the following flaky tests while processing attachment 314341 [details]:

The commit-queue is continuing to process your patch.
Comment 16 WebKit Commit Bot 2017-06-30 23:24:47 PDT
Comment on attachment 314341 [details]
Patch for landing

Clearing flags on attachment: 314341

Committed r219046: <http://trac.webkit.org/changeset/219046>
Comment 17 WebKit Commit Bot 2017-06-30 23:24:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Keith Miller 2017-07-02 15:17:59 PDT
rdar://problem/33099582