The following testcase crashes on mozilla-central revision f0e6cc636021 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion): // Adapted from randomly chosen test: js/src/jit-test/tests/basic/undefined-warning-bug1274499.js options("werror"); // Adapted from randomly chosen test: js/src/jit-test/tests/wasm/basic.js function wasmFailValidateText(str, errorType, pattern) { let binary = wasmTextToBinary(str, 'new-format'); assertEq(WebAssembly.validate(binary), false); } wasmFailValidateText('(module (func) (func) (export "a" 2))', TypeError, /exported function index out of bounds/); Backtrace: 0 js-dbg-64-dm-clang-darwin-f0e6cc636021 0x000000010a7a929f js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 271 (jscntxtinlines.h:242) 1 js-dbg-64-dm-clang-darwin-f0e6cc636021 0x000000010a7a8ec3 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 643 (Interpreter.cpp:446) 2 js-dbg-64-dm-clang-darwin-f0e6cc636021 0x000000010a7a0a54 Interpret(JSContext*, js::RunState&) + 34452 (Interpreter.cpp:2922) 3 js-dbg-64-dm-clang-darwin-f0e6cc636021 0x000000010a7981c4 js::RunScript(JSContext*, js::RunState&) + 452 (Interpreter.cpp:404) 4 js-dbg-64-dm-clang-darwin-f0e6cc636021 0x000000010a7aa48f js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) + 511 (Interpreter.cpp:685) /snip For detailed crash information, see attachment.

autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/5ad42aa79b3d user: Benjamin Bouvier date: Wed Sep 07 17:52:31 2016 +0200 summary: Bug 1292723: Implement a basic WebAssembly.validate; r=luke Benjamin, is bug 1292723 a likely regressor?

Yes, but I think it's only catchable through the report-warning-as-errors shell flag. We probably need something like bool NoExceptionPending() at the end of Webassembly.validate.

(not sure the test has much worth, since it's shell only, so not adding it)

Comment on attachment 8794737 [details] [diff] [review] error.patch Review of attachment 8794737 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/asmjs/WasmJS.cpp @@ +1585,5 @@ > > callArgs.rval().setBoolean(validated); > + > + // Return isExceptionPending in case werror is active. > + return !cx->isExceptionPending(); Could you move the error check up to right after the report by writing "if (!JS_ReportErrorFlagsAndNumber...) return false"? Then no comment is necessary.

Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cb1127ffac27 Return value of WebAssembly_validate must be consistent with werror; r=luke

Benjamin, do you mind backporting this to mozilla-aurora? While wasm is still Nightly-only, the assertion is general enough that ignoring it would block finding other issues with the same assertion name, e.g. bug 1296661. Thanks! (even the stacks are highly similar, there isn't anything wasm-specific on the stack in this bug)

Sure, this one is easy to uplift.

Approval Request Comment [Feature/regressing bug #]: 1292723 [User impact if declined]: taking this will make the life of fuzzers easier (comment 8). [Describe test coverage new/current, TreeHerder]: test added, green for a few days on treeherder. [Risks and why]: no risk [String/UUID change made/needed]: n/a

FWIW, the shipping target is now 52, not 51, so we probably don't need to uplift anything until Nov :)

(In reply to Luke Wagner [:luke] from comment #11) > FWIW, the shipping target is now 52, not 51, so we probably don't need to > uplift anything until Nov :) Small patch easy to uplift + can make aurora fuzzing easier, so this one seems valuable to be uplifted :)

Comment on attachment 8797410 [details] [diff] [review] uplift.patch Take it in 51 aurora to make fuzzing easier.

has problems to apply: unable to find 'js/src/jit-test/tests/wasm/validate.js' for patching 1 out of 1 hunks FAILED -- saving rejects to file js/src/jit-test/tests/wasm/validate.js.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh bug1305197-uplift.patch

For kicks, I tried uplifting this to Aurora on top of bug 1292723, but I ended up having to backout for failures in validate.js. Guess we'll need an updated branch patch instead :( https://hg.mozilla.org/releases/mozilla-aurora/rev/0ac9735113c1a211afd2660a9e7066dcf33a28f3 https://treeherder.mozilla.org/logviewer.html#?job_id=3759004&repo=mozilla-aurora

(In reply to Ryan VanderMeulen [:RyanVM] from comment #16) > For kicks, I tried uplifting this to Aurora on top of bug 1292723, but I > ended up having to backout for failures in validate.js. Guess we'll need an > updated branch patch instead :( > https://hg.mozilla.org/releases/mozilla-aurora/rev/ > 0ac9735113c1a211afd2660a9e7066dcf33a28f3 > > https://treeherder.mozilla.org/logviewer.html#?job_id=3759004&repo=mozilla- > aurora Sorry for the trouble; I think one of these patches is also relying on bug 1287220, which patches we don't want to uplift. I'll make a simpler patch to uplift.

This one applies cleanly and passes on an aurora build. When merging from inbound to aurora, the validate.js aurora file can be removed and replaced by the inbound version (added a comment to that effect in the test itself).