Stream: git-wasmtime

Topic: wasmtime / issue #3217 Cranelift: bitcasting reference-ty...

Wasmtime GitHub notifications bot (Aug 20 2021 at 02:47):

cfallin edited issue #3217:

For the below .clif code Cranelift appears to pass unsolvable input to regalloc. This was previously investigated as an issue with regalloc, but after some investigation @cfallin suggested moving it here.

This has been observed to cause failures when using regalloc and the upcoming regalloc2.

.clif Test Case

function u0:0() -> r64 system_v {
    sig0 = (i64) -> r64 system_v
    sig1 = (r64, r64, r64) -> r64 system_v
    sig2 = (i64) -> r64 system_v
    sig3 = (i64) -> r64 system_v
    sig4 = (r64, r64, r64) -> r64 system_v
    sig5 = (r64, r64, r64) -> r64 system_v
    fn0 = colocated u0:2 sig0
    fn1 = colocated u0:5 sig1
    fn2 = colocated u0:2 sig2
    fn3 = colocated u0:2 sig3

block0:
    v0 = iconst.i64 8
    v1 = call fn0(v0)
    v2 = func_addr.r64 fn1
    store v2, v1
    v3 = raw_bitcast.i64 v1
    v4 = bor_imm v3, 6
    v5 = raw_bitcast.r64 v4
    v6 = raw_bitcast.i64 v5
    v7 = band_imm v6, -8
    v8 = raw_bitcast.r64 v7
    v9 = load.r64 v8
    v10 = iconst.i64 1
    v11 = raw_bitcast.r64 v10
    v12 = iconst.i64 8
    v13 = call fn2(v12)
    v14 = raw_bitcast.i64 v5
    v15 = band_imm v14, -8
    v16 = raw_bitcast.r64 v15
    v17 = load.r64 v16
    v18 = iconst.i64 1
    v19 = raw_bitcast.r64 v18
    v20 = iconst.i64 8
    v21 = call fn3(v20)
    v22 = iconst.i64 8
    v23 = raw_bitcast.r64 v22
    store v23, v21
    v24 = call_indirect sig4, v17(v16, v19, v21)
    store v24, v13
    v25 = call_indirect sig5, v9(v8, v11, v13)
    return v25
}

Steps to Reproduce

1. Place .clif test case above in a file fn.clif
2. Run cargo run -p cranelift-tools -- compile --target x86_64 -D fn.clif and observe a panic.

thread 'main' panicked at 'assertion failed: vlr_env[cand_vlrix].is_ref == is_ref', /Users/zekemedley/.cargo/registry/src/github.com-1ecc6299db9ec823/regalloc-0.0.31/src/bt_spillslot_allocator.rs:298:13

Reproduction with regalloc2

1. Place .clif test case above in a file fn.clif
2. Follow the insutructions here for compiling Cranelift with regalloc2.
3. Run cargo run -p cranelift-tools -- compile --target x86_64 -D fn.clif and observe a panic.

thread 'main' panicked at 'Minimal bundle with conflict!', /Users/zekemedley/Desktop/projects/lust/regalloc-bug/regalloc2/src/ion/process.rs:763:13

Wasmtime GitHub notifications bot (Aug 20 2021 at 02:48):

cfallin labeled issue #3217:

For the below .clif code Cranelift appears to pass unsolvable input to regalloc. This was previously investigated as an issue with regalloc, but after some investigation @cfallin suggested moving it here.

This has been observed to cause failures when using regalloc and the upcoming regalloc2.

.clif Test Case

function u0:0() -> r64 system_v {
    sig0 = (i64) -> r64 system_v
    sig1 = (r64, r64, r64) -> r64 system_v
    sig2 = (i64) -> r64 system_v
    sig3 = (i64) -> r64 system_v
    sig4 = (r64, r64, r64) -> r64 system_v
    sig5 = (r64, r64, r64) -> r64 system_v
    fn0 = colocated u0:2 sig0
    fn1 = colocated u0:5 sig1
    fn2 = colocated u0:2 sig2
    fn3 = colocated u0:2 sig3

block0:
    v0 = iconst.i64 8
    v1 = call fn0(v0)
    v2 = func_addr.r64 fn1
    store v2, v1
    v3 = raw_bitcast.i64 v1
    v4 = bor_imm v3, 6
    v5 = raw_bitcast.r64 v4
    v6 = raw_bitcast.i64 v5
    v7 = band_imm v6, -8
    v8 = raw_bitcast.r64 v7
    v9 = load.r64 v8
    v10 = iconst.i64 1
    v11 = raw_bitcast.r64 v10
    v12 = iconst.i64 8
    v13 = call fn2(v12)
    v14 = raw_bitcast.i64 v5
    v15 = band_imm v14, -8
    v16 = raw_bitcast.r64 v15
    v17 = load.r64 v16
    v18 = iconst.i64 1
    v19 = raw_bitcast.r64 v18
    v20 = iconst.i64 8
    v21 = call fn3(v20)
    v22 = iconst.i64 8
    v23 = raw_bitcast.r64 v22
    store v23, v21
    v24 = call_indirect sig4, v17(v16, v19, v21)
    store v24, v13
    v25 = call_indirect sig5, v9(v8, v11, v13)
    return v25
}

Steps to Reproduce

1. Place .clif test case above in a file fn.clif
2. Run cargo run -p cranelift-tools -- compile --target x86_64 -D fn.clif and observe a panic.

thread 'main' panicked at 'assertion failed: vlr_env[cand_vlrix].is_ref == is_ref', /Users/zekemedley/.cargo/registry/src/github.com-1ecc6299db9ec823/regalloc-0.0.31/src/bt_spillslot_allocator.rs:298:13

Reproduction with regalloc2

1. Place .clif test case above in a file fn.clif
2. Follow the insutructions here for compiling Cranelift with regalloc2.
3. Run cargo run -p cranelift-tools -- compile --target x86_64 -D fn.clif and observe a panic.

thread 'main' panicked at 'Minimal bundle with conflict!', /Users/zekemedley/Desktop/projects/lust/regalloc-bug/regalloc2/src/ion/process.rs:763:13

Wasmtime GitHub notifications bot (Sep 14 2021 at 20:50):

wrbs commented on issue #3217:

I ran in to this too for my projeect which similarly used bitcasting to turn r64 <-> i64`. I ended up vendoring regalloc and commenting out the assert to not be there. I did not attempt to understand why this worked.

It was a JIT backend to OCaml which has a precise GC which needs to know every root's location otherwise things will very quickly break (as I encountered many times).

Thanks to implementing all of OCaml I had a very large 20-year-old test suite to check against. I also had long-running benchmarks doing lots of garbage collections and no odd breakages due to miscompiles.

Wasmtime GitHub notifications bot (Sep 14 2021 at 20:52):

wrbs edited a comment on issue #3217:

I ran in to this too for my project which similarly used bitcasting to turn r64 <-> i64`. I ended up vendoring regalloc and commenting out the assert to not be there. I did not attempt to understand why this worked.

It was a JIT backend to OCaml which has a precise GC which needs to know every root's location otherwise things will very quickly break (as I encountered many times).

Thanks to implementing all of OCaml I had a very large 20-year-old test suite to check against. I also had long-running benchmarks doing lots of garbage collections and no odd breakages due to miscompiles.

Wasmtime GitHub notifications bot (Sep 14 2021 at 20:53):

wrbs edited a comment on issue #3217:

I ran in to this too for my project which similarly used bitcasting to turn r64 <-> i64`. I ended up vendoring regalloc and commenting out the assert to not be there. I did not attempt to understand why this worked (or rather, I did and very quickly realised I would not have the week required to get my head around how everything worked.) When I brought it up I think I remember @cfallin saying to wait for the rewrite of regalloc which seems to have happened now?

It was a JIT backend to OCaml which has a precise GC which needs to know every root's location otherwise things will very quickly break (as I encountered many times).

Thanks to implementing all of OCaml I had a very large 20-year-old test suite to check against. I also had long-running benchmarks doing lots of garbage collections and no odd breakages due to miscompiles.

Wasmtime GitHub notifications bot (Sep 14 2021 at 21:26):

bjorn3 commented on issue #3217:

I did not attempt to understand why this worked

I am fairly certain it doesn't actually fix the problem, but instead allows a silent miscompilation where the stackmap may be missing entries.

Wasmtime GitHub notifications bot (Sep 14 2021 at 21:27):

bjorn3 edited a comment on issue #3217:

I did not attempt to understand why this worked

I am fairly certain it doesn't actually fix the problem, but instead allows a silent miscompilation where the stackmap may be missing entries or have erroneous entries.

Wasmtime GitHub notifications bot (Sep 14 2021 at 21:31):

fitzgen commented on issue #3217:

In general, I think we should be adding support for using r{32,64} anywhere that i{32,64} can be used (e.g. in loads/stores/etc) this way we can always generate correct stack maps, and users don't have to do unsafe bitcasts that can result in missing stack map entries, as @bjorn3 points out.

Wasmtime GitHub notifications bot (Sep 20 2021 at 16:35):

wrbs commented on issue #3217:

I did not attempt to understand why this worked

I am fairly certain it doesn't actually fix the problem, but instead allows a silent miscompilation where the stackmap may be missing entries or have erroneous entries.

Actually thinking back a little bit more this checks out - there was one error I didn't track down, but only for one benchmark just after a GC run. I I suspected at the time it might have been due to this

Last updated: Nov 22 2024 at 16:03 UTC