Bug 174028 - Force crashWithInfo to be out of line.
Summary: Force crashWithInfo to be out of line.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-30 10:45 PDT by Keith Miller
Modified: 2017-07-02 15:17 PDT (History)
11 users (show)

See Also:

Attachments
Patch (12.38 KB, patch)
2017-06-30 10:57 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (12.38 KB, patch)
2017-06-30 11:02 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (13.19 KB, patch)
2017-06-30 13:29 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (9.16 KB, patch)
2017-06-30 13:50 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch (12.39 KB, patch)
2017-06-30 14:09 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch for landing (12.58 KB, patch)
2017-06-30 14:31 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch for landing (12.59 KB, patch)
2017-06-30 14:49 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch for landing (12.59 KB, patch)
2017-06-30 16:56 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch for landing (12.58 KB, patch)
2017-06-30 17:44 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Patch for landing (12.82 KB, patch)
2017-06-30 18:07 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
Show Obsolete (9) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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