1603772 - Cranelift: fix ret value of instanceCall when the ret value is only internally used

Status: RESOLVED FIXED

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

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

In wasm2clif instance_call, we shouldn't return the ret Value when it's only used for runtime error checking, otherwise this trips up assertions.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1603772: Cranelift: only return the ret value when it's not used for internal purposes; r?rhunt

The return value of a wasm builtin call may just be used to check if the
runtime caused an internal error (e.g. oom). There are assertions in code that
the return value of wasm builtins not supposed to return a wasm value actually
do this, so we shouldn't return values that are only internally used.

This could have been done a simpler way by only having "FailureMode::NotZero"
imply "do not return", but this is more future-proof like this: shared memory
/ atomics builtins both check the internal value and return it to the wasm
value stack.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1603772: Revert the error type change when instantiating segments with Cranelift; r?rhunt

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1603772: Cranelift: replace native_pointer_{size,type} by const values; r?rhunt

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

With the first two patches (the last one is the end of a small refactoring we discussed earlier), all the wasm jit-tests now pass on x64 with Cranelift!

Comment 5

[Ryan Hunt [:rhunt]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #4)

With the first two patches (the last one is the end of a small refactoring we discussed earlier), all the wasm jit-tests now pass on x64 with Cranelift!

Nice!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b92d5b9b880a
Cranelift: only return the ret value when it's not used for internal purposes; r=rhunt
https://hg.mozilla.org/integration/autoland/rev/fbeac1746d50
Revert the error type change when instantiating segments with Cranelift; r=rhunt
https://hg.mozilla.org/integration/autoland/rev/b0fb09545a01
Cranelift: replace native_pointer_{size,type} by const values; r=rhunt

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b92d5b9b880a
https://hg.mozilla.org/mozilla-central/rev/fbeac1746d50
https://hg.mozilla.org/mozilla-central/rev/b0fb09545a01