Bug 80566

Summary: Incorrect tracking of abstract values of variables forced double
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: InRadar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch none

Description Filip Pizlo 2012-03-07 21:12:32 PST
The issue is that the DFG CFA was incorrectly setting the abstract value of local variables that are forced double.  In mergeStateAtTail, it figures out what the state of all locals should be at the tail by looking at the last access to the variable.  If it's a SetLocal, it is incorrectly taking the child's abstract value and copying it into the local.  Instead, it should be setting the local's abstract value to PredictDouble if the local is shouldUseDoubleFormat().

This has no real effect in most cases, since executing the GetLocal would correctly give you a DataFormatDouble.  But If you ever did a GetLocal on the forced-double local, and then immediate a SetLocal to a different local, the SetLocal would think that it's setting the second local to the abstract value of the first local's SetLocal's child.  For example, if that first SetLocal had a child that was an int (hence it was performing an int->double cast) then the second local variable would think that it had an int. Subsequent GetLocals would think that it's OK to simply load an Int32, when in fact they were loading the low bits of a double.

Example:

Block #1:
a: SomethingThatProducesInt
b: SetLocal(@a, r1) predicted Double, forced double

Block #2:
c: GetLocal(r1) predicted Double, forced double
d: SetLocal(@c, r2) predicted IntDouble

Block #3:
e: GetLocal(r2) predicted IntDouble

@d will store a double into r2, but it will think that its abstract value is (Int, []).  Then @e will think that it's loading an (Int, []) and issue a 32-bit load of the payload.  And bang, we now have corrupted our numbers.
Comment 1 Filip Pizlo 2012-03-07 21:12:41 PST
<rdar://problem/11001442>
Comment 2 Filip Pizlo 2012-03-07 21:14:33 PST
Created attachment 130760 [details]
the patch
Comment 3 Filip Pizlo 2012-03-08 01:02:46 PST
Comment on attachment 130760 [details]
the patch

Reviewed in person by Gavin.
Comment 4 Filip Pizlo 2012-03-08 01:03:51 PST
Landed in http://trac.webkit.org/changeset/110153