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.

Sign up for GitHub

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

Jump to bottom

Panic in Cranelift when compiling tail calls #8704

Closed
alexcrichton opened this issue May 29, 2024 · 3 comments · Fixed by #8709
Closed

Panic in Cranelift when compiling tail calls #8704

alexcrichton opened this issue May 29, 2024 · 3 comments · Fixed by #8709
Labels
fuzz-bug Bugs found by a fuzzer

Comments

@alexcrichton
Copy link
Member

This input:

foo.wat 
(module
  (type (;0;) (func (result f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32)))
  (type (;1;) (func (result f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32)))
  (type (;2;) (func (result f32 f32 f32 f32)))
  (func (;0;) (type 1) (result f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32)
    unreachable
  )
  (func (;1;) (type 1) (result f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32)
    unreachable
  )
  (func (;2;) (type 1) (result f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32)
    unreachable
  )
  (func (;3;) (type 2) (result f32 f32 f32 f32)
    global.get 0
    block (type 0) (result f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32 f32) ;; label = @1
      call 0
      f32.ne
      f32.load align=1
      f32.lt
      f32.load align=1
      f32.lt
      f32.load align=1
      f32.lt
      f32.load align=1
      f32.ne
      f64.const 0x1.55f3436665f63p-504 (;=0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000025503814566781667;)
      f64.ceil
      i32.trunc_sat_f64_u
      f32.convert_i32_s
      f32.ceil
      f32.ceil
      f32.ceil
      f32.ceil
      f32.floor
      f32.ceil
      f32.floor
      i64.trunc_sat_f32_s
      global.get 1
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load16_s align=1
      i32.clz
      f32.const 0x1.eabe68p-63 (;=0.00000000000000000020783807;)
      f32.floor
      i64.trunc_sat_f32_s
      global.get 1
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.load8_u
      i32.ctz
      f32.const 0x1.e8c2e6p+63 (;=17609482000000000000;)
      f32.floor
      call 2
      f32.lt
      f32.const 0x1.40e8e6p-31 (;=0.00000000058373145;)
      f32.floor
      f32.const 0x1.5c686cp+71 (;=3213495700000000000000;)
      f32.copysign
      f32.const 0x1.606a68p-31 (;=0.00000000064104033;)
      f32.gt
      i64.load16_u align=1
      f64.const 0x1.030303030303p-252 (;=0.0000000000000000000000000000000000000000000000000000000000000000000000000001398043286095289;)
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f64.ceil
      f32.const 0x1.5c686cp+105 (;=55207435000000000000000000000000;)
      global.get 0
      i64.trunc_sat_f64_s
      call 2
      f32.gt
      memory.grow
      f64.const 0x1.030303030302p-252 (;=0.0000000000000000000000000000000000000000000000000000000000000000000000000001398043286095284;)
      f64.neg
      f64.neg
      f64.neg
      f64.neg
      f64.neg
      f64.neg
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      return_call 1
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      drop
      f32.const 0x1.60606p-31 (;=0.00000000064096906;)
      f32.const 0x1.d2406p-19 (;=0.0000034738441;)
      f32.const 0x1.e85c68p+101 (;=4836492700000000000000000000000;)
      f32.const 0x1.c6dceap+63 (;=16388165000000000000;)
      f32.const 0x1.686cccp+63 (;=12985679000000000000;)
      f32.const 0x1.cc40eap-19 (;=0.0000034291563;)
    end
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    drop
    f32.const 0x1.dcdec6p+103 (;=18890775000000000000000000000000;)
    f32.const 0x1.6040e8p-31 (;=0.00000000064074546;)
    f32.const 0x1.60606p-25 (;=0.00000004102202;)
    f32.const 0x1.60606p-31 (;=0.00000000064096906;)
  )
  (memory (;0;) 0)
  (global (;0;) f64 f64.const 0x1.b0a2b292b292bp-365 (;=0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000022487118447499093;))
  (global (;1;) (mut i32) i32.const 0)
)

Fails with:

$ cargo run -q compile ~/Downloads/clusterfuzz-testcase-minimized-compile-4912239181103104
thread '<unnamed>' panicked at cranelift/codegen/src/machinst/abi.rs:2281:55:
if the tail callee has a return pointer, then the tail caller must as well
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Local bisection points to #8389 as the cause, but the panic was added in that commit as well so it may not be the original source.

cc @fitzgen, @elliottt
@alexcrichton alexcrichton added the fuzz-bug Bugs found by a fuzzer label May 29, 2024
@alexcrichton
Copy link
Member Author

My local bisection pointing to #8389 was done with the arm64 backend. Doing another bisection with the x64 backend points to #6774 so I don't think this is a regression, it's just been a bug all along.

@elliottt
Copy link
Member

It looks like this is invalid wasm: the function 3 tail calls to 1, but they differ in return type. Validating in the browser shows this as well:

CompileError: WebAssembly.Module(): Compiling function #3 failed: return_call: tail call type error @+366
    at wasm (<anonymous>:7:10)
    at <anonymous>:1:1

@fitzgen
Copy link
Member

fitzgen commented May 29, 2024

I have a fix incoming.

fitzgen added a commit to fitzgen/wasm-tools that referenced this issue May 29, 2024
@fitzgen
wasmparser: Fix validation of the return_call family of instructions
6f9b07b 
We need to additionally check that the callee's results are an exact match of
the caller's results. We were incorrectly allowing return calls that would push
more values on the operand stack than would be returned. That is fine with a
`call; return` sequence, where extra values on the stack are allowed to dangle,
but not okay with a `return_call`. With a `return_call` it doesn't make sense
because the callee might need a return pointer to put all its results into, but
the caller can't supply one since its frame is going away, nor can the caller
forward a return pointer that it received to the callee, since it might not
return enough values to require a return pointer. This commit fixes the
validation to match the spec and disallow `return_call`s that would leave
dangling values on the operand stack.

cc bytecodealliance/wasmtime#8704
@fitzgen fitzgen mentioned this issue May 29, 2024
Merged
fitzgen added a commit to fitzgen/wasm-tools that referenced this issue May 29, 2024
@fitzgen @elliottt
wasmparser: Fix validation of the return_call family of instructions
d7eabc4 
We need to additionally check that the callee's results are an exact match of
the caller's results. We were incorrectly allowing return calls that would push
more values on the operand stack than would be returned. That is fine with a
`call; return` sequence, where extra values on the stack are allowed to dangle,
but not okay with a `return_call`. With a `return_call` it doesn't make sense
because the callee might need a return pointer to put all its results into, but
the caller can't supply one since its frame is going away, nor can the caller
forward a return pointer that it received to the callee, since it might not
return enough values to require a return pointer. This commit fixes the
validation to match the spec and disallow `return_call`s that would leave
dangling values on the operand stack.

cc bytecodealliance/wasmtime#8704

Co-Authored-By: Trevor Elliott <[email protected]>
github-merge-queue bot pushed a commit to bytecodealliance/wasm-tools that referenced this issue May 29, 2024
@fitzgen @elliottt
wasmparser: Fix validation of the return_call family of instructions (
80dc262 
#1585)

We need to additionally check that the callee's results are an exact match of
the caller's results. We were incorrectly allowing return calls that would push
more values on the operand stack than would be returned. That is fine with a
`call; return` sequence, where extra values on the stack are allowed to dangle,
but not okay with a `return_call`. With a `return_call` it doesn't make sense
because the callee might need a return pointer to put all its results into, but
the caller can't supply one since its frame is going away, nor can the caller
forward a return pointer that it received to the callee, since it might not
return enough values to require a return pointer. This commit fixes the
validation to match the spec and disallow `return_call`s that would leave
dangling values on the operand stack.

cc bytecodealliance/wasmtime#8704

Co-authored-by: Trevor Elliott <[email protected]>
fitzgen added a commit to fitzgen/wasmtime that referenced this issue May 29, 2024
@fitzgen
Update the wasm-tools family of crates to 0.209.1
e02967d 
Fixes bytecodealliance#8704
@fitzgen fitzgen mentioned this issue May 29, 2024
Merged
fitzgen added a commit to fitzgen/wasmtime that referenced this issue May 29, 2024
@fitzgen
Update the wasm-tools family of crates to 0.209.1
e7fbdff 
Fixes bytecodealliance#8704
github-merge-queue bot pushed a commit that referenced this issue May 29, 2024
@fitzgen
Update the wasm-tools family of crates to 0.209.1 (#8709)
38bc915 
Fixes #8704
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fuzz-bug Bugs found by a fuzzer
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update the wasm-tools family of crates to 0.209.1 fitzgen/wasmtime
3 participants
@elliottt @alexcrichton @fitzgen