1324116 - Baldr: add WebAssembly.LinkError and throw it for errors during instantiation

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Luke Wagner [:luke]]

As added by https://github.com/WebAssembly/design/pull/901

Comment 1

[Luke Wagner [:luke]]

Attachment: link-error

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8821284 [details] [diff] [review]
link-error

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

Thanks, there's still one case where the spec says to throw a TypeError and we'd now throw a LinkError, so cancelling review until either the spec or our code gets fixed.

For what it's worth, the spec proto also shows two other implicit uses of LinkError:
- https://github.com/WebAssembly/spec/blob/master/interpreter/spec/eval.ml#L425, which I think we can't have by construction (looks like it could be an assert in the spec proto).
- https://github.com/WebAssembly/spec/blob/master/interpreter/spec/eval.ml#L405, which I also think we can't have by construction (we create imports and control their kinds; that being said, we could and should provide a default case in GetImports's switch, just in case something gets terribly wrong)

::: js/src/jit-test/tests/wasm/basic.js
@@ +104,5 @@
>  
>  var code = '(module (import "a" "b"))';
>  assertErrorMessage(() => wasmEvalText(code), TypeError, noImportObj);
> +assertErrorMessage(() => wasmEvalText(code, {}), LinkError, notObject);
> +assertErrorMessage(() => wasmEvalText(code, {a:1}), LinkError, notObject);

(see other comment: the last two should still be TypeErrors per spec)

::: js/src/js.msg
@@ +358,5 @@
>  MSG_DEF(JSMSG_USE_ASM_TYPE_OK,         1, JSEXN_WARN,    "Successfully compiled asm.js code ({0})")
>  
>  // wasm
>  MSG_DEF(JSMSG_WASM_COMPILE_ERROR,      1, JSEXN_WASMCOMPILEERROR, "{0}")
> +MSG_DEF(JSMSG_WASM_BAD_IMPORT_FIELD,   2, JSEXN_WASMLINKERROR, "import object field '{0}' is not {1}")

This error is used when importObject[module_name] is not an object, but the spec (step 2) still says this should be a TypeError, so we have a discrepancy here and one of both sides has to be fixed.

Comment 3

[Luke Wagner [:luke]]

Good catch!

Comment 4

[Luke Wagner [:luke]]

Attachment: link-error

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8821608 [details] [diff] [review]
link-error

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

Great, thanks

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67286eb47060
Baldr: add WebAssembly.LinkError and throw it for errors during instantiation (r=bbouvier)

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/67286eb47060