Bug 109387

Summary: DFG CompareEq(a, null) and CompareStrictEq(a, const) are unsound with respect to constant folding
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, jberlin, mark.lam, mhahnenberg, msaboff, oliver, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 109470    
Bug Blocks:    
Attachments:
Description Flags
the patch
none
the patch oliver: review+

Filip Pizlo
Reported 2013-02-10 13:47:18 PST
CompareEq(a, null) does strictly less speculation than CompareEq(a, b). Hence if we do one run of CFA and eliminate checks based on seeing CompareEq(a, b) - which can happen because CompareEq(a, b) "proves" that we had done a check which we may later eliminate - and then later prove that b is null, then we will end up emitting zero checks. And then weirdness ensues.
Attachments
the patch (12.22 KB, patch)
2013-02-10 21:01 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (14.58 KB, patch)
2013-02-10 22:36 PST, Filip Pizlo 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2013-02-10 21:01:31 PST
Created attachment 187510 [details] the patch Not marking r? yet because I want to run more benchmarks...
Filip Pizlo
Comment 2 2013-02-10 22:36:47 PST
Created attachment 187514 [details] the patch
Filip Pizlo
Comment 3 2013-02-11 11:23:05 PST
Landed in http://trac.webkit.org/changeset/142491
Jessie Berlin
Comment 4 2013-02-11 11:52:13 PST
(In reply to comment #3) > Landed in http://trac.webkit.org/changeset/142491 This caused build breakage: DFGSpeculativeJIT32_64.cpp:2113:13: error: enumeration values 'CompareEqConstant' and 'CompareStrictEqConstant' not handled in switch [-Werror,-Wswitch] switch (op) { ^
WebKit Review Bot
Comment 5 2013-02-11 12:31:07 PST
Re-opened since this is blocked by bug 109470
Jessie Berlin
Comment 6 2013-02-11 12:34:10 PST
Rollout is happening in https://bugs.webkit.org/show_bug.cgi?id=109470
Filip Pizlo
Comment 7 2013-02-11 14:24:54 PST
Landed in http://trac.webkit.org/changeset/142515
Note You need to log in before you can comment on or make changes to this bug.