Closed Bug 927112 Opened 6 years ago Closed 6 years ago

OdinMonkey: loosen up type rules for ~~ and +

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: luke, Assigned: luke)

Details

Attachments

(2 files)

This bug loosens the asm.js type system in two places where we are currently overly conservative:

~~ can take doublish since it will coerce 'undefined' to 0 which is equivalent to ~~+undefined.

Binary + can take doublish since 'undefined' will be coerced to NaN.  The danger is if + could somehow end up doing a string concat, but this requires one of lhs or rhs to already be a string, so we're safe.
This patch also removes Type::Unknown which is no longer part of the asm.js type system (it was removed with the new explicit call coercion rules).
Attachment #817415 - Flags: review?(sstangl)
Attached patch change-add-ruleSplinter Review
Attachment #817416 - Flags: review?(sstangl)
Attachment #817415 - Flags: review?(sstangl) → review+
Attachment #817416 - Flags: review?(sstangl) → review+
Sketch of the correctness argument below:

+: The only way a non-double value can flow into an expression of type doublish is the value undefined being triggered by out-of-bounds reads from the heap typed array. So the only cases for + we have to consider are double + double, undefined + double, double + undefined, and undefined + undefined. In all three cases, we are at step 8 in 11.6.1 of ES5:

http://ecma-international.org/ecma-262/5.1/#sec-11.6.1

ToNumber(undefined) is NaN, so the implementation of the out-of-bounds load can just produce NaN.

~~: We are at step 2 in 11.4.8 of ES5:

http://ecma-international.org/ecma-262/5.1/#sec-11.4.8

ToInt32(undefined) converts undefined to NaN (step 1) and then returns 0 (step 2). So again the out-of-bounds load can just produce NaN.

Dave
https://hg.mozilla.org/mozilla-central/rev/33d9f7e62128
https://hg.mozilla.org/mozilla-central/rev/84d504a2c92a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.