wasmtime / issue #3347 cranelift-fuzzgen: Invalid Craneli... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #3347 cranelift-fuzzgen: Invalid Craneli...

Wasmtime GitHub notifications bot (Sep 13 2021 at 20:41):

alexcrichton opened issue #3347:

Found via oss-fuzz recently this input file when fed into the cranelift-fuzzgen fuzzer as of 4d86f0ca102345555d87eaaa7524acd4260f7607 will generate this panic:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: VerifierErrors([VerifierError { location: inst3, context: Some("jump block2(v2)"), message: "uses value arg from non-dominating block1" }])', wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:45
--
  | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  | AddressSanitizer:DEADLYSIGNAL
  | =================================================================
  | ==916==ERROR: AddressSanitizer: ABRT on unknown address 0x053900000394 (pc 0x7facee43b18b bp 0x7ffe46a99210 sp 0x7ffe46a98fa0 T0)
  | SCARINESS: 10 (signal)
  | #0 0x7facee43b18b in raise /build/glibc-eX1tMB/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:51:1
  | #1 0x7facee41a858 in abort /build/glibc-eX1tMB/glibc-2.31/stdlib/abort.c:79:7
  | #2 0x5652cf6ed936 in std::sys::unix::abort_internal::h810beef058600b40 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys/unix/mod.rs:259:14
  | #3 0x5652cdd4eb15 in std::process::abort::heb2aba170fd673de /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/process.rs:1979:5
  | #4 0x5652cf601a7e in libfuzzer_sys::initialize::_$u7b$u7b$closure$u7d$u7d$::h579d487df9b334a1 /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:57:9
  | #5 0x5652cf6e0c78 in std::panicking::rust_panic_with_hook::hc09e869c4cf00885 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:628:17
  | #6 0x5652cf6e072f in std::panicking::begin_panic_handler::_$u7b$u7b$closure$u7d$u7d$::hc2c6d70142458fc8 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:521:13
  | #7 0x5652cf6dd3a3 in std::sys_common::backtrace::__rust_end_short_backtrace::had58f7c459a1cd6e /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys_common/backtrace.rs:141:18
  | #8 0x5652cf6e0698 in rust_begin_unwind /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:517:5
  | #9 0x5652cdd4fde0 in core::panicking::panic_fmt::hf443e5eeb6cc9eab /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/panicking.rs:96:14
  | #10 0x5652cdd4fed2 in core::result::unwrap_failed::h885d3f7beb571353 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1617:5
  | #11 0x5652cde1b096 in core::result::Result$LT$T$C$E$GT$::unwrap::h63bc4d7eddd801dc /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1299:23
  | #12 0x5652cde1b096 in rust_fuzzer_test_input wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:5
  | #13 0x5652cf601bb8 in __rust_try libfuzzer_sys.7250e05b-cgu.0:0
  | #14 0x5652cf60131f in std::panicking::try::hdb574c15f35ab6b9 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:367:19
  | #15 0x5652cf60131f in std::panic::catch_unwind::he6e3d6241b2b560d /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panic.rs:129:14
  | #16 0x5652cf60131f in LLVMFuzzerTestOneInput /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:28:22
  | #17 0x5652cf637d73 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp:0
  | #18 0x5652cf624a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
  | #19 0x5652cf62a43b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp:0
  | #20 0x5652cf6502e2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
  | #21 0x7facee41c0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
  | #22 0x5652cdd5124d in _start

cc @afonso360, I think you might be interested in this? Sorry I'm not super familiar with this fuzzer so I'm not sure how to reduce further before handing it off here

Wasmtime GitHub notifications bot (Sep 13 2021 at 20:41):

alexcrichton labeled issue #3347:

Found via oss-fuzz recently this input file when fed into the cranelift-fuzzgen fuzzer as of 4d86f0ca102345555d87eaaa7524acd4260f7607 will generate this panic:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: VerifierErrors([VerifierError { location: inst3, context: Some("jump block2(v2)"), message: "uses value arg from non-dominating block1" }])', wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:45
--
  | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  | AddressSanitizer:DEADLYSIGNAL
  | =================================================================
  | ==916==ERROR: AddressSanitizer: ABRT on unknown address 0x053900000394 (pc 0x7facee43b18b bp 0x7ffe46a99210 sp 0x7ffe46a98fa0 T0)
  | SCARINESS: 10 (signal)
  | #0 0x7facee43b18b in raise /build/glibc-eX1tMB/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:51:1
  | #1 0x7facee41a858 in abort /build/glibc-eX1tMB/glibc-2.31/stdlib/abort.c:79:7
  | #2 0x5652cf6ed936 in std::sys::unix::abort_internal::h810beef058600b40 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys/unix/mod.rs:259:14
  | #3 0x5652cdd4eb15 in std::process::abort::heb2aba170fd673de /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/process.rs:1979:5
  | #4 0x5652cf601a7e in libfuzzer_sys::initialize::_$u7b$u7b$closure$u7d$u7d$::h579d487df9b334a1 /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:57:9
  | #5 0x5652cf6e0c78 in std::panicking::rust_panic_with_hook::hc09e869c4cf00885 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:628:17
  | #6 0x5652cf6e072f in std::panicking::begin_panic_handler::_$u7b$u7b$closure$u7d$u7d$::hc2c6d70142458fc8 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:521:13
  | #7 0x5652cf6dd3a3 in std::sys_common::backtrace::__rust_end_short_backtrace::had58f7c459a1cd6e /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys_common/backtrace.rs:141:18
  | #8 0x5652cf6e0698 in rust_begin_unwind /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:517:5
  | #9 0x5652cdd4fde0 in core::panicking::panic_fmt::hf443e5eeb6cc9eab /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/panicking.rs:96:14
  | #10 0x5652cdd4fed2 in core::result::unwrap_failed::h885d3f7beb571353 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1617:5
  | #11 0x5652cde1b096 in core::result::Result$LT$T$C$E$GT$::unwrap::h63bc4d7eddd801dc /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1299:23
  | #12 0x5652cde1b096 in rust_fuzzer_test_input wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:5
  | #13 0x5652cf601bb8 in __rust_try libfuzzer_sys.7250e05b-cgu.0:0
  | #14 0x5652cf60131f in std::panicking::try::hdb574c15f35ab6b9 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:367:19
  | #15 0x5652cf60131f in std::panic::catch_unwind::he6e3d6241b2b560d /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panic.rs:129:14
  | #16 0x5652cf60131f in LLVMFuzzerTestOneInput /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:28:22
  | #17 0x5652cf637d73 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp:0
  | #18 0x5652cf624a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
  | #19 0x5652cf62a43b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp:0
  | #20 0x5652cf6502e2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
  | #21 0x7facee41c0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
  | #22 0x5652cdd5124d in _start

cc @afonso360, I think you might be interested in this? Sorry I'm not super familiar with this fuzzer so I'm not sure how to reduce further before handing it off here

Wasmtime GitHub notifications bot (Sep 13 2021 at 20:41):

alexcrichton edited issue #3347:

Found via oss-fuzz recently this input file when fed into the cranelift-fuzzgen fuzzer as of 4d86f0ca102345555d87eaaa7524acd4260f7607 will generate this panic:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: VerifierErrors([VerifierError { location: inst3, context: Some("jump block2(v2)"), message: "uses value arg from non-dominating block1" }])', wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:45
--
  | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  | AddressSanitizer:DEADLYSIGNAL
  | =================================================================
  | ==916==ERROR: AddressSanitizer: ABRT on unknown address 0x053900000394 (pc 0x7facee43b18b bp 0x7ffe46a99210 sp 0x7ffe46a98fa0 T0)
  | SCARINESS: 10 (signal)
  | #0 0x7facee43b18b in raise /build/glibc-eX1tMB/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:51:1
  | #1 0x7facee41a858 in abort /build/glibc-eX1tMB/glibc-2.31/stdlib/abort.c:79:7
  | #2 0x5652cf6ed936 in std::sys::unix::abort_internal::h810beef058600b40 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys/unix/mod.rs:259:14
  | #3 0x5652cdd4eb15 in std::process::abort::heb2aba170fd673de /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/process.rs:1979:5
  | #4 0x5652cf601a7e in libfuzzer_sys::initialize::_$u7b$u7b$closure$u7d$u7d$::h579d487df9b334a1 /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:57:9
  | #5 0x5652cf6e0c78 in std::panicking::rust_panic_with_hook::hc09e869c4cf00885 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:628:17
  | #6 0x5652cf6e072f in std::panicking::begin_panic_handler::_$u7b$u7b$closure$u7d$u7d$::hc2c6d70142458fc8 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:521:13
  | #7 0x5652cf6dd3a3 in std::sys_common::backtrace::__rust_end_short_backtrace::had58f7c459a1cd6e /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys_common/backtrace.rs:141:18
  | #8 0x5652cf6e0698 in rust_begin_unwind /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:517:5
  | #9 0x5652cdd4fde0 in core::panicking::panic_fmt::hf443e5eeb6cc9eab /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/panicking.rs:96:14
  | #10 0x5652cdd4fed2 in core::result::unwrap_failed::h885d3f7beb571353 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1617:5
  | #11 0x5652cde1b096 in core::result::Result$LT$T$C$E$GT$::unwrap::h63bc4d7eddd801dc /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1299:23
  | #12 0x5652cde1b096 in rust_fuzzer_test_input wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:5
  | #13 0x5652cf601bb8 in __rust_try libfuzzer_sys.7250e05b-cgu.0:0
  | #14 0x5652cf60131f in std::panicking::try::hdb574c15f35ab6b9 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:367:19
  | #15 0x5652cf60131f in std::panic::catch_unwind::he6e3d6241b2b560d /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panic.rs:129:14
  | #16 0x5652cf60131f in LLVMFuzzerTestOneInput /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:28:22
  | #17 0x5652cf637d73 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp:0
  | #18 0x5652cf624a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
  | #19 0x5652cf62a43b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp:0
  | #20 0x5652cf6502e2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
  | #21 0x7facee41c0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
  | #22 0x5652cdd5124d in _start

cc @afonso360, I think you might be interested in this? Sorry I'm not super familiar with this fuzzer so I'm not sure how to reduce further before handing it off here

Wasmtime GitHub notifications bot (Sep 13 2021 at 21:18):

afonso360 commented on issue #3347:

cc @afonso360, I think you might be interested in this? Sorry I'm not super familiar with this fuzzer so I'm not sure how to reduce further before handing it off here

Yeah, ill take a look, thanks!

Wasmtime GitHub notifications bot (Jul 07 2022 at 00:47):

jameysharp commented on issue #3347:

Here's the CLIF generated for that fuzzer input:
function u0:0(b1, i16) system_v {
    jt0 = jump_table [block3, block3, block3, block3, block3, block3, block3, block3, block3, block3, block3, block3, block3, block3, block3]

block0(v0: b1, v1: i16):
    jump block2(v1)

block1(v2: i16):
    br_table v2, block3, jt0

block2(v3: i16):
    br_table v3, block5, jt0

block3:
    jump block2(v2)

block4:
    jump block2(v2)

block5:
    jump block2(v3)

block6:
    jump block2(v3)
}
I notice that the SSA value it's complaining about, v2, is defined in block1. But as the error message says, block1 doesn't dominate block3, which is also reachable through block2. In fact there are no branches to block1.

I'm finding this confusing to reason about because the fuzz target doesn't use SSA values directly, but rather Variables which can be bound to different SSA values at different points.

I'm guessing that if you ensure that the fuzz target makes every block reachable from the function entry, then variables will always be bound to values from dominating blocks. But I could be wrong!

Wasmtime GitHub notifications bot (Jul 07 2022 at 08:51):

afonso360 commented on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks an finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We do only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available. v1 gets picked as v3 in block2.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 08:51):

afonso360 edited a comment on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks and finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We do only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available. v1 gets picked as v3 in block2.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 08:52):

afonso360 edited a comment on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks and finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We do only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available. v1 gets picked as v2 in block1 and v3 in block2.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 08:53):

afonso360 edited a comment on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks and finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We do only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available. v1 gets picked as v3 in block2.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 08:56):

afonso360 edited a comment on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks and finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We do only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available. v1 gets picked as v2 in block1 and v3 in block2.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 08:57):

afonso360 edited a comment on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks and finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We do only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available.

v1 gets picked as v2 in block1 and v3 in block2. The difference being that v2 is part of the signature of block1 when generating and block2 supposedly didn't have any variables in the signature.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 09:13):

afonso360 edited a comment on issue #3347:

Here's what the fuzzer generates before calling: seal_all_blocks and finalize
fn: function u0:0(b1, i16) system_v {
    jt0 = jump_table [block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2, block2]

block0(v0: b1, v1: i16):
    jump block2

block1(v2: i16):
    br_table v2, block2, jt0

block2(v3: i16):
    br_table v3, block2, jt0
}
When generating the function, block2 does not have any params. That's why its picked up as a valid jump_table and br_table target.

However, it looks like that when generating the last br_table instruction it uses a variable from our global variable pool, and that adds a block param somehow?

We only have 2 variables in this example, and they are conveniently bound to v0 and v1 on block0, so they should always be available.

v1 gets picked as v2 in block1 and v3 in block2. The difference being that v2 is part of the signature of block1 when generating and block2 supposedly didn't have any variables in the signature.

I'm not really sure how to investigate this. But it looks like we are doing something wrong when converting to SSA.

Wasmtime GitHub notifications bot (Jul 07 2022 at 18:00):

jameysharp commented on issue #3347:

I think the chain of events leading to that block param is:

generate_br_table calls builder.use_var

the variable doesn't already have a definition in block2, so eventually use_var_nonlocal is called in cranelift_frontend::ssa

block2 isn't sealed, and even if it were it already has multiple predecessors, so no matter what, use_var_nonlocal is going to call func.dfg.append_block_param

I think that makes sense because the SSA builder doesn't know at that point whether there will be another definition added later in some predecessor of this block.

Is it possible to arrange that any block used as the target of a br_table has only the one predecessor, and is sealed before any instructions are added to it?

It looks like other callers always generate a jump table at the same time as the br_table instruction that uses it, so each jump table is used exactly once. Maybe doing the same thing here would help with this bug?

Wasmtime GitHub notifications bot (Jul 07 2022 at 18:23):

cfallin commented on issue #3347:

@jameysharp and I talked briefly offline; it seems the issue is with the handling of unreachable code in the SSA-building algorithms in cranelift-frontend. I suspect the least error-prone approach is to disallow unreachable code to be built with cranelift-frontend (and debug_assert that somewhere). This does imply that embedders using the SSA-builder frontend need to ensure all blocks are reachable (or do a quick DFS to mark reachable blocks and skip unreachable ones first), but it removes a bunch of special cases and resolves bugs as a result.

So I think a good way out here is to:

Check and assert for no unreachable code in the frontend;

Do any cleanup that allows;

Ensure that cranelift-wasm's unreachable-code handling is compatible with this (i.e. does not generate unreachable blocks at the CLIF level);

Modify cranelift-fuzzgen to ensure generated blocks are always reachable.

If we can resolve the bugs in cranelift-frontend arising from unreachable code I'm fine with that outcome as well, but I don't see it as a really necessary case to support at that level.

Wasmtime GitHub notifications bot (Jul 07 2022 at 19:09):

jameysharp commented on issue #3347:

@cfallin and I discussed this a little further and I think there is actually a frontend bug here, and if we can fix that then the current fuzzgen strategy should work.

Adding block parameters during SSA construction should actually be okay, even to blocks which are used in jump tables. There's code in the frontend crate to rewrite the jump table when it does that.

The problem here is that if a jump-table is shared by multiple instructions, then we need to be careful about how we rewrite it. During sealing, block2 should have gotten a different jump table than block1, where all the original references to block2 were rewritten to block5, rather than block3. Then v3 is used when the branch comes from block2, and v2 is used when the branch comes from block1, and the validator should be happy.

I think the simplest way to fix it is that append_jump_argument (in cranelift/frontend/src/ssa.rs) needs to not update func.jump_tables in-place. Instead, it should copy the jump table, and update jump_inst to use the new table.

This may leave unused jump-tables lying around. In fact, since everywhere else uses a jump-table in exactly one instruction, it will leave a lot of unnecessary copies of these tables lying around.

As an optimization, it'd be nice if append_jump_argument could check whether there are any other users of the jump table. I'm having trouble following the control flow, but I think this function is only called after the target block is sealed. Unfortunately I don't think there's any guarantee that other targets in the jump table are also sealed, so I don't think it's safe to assume we've seen all uses of the jump table at that point.

Wasmtime GitHub notifications bot (Jul 07 2022 at 19:56):

bjorn3 commented on issue #3347:

Maybe add a seal_jump_table function that can be called after all instructions using a jump.table have been inserted?

Wasmtime GitHub notifications bot (Jul 07 2022 at 19:56):

bjorn3 edited a comment on issue #3347:

Maybe add a seal_jump_table function that can be called after all instructions using a jump table have been inserted?

Wasmtime GitHub notifications bot (Jul 07 2022 at 20:09):

jameysharp commented on issue #3347:

It's not immediately obvious to me how seal_jump_table would work; do you have suggestions? I think this operation needs to wait not only for all instructions using the jump table to be inserted, but also for all blocks targeted by the jump table to be sealed.

It might be feasible to defer the work that append_jump_argument does until sometime like FunctionBuilder::finalize, but I'm not sure how much complexity that would add.

Alternatively, append_jump_argument could just clone jump tables as needed, and then any unused jump tables could get cleaned up in FunctionBuilder::finalize?

Either way I think it'd be nice to get a patch together that fixes the bug first, then figure out how to optimize it if necessary.

Wasmtime GitHub notifications bot (Jul 07 2022 at 23:20):

bjorn3 commented on issue #3347:

seal_jump_table could set a flag and then when it is actually time to modify jump tablea this flag could be checked to see if in-place modification is possible or not.

Wasmtime GitHub notifications bot (Jul 11 2022 at 20:09):

alexcrichton closed issue #3347:

Found via oss-fuzz recently this input file when fed into the cranelift-fuzzgen fuzzer as of 4d86f0ca102345555d87eaaa7524acd4260f7607 will generate this panic:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: VerifierErrors([VerifierError { location: inst3, context: Some("jump block2(v2)"), message: "uses value arg from non-dominating block1" }])', wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:45
--
  | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  | AddressSanitizer:DEADLYSIGNAL
  | =================================================================
  | ==916==ERROR: AddressSanitizer: ABRT on unknown address 0x053900000394 (pc 0x7facee43b18b bp 0x7ffe46a99210 sp 0x7ffe46a98fa0 T0)
  | SCARINESS: 10 (signal)
  | #0 0x7facee43b18b in raise /build/glibc-eX1tMB/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:51:1
  | #1 0x7facee41a858 in abort /build/glibc-eX1tMB/glibc-2.31/stdlib/abort.c:79:7
  | #2 0x5652cf6ed936 in std::sys::unix::abort_internal::h810beef058600b40 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys/unix/mod.rs:259:14
  | #3 0x5652cdd4eb15 in std::process::abort::heb2aba170fd673de /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/process.rs:1979:5
  | #4 0x5652cf601a7e in libfuzzer_sys::initialize::_$u7b$u7b$closure$u7d$u7d$::h579d487df9b334a1 /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:57:9
  | #5 0x5652cf6e0c78 in std::panicking::rust_panic_with_hook::hc09e869c4cf00885 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:628:17
  | #6 0x5652cf6e072f in std::panicking::begin_panic_handler::_$u7b$u7b$closure$u7d$u7d$::hc2c6d70142458fc8 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:521:13
  | #7 0x5652cf6dd3a3 in std::sys_common::backtrace::__rust_end_short_backtrace::had58f7c459a1cd6e /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/sys_common/backtrace.rs:141:18
  | #8 0x5652cf6e0698 in rust_begin_unwind /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:517:5
  | #9 0x5652cdd4fde0 in core::panicking::panic_fmt::hf443e5eeb6cc9eab /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/panicking.rs:96:14
  | #10 0x5652cdd4fed2 in core::result::unwrap_failed::h885d3f7beb571353 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1617:5
  | #11 0x5652cde1b096 in core::result::Result$LT$T$C$E$GT$::unwrap::h63bc4d7eddd801dc /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/core/src/result.rs:1299:23
  | #12 0x5652cde1b096 in rust_fuzzer_test_input wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen-verify.rs:10:5
  | #13 0x5652cf601bb8 in __rust_try libfuzzer_sys.7250e05b-cgu.0:0
  | #14 0x5652cf60131f in std::panicking::try::hdb574c15f35ab6b9 /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panicking.rs:367:19
  | #15 0x5652cf60131f in std::panic::catch_unwind::he6e3d6241b2b560d /rustc/51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8/library/std/src/panic.rs:129:14
  | #16 0x5652cf60131f in LLVMFuzzerTestOneInput /rust/registry/src/github.com-1ecc6299db9ec823/libfuzzer-sys-0.4.2/src/lib.rs:28:22
  | #17 0x5652cf637d73 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp:0
  | #18 0x5652cf624a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
  | #19 0x5652cf62a43b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp:0
  | #20 0x5652cf6502e2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
  | #21 0x7facee41c0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
  | #22 0x5652cdd5124d in _start

cc @afonso360, I think you might be interested in this? Sorry I'm not super familiar with this fuzzer so I'm not sure how to reduce further before handing it off here

Last updated: Nov 22 2024 at 17:03 UTC