983560 - OdinMonkey: Invalid JavaScript files get treated as asm.js even though they have invalid syntax

Status: RESOLVED INVALID

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(function() {
    "use asm"
    function f() {
        return
        if (1) 0
    }

A variant of this testcase was fixed as part of bug 981314, but notice that it is invalid JavaScript (the first anonymous function does not get completed), yet the assertion still shows on rev 6048059d6ea1.

It feels strange that testcases with invalid JS syntax get parsed as asm.js.

stdout:

$ ./js-dbg-64-dm-ts-darwin-6048059d6ea1 --no-asmjs 981314.js
981314.js:warning: asm.js type error: Disabled by javascript.options.asmjs in about:config
981314.js:6:4 SyntaxError: missing } after function body

$ ./js-dbg-64-dm-ts-darwin-6048059d6ea1 981314.js
Assertion failure: *thenBlock && *elseOrJoinBlock, at /Users/skywalker/trees/mozilla-central/js/src/jit/AsmJS.cpp:5017
Segmentation fault: 11


Thanks to :sunfish for pointing out during our long Caltrain adventures that this might be a bug. Luke, thoughts?

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh ../configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Odin lazily parses the different parts of an AsmJS module (module head, global statements, functions, etc.) to limit memory usage iirc. After fix for bug981314 landed, CheckModuleReturn returns false, as the next token is not the one we expect, implying the CheckModule function returns false, and the module gets reparsed => the syntax error shows up only at that point.

A possible fix would be to parse entirely the function at start once to check that there aren't obvious syntax errors, but that would eliminate all the benefits from parsing lazily, right?

Comment 2

[Luke Wagner [:luke]]

Benjamin's right: once we see "use asm", we start attempting to validate as asm.js before even seeing the next token, so this is expected behavior.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Luke Wagner [:luke] from comment #2)
> Benjamin's right: once we see "use asm", we start attempting to validate as
> asm.js before even seeing the next token, so this is expected behavior.

Thanks for the clarification! I guess this is RESOLVED INVALID then, since it is expected.