1495662 - --wasm-gc is not honored by JS shell with ENABLE_WASM_CRANELIFT

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Lars T Hansen [:lth]]

Superficially this seems to be a result of some new machinery introduced around a 'ScriptedCaller' object in the wasm subtree; this does not carry the required information and the shortfall is not made up by other logic.  I'm guessing this came in with Cranelift but I'm not sure yet.

This should have broken all wasm/gc test cases, except if our implementation of wasmGcSupported() is affected by the same problem.

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1495662-cranelift-and-gc-switches.patch

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 9013544 [details] [diff] [review]
bug1495662-cranelift-and-gc-switches.patch

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

oops, my bad, sorry. Thanks for the fix!

::: js/src/shell/js.cpp
@@ +10064,5 @@
> +#if defined ENABLE_WASM_GC && defined ENABLE_WASM_CRANELIFT
> +    // Note, once we remove --wasm-gc this test will no longer make any sense
> +    // and we'll need a better solution.
> +    if (enableWasmGc && wasmForceCranelift) {
> +        fprintf(stderr, "Do not combine --wasm-gc and --wasm-force-cranelift, they are incompatible.\n");

Alternatively, we could just have `enableWasmGc = op.getBoolOption("wasm-gc") && !wasmForceCranelift;`. Your solution works fine too.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16385c9bc942
cranelift presence must not disable wasm-gc.  r=bbouvier

Comment 4

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/16385c9bc942