bytecodealliance / bytecodealliance/wasmtime

Masking of shift amounts

Open
#2,409 0 comments 0 reactions 0 assignees View on GitHub
cranelift:area:x64
Dominant language
Rust
Stars
18.6k
Forks
1.8k
Avg merge
1d 18h
Merged PRs (30d)
126

Description

As discussed in https://github.com/bytecodealliance/wasmtime/pull/2397#discussion_r522410217, the Cranelift semantics for shift amounts must be masked to the number of bits that are present in the value to be shifted; e.g. if we try to shift a 32-bit integer by `33` we would actually shift by `33 mod 32 = 1`. Currently, these semantics (which match the Wasm semantics) are enforced in the translation from Wasm to CLIF (i.e. `code_translator.rs`) and not by the compiler backends. This means that users of Cranelift are able to use shifts with the incorrect semantics.

My proposed fix is to remove the masking from `code_translator.rs` and add it directly to the x64 backend--might need some help for the other backends (cc: @akirilov-arm, @cfallin). Another fix would be to change the semantics, but this seems less desirable so I don't plan to pursue it (let me know in the comments if you disagree!).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.