Cranelift: panic in cranelife-codegen-0.77.0 machinst lower.rs:529

[serprex]

Thanks for filing an issue! Please fill out the TODOs below.

.clif Test Case

Is printing the JITModule's context's func the clif result? If so that's included at the end of the stdout output. Can look into narrowing down test case with some guidance about clif-util & how to get clif output (cargo install clif-util doesn't seem to be a thing)

Steps to Reproduce

• git clone https://github.com/serprex/Befunge
• cd to barfs directory
• download https://github.com/catseye/Befunge-93/blob/master/eg/mandel.bf
• cargo run mandel.bf

Expected Results

Receive an error probably, or do whatever it is my code generator is doing (it's WIP, so still buggy itself)

Actual Results

thread 'main' panicked at 'assertion failed: num_args == self.f.dfg.inst_variable_args(inst).len()', .../cranelift-codegen-0.77.0/src/machinst/lower.rs:529:9

Versions and Environment

Cranelift version or commit: 0.77

Operating system: Archlinux

Architecture: x86_64

Extra Info

Anything else you'd like to add?

barfs stdout on mandel.bf: https://pastebin.com/4fLDcktw

Also unrelated but doc for icmp is missing info on output type, "A boolean type with 1 bits."

[cfallin]

That assert is testing that the number of args on a branch instruction matches the number of block parameters on the destination block. Can you verify that these match in your test case for all the branches?

cargo install clif-util doesn't seem to be a thing

I'm not sure if we provide a cargo install-able crate with clif-util, but you can get it by cloning the repo then cargo build --release -p cranelift-tools. (Sorry this isn't more convenient!)

[serprex]

I don't think I'm mismatching parameter counts: the compiler only creates parameter taking blocks at 2 points in the code (both cases taking 1 parameter)

The cargo build command fails with

error: failed to load manifest for workspace member `.../wasmtime/crates/bench-api`

Caused by:
  failed to load manifest for dependency `wasi-cap-std-sync`

Caused by:
  failed to load manifest for dependency `wasi-common`

Caused by:
  failed to load manifest for dependency `wiggle`

Caused by:
  failed to load manifest for dependency `wiggle-macro`

Caused by:
  failed to load manifest for dependency `wiggle-generate`

Caused by:
  failed to load manifest for dependency `witx`

Caused by:
  No such file or directory (os error 2)

cargo version outputs cargo 1.55.0 (32da73ab1 2021-08-23) while rustc -V outputs rustc 1.55.0 (c8dfcfe04 2021-09-06)

Also silly me didn't include RUST_BACKTRACE=1 output

thread 'main' panicked at 'assertion failed: num_args == self.f.dfg.inst_variable_args(inst).len()', /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/machinst/lower.rs:529:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:50:5
   3: cranelift_codegen::machinst::lower::Lower<I>::lower_edge
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/machinst/lower.rs:529:9
   4: cranelift_codegen::machinst::lower::Lower<I>::lower
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/machinst/lower.rs:974:17
   5: cranelift_codegen::machinst::compile::compile
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/machinst/compile.rs:29:9
   6: cranelift_codegen::isa::x64::X64Backend::compile_vcode
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/isa/x64/mod.rs:54:9
   7: <cranelift_codegen::isa::x64::X64Backend as cranelift_codegen::machinst::MachBackend>::compile_function
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/isa/x64/mod.rs:65:21
   8: cranelift_codegen::context::Context::compile
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-codegen-0.77.0/src/context.rs:196:26
   9: <cranelift_jit::backend::JITModule as cranelift_module::module::Module>::define_function
             at /home/erpre/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-jit-0.77.0/src/backend.rs:641:13
  10: barfs::jit::execute
             at ./src/jit.rs:318:2
  11: barfs::main
             at ./src/main.rs:43:10
  12: core::ops::function::FnOnce::call_once
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/ops/function.rs:227:5

[cfallin]

That cargo build failure indicates the git submodules are missing -- try git submodule update --init? (This is possibly git's least intuitive feature ever; sorry and thanks for the patience!).

I don't think I'm mismatching parameter counts: the compiler only creates parameter taking blocks at 2 points in the code (both cases taking 1 parameter)

Interesting; nevertheless, the assert is checking exactly this condition; so either it's hiding somewhere in the input, or being introduced somewhere along the way. A minimized test case input would be very helpful (as a .clif that we can compile without needing your infrastructure), if you're able!

[serprex]

Running clif-util bugpoint found nothing. Was able to compile to x86_64 assembly. Also checked out a1f4b46f64e7c8526dc37bbe220043dff050bf3e (0.77 release commit) & it doesn't raise an error either

I created a clif file by taking the ir output from the pastebin I linked, only clif-util complained about fallthrough not being understood, but it worked once I replaced fallthrough with jump. As an experiment I converted parameterized ins().fallthrough calls in my code to ins().jump but the panic remains

[bjorn3]

572fbc8c5 (the commit tagged as wasmtime v0.30.0, so cranelift version 0.77) does return an error for me. The main branch gives signal SIGFPE: integer divide by zero after replacing all fallthrough with jump as fallthrough is removed on the main branch.

```diff diff --git a/barfs/src/jit.rs b/barfs/src/jit.rs index a88cd46..d313de4 100644 --- a/barfs/src/jit.rs +++ b/barfs/src/jit.rs @@ -103,7 +103,7 @@ pub fn execute( let stidx = builder.use_var(vsidx); let zerocc = builder.ins().iconst(ti64, 0); builder.ins().br_icmp(IntCC::SignedLessThan, stidx, zerocc, zbb, &[]); - builder.ins().fallthrough(bbpop, &[]); + builder.ins().jump(bbpop, &[]); builder.switch_to_block(bbpop); let four = builder.ins().iconst(ti64, 4); let newstidx = builder.ins().isub(stidx, four); @@ -114,7 +114,7 @@ pub fn execute( builder.ins().jump(bb, &[loadres]); builder.switch_to_block(zbb); let zero = builder.ins().iconst(ti32, 0); - builder.ins().fallthrough(bb, &[zero]); + builder.ins().jump(bb, &[zero]); builder.switch_to_block(bb); builder.def_var(var, builder.block_params(bb)[0]); }; @@ -138,7 +138,7 @@ pub fn execute( let newbb = builder.create_block(); if !builder.is_filled() { - builder.ins().fallthrough(newbb, &[]); + builder.ins().jump(newbb, &[]); } builder.switch_to_block(newbb); bbmap.insert(n, newbb); @@ -225,13 +225,13 @@ pub fn execute( builder.append_block_param(bb, ti32); let zero = builder.ins().iconst(ti32, 0); builder.ins().br_icmp(IntCC::UnsignedLessThan, ab, twofivesixzero, bb, &[zero]); - builder.ins().fallthrough(idxbb, &[]); + builder.ins().jump(idxbb, &[]); builder.switch_to_block(idxbb); let ab = builder.ins().uextend(ti64, ab); let vcode = builder.ins().iconst(tptr, code.as_ptr() as i64); let vcodeab = builder.ins().iadd(vcode, ab); let result = builder.ins().load(ti32, aligned, vcodeab, 0); - builder.ins().fallthrough(bb, &[result]); + builder.ins().jump(bb, &[result]); builder.switch_to_block(bb); let val = builder.block_params(bb)[0]; clpush(&mut builder, val); @@ -248,7 +248,7 @@ pub fn execute( let b25 = builder.ins().icmp_imm(IntCC::UnsignedLessThan, b, 25); let ab8025 = builder.ins().band(a80, b25); builder.ins().brz(ab8025, bb, &[]); - builder.ins().fallthrough(bbwrite, &[]); + builder.ins().jump(bbwrite, &[]); builder.switch_to_block(bbwrite); let five = builder.ins().iconst(ti32, 5); let a5 = builder.ins().ishl(a, five); @@ -258,7 +258,7 @@ pub fn execute( let vcodeab = builder.ins().iadd(vcode, ab); let c = builder.use_var(tc); builder.ins().store(aligned, c, vcodeab, 0); - builder.ins().fallthrough(bb, &[]); + builder.ins().jump(bb, &[]); builder.switch_to_block(bb); } Op::Jr(r0, r1, r2) => { @@ -272,7 +272,7 @@ pub fn execute( let j = builder.ins().brz(a, Block::from_u32(0), &[]); jumpmap.push((j, rz)); compstack.push(rz); - let j = builder.ins().fallthrough(Block::from_u32(0), &[]); + let j = builder.ins().jump(Block::from_u32(0), &[]); jumpmap.push((j, op.n)); } Op::Ret => { ```

[bjorn3]

Replacing all fallthrough with jump also fixes the issue for cranelift 0.77. The issue is that a fallthrough instruction actually specifies a destination that doesn't come directly after the fallthrough:

block1035(v4902: i32, v4950: i64):
    v4911 -> v4950
    brz v4902, block1036
    fallthrough block266(v4950)

block1036:
    v4910 = iconst.i32 35
    v4912 = iconst.i64 4
    v4913 = iadd.i64 v4911, v4912
    v4914 = iconst.i64 0x7fff_ffff_9698
    v4915 = iadd v4914, v4913
    store aligned v4910, v4915
    v4917 = iconst.i64 0
    br_icmp slt v4913, v4917, block1038
    fallthrough block1037

block266 doesn't follow block1035. If it weren't for the fact that falthrough has been removed anyway it would be nice to have a verifier error for this.

[serprex]

The floating point error is likely a division by zero in my buggy generated code. Looks like this is effectively fixed in master

Thanks

[cfallin]

Ah, thank you for finding the issue, @bjorn3! In hindsight I also should have suggested enabling the CLIF verifier -- this should have also found the issue with the erroneous fallthrough. Possibly a useful tip for any future issues with your generated CLIF :-)