864600 - OdinMonkey: change asm.js validation to require explicit coercion of all call expressions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Right now, only FFI calls require coercion, but a single-pass asm.js parser wants the result of all calls (whose return value is used) to be coerced.  Apparently Emscripten already does this, so existing code *should* continue to validate (but we'll see).

Comment 1

[Alon Zakai (:azakai)]

I noticed while verifying this issue that emscripten only coerces arguments into library calls, not other asm functions,

  otherAsmFunc($x, $y);
  libraryFunc($x|0, +$y);

But I think we do not need this for a single-pass parser, since you know during the very beginning which are library calls and which are not, and know the local variable types anyhow?

Comment 2

[Luke Wagner [:luke]]

That's right, we should know the type of the actual argument expressions; FFI calls only need the coercions because they can observe signed/unsigned.  (The coercion isn't necessary for double args, btw, double <: extern.)

Comment 3

[Luke Wagner [:luke]]

Attachment: coerce calls (WIP)

This patch makes the change.  I still need to fix up all the unit tests to validate with the new rules.

Alon: much Emscripten-generated asm.js validates with these new rules (bullet, zlib, skinning), but the big apps seem to hit a few missing corner cases (involving assignment).  If you attach the patch, you can see the type errors.

Comment 4

[Luke Wagner [:luke]]

Attachment: tweak doublish rules (WIP)

This patch restricts ~~ (double-to-int conversion) and binary - to only take double.  The reasoning is that, for any expression, only one of {intish,doublish} should be accepted.

Comment 5

[Alon Zakai (:azakai)]

I fixed the few small missing coercions on the emscripten test suite that popped up with these two patches applied.

Comment 6

[Luke Wagner [:luke]]

Attachment: tweak doublish rules

The spec change here is to change doublish to double in the signature of ~~ and binary -.  The reasoning is that there must be only one *ish type flowing into these operator signatures or else it is ambiguous, if the operand is a call, what the implied return type is.

Comment 7

[Luke Wagner [:luke]]

Attachment: coerce calls

See the big comment.

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 742640 [details] [diff] [review]
tweak doublish rules

Oops, did not mean to obsolete this one.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/66964658a097
https://hg.mozilla.org/mozilla-central/rev/42bcb974cfdd

Comment 10

[cscott]

This change doesn't seem to have made it into the spec at http://asmjs.org/spec/latest/ yet?  (And that spec could use a changelog...)