search

asm-js / validator

A reference validator for asm.js.
Apache License 2.0
1.78k stars 154 forks source link

Validator accepts code that it shouldn't #104

Open kripken opened 9 years ago

kripken commented 9 years ago

See https://github.com/mbebenita/Broadway/pull/57 , it looks like the validator accepts

var max = stdlib.Math.max;
[..]
r = min(255, max(0, r)|0)|0;

even though there should be a coercion on r.

sunfishcode commented 9 years ago

In the current spec, the signature for max is (int, int…) → signed. r has type int, so I don't see anything in the spec making this fragment invalid. Is this also a spec bug?

OdinMonkey appears to check that max arguments are signed, in apparent disagreement with the spec. The error message says int though.

kripken commented 9 years ago

Looks like a spec bug to me. We don't know if the input variable is signed or unsigned (or neither) at that point, and it's not going to be signed by the call target, so we have to sign it (or unsign it) here.

dead-claudia commented 9 years ago

Note from the readme (4th paragraph down):

"This repo also hosts JS source code which performs asm.js validation, however as of this update, this code is not up to date with the latest working draft and is not extensively tested. Patches to update it or fix bugs are welcome though."

I think this is a known problem with the validator.

sunfishcode commented 9 years ago

What we have here is a discrepancy between the spec and OdinMonkey, and it looks like an actual spec bug. When that's resolved, we'll fix the validator too.