1727953 - Wasm Exception Handling in Wasm-baseline: test failing involving rethrow

Status: RESOLVED FIXED

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

Description

[Ioanna M. Dimitriou]

While implementing in Wasm-Ion the rethrow instruction of the WebAssembly Exception Handling proposal (Wasm-EH), I found that the following test fails for Wasm-Baseline:

assertEq(
  wasmEvalText(
    `(module
       (tag (param i32))
       (func (export "f") (result i32)
         (try (result i32)
           (do (try ;; Would pass if this was declared (result i32) instead.
                 (do (i32.const 13)
                     (throw 0))
                 (catch 0
                   ;; Also would pass if the value in the exception package was dropped here instead.
                   (rethrow 0))) ;; Also would pass if this was an (i32.const 13) (throw 0) instead.
               (unreachable)) ;; Having an actual i32.const value here instead, doesn't make a difference.
           (catch 0))))`
  ).exports.f(),
  13
);

With error message:

Assertion failure: stk_.length() == tryCatch.stackSize + type.length() + (tryKind == LabelKind::Try ? 0 : 1), at /home/ioa/igalia/code/gecko/js/src/wasm/WasmBaselineCompile.cpp:10820

Commenting out that assertion, and then the one just afterwards, which fails next, gives an error from this non-Wasm-EH assertion failing:

Assertion failure: size_t(np) == countedPointers, at /home/ioa/igalia/code/gecko/js/src/wasm/WasmBaselineCompile.cpp:3038

This might be an issue with the polymorphicity of rethrow in Wasm-Baseline, because it seems to fail for any leftover values in the catch block. So the following test fails with the same error message.

assertEq( // Fails for any left over values in the catch block.
  wasmEvalText(
    `(module
       (tag)
       (func (export "f") (result i32)
         (try (result i32)
           (do
             (try   ;; Would pass if this was declared (result i32) instead.
               (do (throw 0))
               (catch 0
                 (i32.const 4) ;; Also would pass without this line instead.
                 (rethrow 0))) ;; Also would pass if this was a (throw 0) instead.
             (unreachable))
           (catch 0
              (i32.const 13)))))`
  ).exports.f(),
  13
);

Comment 1

[Asumu Takikawa]

I believe what's going on here is that the rethrow code fails to correctly set deadCode_ = true. I can upload a patch after investigating a bit more to see if there's a more complex issue in this code path too.

Comment 2

[Asumu Takikawa]

Attachment: Bug 1727953 - Fix dead code flag for Wasm rethrow

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b126d147085f
Fix dead code flag for Wasm rethrow r=rhunt

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b126d147085f