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

Cranelift: Interpreter panic when bitcasting SIMD values #5915

Closed
afonso360 opened this issue Mar 2, 2023 · 1 comment
Closed

Cranelift: Interpreter panic when bitcasting SIMD values #5915

afonso360 opened this issue Mar 2, 2023 · 1 comment
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! cranelift Issues related to the Cranelift code generator good first issue Issues that are good for new contributors to tackle!

Comments

@afonso360
Copy link
Contributor

👋 Hey,

.clif Test Case

test interpret

function %a(i16x8) -> i64x2 {
block0(v3: i16x8):
    v23 = bitcast.i64x2 little v3
    return v23
}

; run: %a(0x00000000000000000000000000000000) == 0x00000000000000000000000000000000

Steps to Reproduce

  • cd cranelift
  • cargo run -- test ./the-above.clif

Expected Results

The test to pass.

Actual Results

     Running `/home/afonso/git/wasmtime/target/debug/clif-util test ./lmao.clif`
thread 'worker #0' panicked at 'index out of bounds: the len is 16 but the index is 16', cranelift/interpreter/src/step.rs:1565:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR cranelift_filetests::concurrent > FAIL: panicked in worker #0: index out of bounds: the len is 16 but the index is 16
FAIL ./lmao.clif: panicked in worker #0: index out of bounds: the len is 16 but the index is 16
1 tests
Error: 1 failure

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this let me know!

@afonso360 afonso360 added bug Incorrect behavior in the current implementation that needs fixing good first issue Issues that are good for new contributors to tackle! cranelift Issues related to the Cranelift code generator cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! labels Mar 2, 2023
@jameysharp
Copy link
Contributor

Here's the line where the array index is out of bounds. (The line number is different on current main than in the above stack trace.)
https://github.com/bytecodealliance/wasmtime/blob/3ff3994a1/cranelift/interpreter/src/step.rs#L1578

I don't much like the structure of the loop here, but despite that it looks to me like it ought to work. Interesting!

afonso360 added a commit to afonso360/wasmtime that referenced this issue Mar 3, 2023
afonso360 added a commit to afonso360/wasmtime that referenced this issue Mar 9, 2023
afonso360 added a commit that referenced this issue Mar 11, 2023
* fuzzgen: Add some SIMD instructions

* fuzzgen: Remove `scalar_to_vector`

Broken in the interpreter #5911

* fuzzgen: Remove SIMD bitcasts

Broken in the interpreter #5915

* fuzzgen: Fix insert lane

* fuzzgen: Remove debug code

* fuzzgen: Remove vall_true

This is broken in the interpreter #5916

* fuzzgen: Disable a few more ops

* fuzzgen: Remove `iadd_pairwise.i64x2`

Turns out it doesen't exist

* fuzzgen: Remove scalar `sqmul_round_sat`

#5923

* fuzzgen: Disable aligned loads to SIMD values

* fuzzgen: Address Review Feedback

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

* fuzzgen: Rework `cmp` exclusion rules

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
afonso360 added a commit to afonso360/wasmtime that referenced this issue Mar 13, 2023
…alliance#5971)

* fuzzgen: Add some SIMD instructions

* fuzzgen: Remove `scalar_to_vector`

Broken in the interpreter bytecodealliance#5911

* fuzzgen: Remove SIMD bitcasts

Broken in the interpreter bytecodealliance#5915

* fuzzgen: Fix insert lane

* fuzzgen: Remove debug code

* fuzzgen: Remove vall_true

This is broken in the interpreter bytecodealliance#5916

* fuzzgen: Disable a few more ops

* fuzzgen: Remove `iadd_pairwise.i64x2`

Turns out it doesen't exist

* fuzzgen: Remove scalar `sqmul_round_sat`

bytecodealliance#5923

* fuzzgen: Disable aligned loads to SIMD values

* fuzzgen: Address Review Feedback

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

* fuzzgen: Rework `cmp` exclusion rules

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
jan-justin added a commit to jan-justin/wasmtime that referenced this issue May 15, 2023
jan-justin added a commit to jan-justin/wasmtime that referenced this issue May 15, 2023
jan-justin added a commit to jan-justin/wasmtime that referenced this issue May 15, 2023
jan-justin added a commit to jan-justin/wasmtime that referenced this issue May 16, 2023
jan-justin added a commit to jan-justin/wasmtime that referenced this issue May 31, 2023
jan-justin added a commit to jan-justin/wasmtime that referenced this issue Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! cranelift Issues related to the Cranelift code generator good first issue Issues that are good for new contributors to tackle!
Projects
None yet
Development

No branches or pull requests

2 participants