[webkit-reviews] review denied: [Bug 174028] Force crashWithInfo to be out of line. : [Attachment 314276] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 30 11:08:27 PDT 2017


Filip Pizlo <fpizlo at apple.com> has denied Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 174028: Force crashWithInfo to be out of line.
https://bugs.webkit.org/show_bug.cgi?id=174028

Attachment 314276: Patch

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




--- Comment #4 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 314276
  --> https://bugs.webkit.org/attachment.cgi?id=314276
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.


More information about the webkit-reviews mailing list