1747704 - Differential output between ion/baseline with WebAssembly exception handling

Status: RESOLVED FIXED

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

Description

[Christian Holler (:decoder)]

The attached testcase shows differential behavior on mozilla-central revision 20211223-bd494168b95a (opt build):

Run with --no-threads --wasm-compiler=ion --wasm-exceptions:

    //RuntimeError: table index out of bounds

Run with --no-threads --wasm-compiler=baseline --wasm-exceptions:

    0,0,0

Found by our wasm-smith differential testing target. Marking s-s until investigated.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Lars T Hansen [:lth]]

To Ryan for further triage.

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1747704 - wasm: Mark runtime errors generated in instance methods as traps. r?lth

We already mark runtime errors that are generated using the HandleTrap/signal
handler mechanism as 'traps'. We need to extend this to errors reported by
instance methods.

Comment 5

[Ryan Hunt [:rhunt] (on leave until early May)]

The root cause here is that instance calls are used to implement certain wasm instructions that may trap, but the instance call mechanism doesn't mark these WA.RuntimeError's as traps the way that the signal handler mechanism does. The differential execution comes down to the fact that the try notes for baseline are 1:1 with a whole try block, while Ion generates try notes for each wasm call inside of a try body (and not for instance calls, as they are assumed to only be able to throw non-catchable exceptions).

So we are generating exceptions that should not be catchable by wasm, but are not marked as such yet. And baseline can catch these exceptions because its try notes can cover instance calls, while Ion try notes will not because they are under the assumption that no catchable error should be thrown by instance calls.

This is not security sensitive because this is only trigger-able via pref'ed off code, and is only correctness based anyways.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6d4681c1280
wasm: Mark runtime errors generated in instance methods as traps. r=lth

Comment 7

[Marian-Vasile Laza]

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