Closed Bug 1206520 Opened 5 years ago Closed 4 years ago

Add a pref that makes asm.js validation failure throw an error?

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox43 --- affected
firefox44 --- fixed

People

(Reporter: jujjyl, Assigned: bbouvier)

Details

Attachments

(2 files)

It would be very useful to have a developer pref javascript.options.throw_on_asmjs_validation_error, which, if enabled, would cause an exception to be thrown is asm.js validation fails. This would make it easier in Emscripten and pthreads suites to catch situations where due to some bug validation has failed, by installing a window.onerror() handler to catch the validation error and report a failure back to the harness.

In Emscripten, we do validate asm.js on command line with SpiderMonkey before executing, however that is not quite good enough, since it uses a separately compiled executable, and sometimes a disparity happens between current FF Nightly vs whatever compiled SpiderMonkey one happens to have set up locally. If there was such a pref, we could be certain that validation tests are done in the same engine that we are actually running in.
Sounds fine to me
This adds an option to throw on validation failures (preconditions errors (e.g. the asm.js module function is a generator) and actual validation failures). Note that it only applies to validation failures, not link failures or directive failures (which are handled in several other places).

The suggested name is a bit long ("throw_on_asmjs_validation_failure"), but I didn't see how to make it shorter without making it less explicit.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8666640 - Flags: review?(luke)
This creates a boolean pref for enabling "throw_on_asmjs_validation_failure", which is specified in the previous comment and implemented in the first patch.
Attachment #8666651 - Flags: review?(bzbarsky)
Comment on attachment 8666640 [details] [diff] [review]
1.throw_on_asmjs_validation_failure.patch

Review of attachment 8666640 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!
Attachment #8666640 - Flags: review?(luke) → review+
Thinking about it, another alternative is just to set javascript.options.werror to true (it must be a boolean value). Even though asm.js compilation also warns when it succeeds, the error message could be pattern-matched and the successful compilation case could be put aside. Would that be enough?
Flags: needinfo?(jujjyl)
Oh, interesting. I did not know such an option already existed. Looks like the pref javascript.options.werror is hidden and must be added manually. With brief testing, it does look like it is usable to catch asm.js validation error in window.onerror(e), but the error it receives is a string "Script error." which is not ideal, but could work.

Is javascript.options.werror documented somewhere? What does it do exactly? Could it have unintended consequences?
Flags: needinfo?(jujjyl)
werror could do whatever it wants, since nothing really defines when the JS engine will do a warning.  It's particularly bad in "strict warnings" mode....
Comment on attachment 8666651 [details] [diff] [review]
2.add.about-config.prefs.patch

You need to add this to all.js to have it show up in about:config, right?

r=me with that
Attachment #8666651 - Flags: review?(bzbarsky) → review+
Thanks for doing this :bbouvier! Integrated the feature to emrun at https://github.com/juj/emrun/commit/ca9aa1767cb60d9e717ff5574b09c556ecfbe83e and Emscripten unit test suites at http://clb.demon.fi:8112/waterfall .
You need to log in before you can comment on or make changes to this bug.