Bug 80566 - Incorrect tracking of abstract values of variables forced double
Summary: Incorrect tracking of abstract values of variables forced double
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2012-03-07 21:12 PST by Filip Pizlo
Modified: 2012-03-08 01:03 PST (History)
0 users

See Also:

the patch (3.14 KB, patch)
2012-03-07 21:14 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.


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