Closed Bug 1603772 Opened 2 years ago Closed 2 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: bbouvier, Assigned: bbouvier)

References

Details

Attachments

(3 files)

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.

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.

Priority: -- → P3

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!

(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!

Pushed by bbouvier@mozilla.com:
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
Duplicate of this bug: 1604004
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
You need to log in before you can comment on or make changes to this bug.