Bug 80566 – Incorrect tracking of abstract values of variables forced double

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

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.

[Filip Pizlo]

<rdar://problem/11001442>

[Filip Pizlo]

Created attachment 130760 [details]
the patch

[Filip Pizlo]

Comment on attachment 130760 [details]
the patch

Reviewed in person by Gavin.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/110153