Rotate right instruction is mis-compiled with LLVM

HeliosPanoptes commented 3 years ago

Describe the bug

echo "`wasmer -V` | `rustc -V` | `uname -m`"
wasmer-cli | rustc 1.52.0-nightly (4f20caa62 2021-03-01) | x86_64

git rev-parse HEAD
a4ddf2b4bbb9e370b08fddd7dade9715dbd8b15f

The i64.rotr instruction is mis-compiled with LLVM when given a rotate amount of 0.

Steps to reproduce

Reduced test case:

(module
  (func (;0;) (result i64)
    i64.const 4
    i64.const 0
    i64.rotr)
  (export "_main" (func 0)))

$ ./wasmer/target/release/wasmer rotate_bug_2.wat -i _main --cranelift
4

$ ./wasmer/target/release/wasmer rotate_bug_2.wat -i _main --llvm
-1

Expected behavior

Rotate by 0 should not change the first operand. I would expect the result to be 4.

Actual behavior

The result when compiled by llvm is -1.

Additional context

This behavior doesn't happen with i64.rotl, i32.rotr, or i32.rotl.

This test case is derived from a program created by Wasmlike, an Xsmith-based random program generator. https://www.flux.utah.edu/project/xsmith

syrusakbary commented 3 years ago

I believe this is a duplicate of #2143, which was solved with #2155. We have to create a new release with the fix though :)

HeliosPanoptes commented 3 years ago

This is from the latest version of the master branch though. I made sure to compile from source.

HeliosPanoptes commented 3 years ago

As a sanity check, I ran the reduced test case from #2143, and got back the correct result of 235. It's definitely related, but there's still bug present

syrusakbary commented 3 years ago

Awesome, thanks for the context. Adding @nlewycky here so he can take a look

nlewycky commented 3 years ago

This test case is derived from a program created by Wasmlike, an Xsmith-based random program generator. https://www.flux.utah.edu/project/xsmith

Nice!! We referred to Regehr's blog post on how to do rotates properly when fixing it!

As a response to your previous bug report we're also looking at adding a fuzzer based on wasm-smith for generation of valid wasm modules and libFuzzer to drive the fuzzing. Unfortunately this fuzzer isn't quite ready yet, it keeps finding that two compilers produce different stack traces on traps.

As an side, I'm worried that the way wasm-smith's derives valid wasm modules from the fuzzer controlled input will break the fuzzer's ability to reach similar paths in the code with similar inputs. That seems to be important for good fuzzing, second to exec/s.

Thanks for the bug reports, please keep them coming!

wasmerio / wasmer