Summary: | Force crashWithInfo to be out of line. | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keith Miller <keith_miller> | ||||||||||||||||||||||
Component: | New Bugs | Assignee: | 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
Keith Miller
2017-06-30 10:45:23 PDT
Created attachment 314275 [details]
Patch
Created attachment 314276 [details]
Patch
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 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. Created attachment 314283 [details]
Patch
Created attachment 314284 [details]
Patch
Created attachment 314286 [details]
Patch
(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. Created attachment 314293 [details]
Patch for landing
Created attachment 314296 [details]
Patch for landing
Created attachment 314325 [details]
Patch for landing
Created attachment 314336 [details]
Patch for landing
Created attachment 314341 [details]
Patch for landing
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. The commit-queue encountered the following flaky tests while processing attachment 314341 [details]:
The commit-queue is continuing to process your patch.
Comment on attachment 314341 [details] Patch for landing Clearing flags on attachment: 314341 Committed r219046: <http://trac.webkit.org/changeset/219046> All reviewed patches have been landed. Closing bug. |