Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotate right instruction is mis-compiled with LLVM #2215

Closed
HeliosPanoptes opened this issue Mar 26, 2021 · 5 comments · Fixed by #2217
Closed

Rotate right instruction is mis-compiled with LLVM #2215

HeliosPanoptes opened this issue Mar 26, 2021 · 5 comments · Fixed by #2217
Labels
bug Something isn't working 🏆 fuzzer-trophy Bugs found automatically by fuzzers. 📦 lib-compiler-llvm About wasmer-compiler-llvm

Comments

@HeliosPanoptes
Copy link

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

@HeliosPanoptes HeliosPanoptes added the bug Something isn't working label Mar 26, 2021
@syrusakbary
Copy link
Member

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
Copy link
Author

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

@HeliosPanoptes
Copy link
Author

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
Copy link
Member

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

@nlewycky
Copy link
Contributor

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!

@nlewycky nlewycky added 🏆 fuzzer-trophy Bugs found automatically by fuzzers. 📦 lib-compiler-llvm About wasmer-compiler-llvm labels Mar 26, 2021
@bors bors bot closed this as completed in 72d4733 Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🏆 fuzzer-trophy Bugs found automatically by fuzzers. 📦 lib-compiler-llvm About wasmer-compiler-llvm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants