Bug 174028 – Force crashWithInfo to be out of line.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

Force crashWithInfo to be out of line.

[Keith Miller]

Created attachment 314275 [details]
Patch

[Keith Miller]

Created attachment 314276 [details]
Patch

[Filip Pizlo]

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.

[Filip Pizlo]

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.

[Keith Miller]

Created attachment 314283 [details]
Patch

[Keith Miller]

Created attachment 314284 [details]
Patch

[Keith Miller]

Created attachment 314286 [details]
Patch

[Keith Miller]

(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.

[Keith Miller]

Created attachment 314293 [details]
Patch for landing

[Keith Miller]

Created attachment 314296 [details]
Patch for landing

[Keith Miller]

Created attachment 314325 [details]
Patch for landing

[Keith Miller]

Created attachment 314336 [details]
Patch for landing

[Keith Miller]

Created attachment 314341 [details]
Patch for landing

[WebKit Commit Bot]

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.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 314341 [details]:

The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 314341 [details]
Patch for landing

Clearing flags on attachment: 314341

Committed r219046: <http://trac.webkit.org/changeset/219046>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Keith Miller]

rdar://problem/33099582