wasmtime / issue #4758 cranelift-fuzzgen fuzzbug: "interp... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #4758 cranelift-fuzzgen fuzzbug: "interp...

Wasmtime GitHub notifications bot (Aug 23 2022 at 15:51):

OSS-Fuzz: https://oss-fuzz.com/testcase-detail/4625103710715904

thread '<unnamed>' panicked at 'interpreter failed: Error(StepError(UnknownFunction(fn0)))', [wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen.rs:93](https://github.com/bytecodealliance/wasmtime/blob/d620705a323e3da59bd90473b4e627c8502b1255/fuzz/fuzz_targets/cranelift-fuzzgen.rs#L93):36

input:

/////wH/////AP8B64L/

cc @afonso360

Wasmtime GitHub notifications bot (Aug 23 2022 at 16:17):

afonso360 commented on issue #4758:

Formatted:

ubuntu@instance-20220805-0848:~/git/wasmtime/fuzz$ cargo fuzz fmt cranelift-fuzzgen ./4758.in --no-default-features

Output of `std::fmt::Debug`:

;; Fuzzgen test case

test interpret
test run
set enable_llvm_abi_extensions
target aarch64
target s390x
target x86_64

function u0:1() system_v {
    sig0 = () system_v
    fn0 = colocated u0:0 sig0

block0:
    v0 = iconst.i128 0
    v1 = iconst.i64 0
    v2 = iconst.i32 0
    v3 = iconst.i16 0
    v4 = iconst.i8 0
    call fn0()
    return
}

; Note: the results in the below test cases are simply a placeholder and probably will be wrong

; run: u0:1()
; run: u0:1()
; run: u0:1()
; run: u0:1()
; run: u0:1()
; run: u0:1()

I think this was introduced in #4155, and at the time I didn't think it would cause issues. I think the optimal fix for this is #4667 and subsequently adding support for generating multiple functions in fuzzgen (ensuring that we only produce valid references).

That way we don't have to disable important features for the cranelift-icache fuzzer.

Wasmtime GitHub notifications bot (Aug 23 2022 at 16:17):

afonso360 edited a comment on issue #4758:

Formatted:

ubuntu@instance-20220805-0848:~/git/wasmtime/fuzz$ cargo fuzz fmt cranelift-fuzzgen ./4758.in --no-default-features

Output of `std::fmt::Debug`:

;; Fuzzgen test case

test interpret
test run
set enable_llvm_abi_extensions
target aarch64
target s390x
target x86_64

function u0:1() system_v {
    sig0 = () system_v
    fn0 = colocated u0:0 sig0

block0:
    v0 = iconst.i128 0
    v1 = iconst.i64 0
    v2 = iconst.i32 0
    v3 = iconst.i16 0
    v4 = iconst.i8 0
    call fn0()
    return
}

; Note: the results in the below test cases are simply a placeholder and probably will be wrong

; run: u0:1()
; run: u0:1()
; run: u0:1()
; run: u0:1()
; run: u0:1()
; run: u0:1()

That way we don't have to disable important features for the cranelift-icache fuzzer.

Wasmtime GitHub notifications bot (Aug 23 2022 at 16:23):

jameysharp commented on issue #4758:

Oh! I was just looking at that part of the function generator last week, and wondering how that ever worked. I didn't realize it was introduced in the icache PR.

Wasmtime GitHub notifications bot (Aug 25 2022 at 16:50):

jameysharp commented on issue #4758:

@bnjbvr and @cfallin: The incremental cache PR added support to cranelift_fuzzgen for generating random function calls. This is fine for the icache fuzz target which only compiles functions, so it doesn't care if the target of the call exists. But the fuzzgen target tries to run the generated functions so it needs the callee to exist.

How important is testing function calls to our confidence that the incremental cache is working? If we just turn off that part of the function generator, would that significantly reduce our confidence that we can detect bugs in the incremental cache?

Maybe we can parameterize the function generator over a list of other functions and libcalls that it can use. In the fuzzgen target we'd currently pass an empty list, and as work like #4667 develops we can add support there later. Then we'd move the current implementation of generate_funcrefs into the icache target and pass its output into the function generator. Does that seem sensible?

Wasmtime GitHub notifications bot (Aug 25 2022 at 16:52):

bjorn3 commented on issue #4758:

How important is testing function calls to our confidence that the incremental cache is working?

One of my main worries with a previous version of the incremental cache PR was that it would misbehave around function calls, so I did say pretty important.

Wasmtime GitHub notifications bot (Aug 25 2022 at 16:59):

afonso360 commented on issue #4758:

I think we should be able to disable function calls by altering the funcrefs_per_func config on the fuzzer. I think if we don't generate function references we also never generate call's. And its already a control that's built in so we just need to alter it slightly

Wasmtime GitHub notifications bot (Aug 25 2022 at 17:00):

cfallin commented on issue #4758:

Yes, one of two (IIRC?) ways that we are "resilient" in the incremental cache is when specific function references change but the shape of the CLIF remains the same; so we definitely want to test this. Luckily it sounds from above like it's pretty easy to parameterize this differently in different fuzz targets!

Wasmtime GitHub notifications bot (Aug 25 2022 at 17:01):

afonso360 commented on issue #4758:

This has the same issue as the other PR's that it changes the input format, so we won't be able to merge it, but its a pretty easy solution for now.

Wasmtime GitHub notifications bot (Aug 29 2022 at 21:30):

jameysharp closed issue #4758:

OSS-Fuzz: https://oss-fuzz.com/testcase-detail/4625103710715904

thread '<unnamed>' panicked at 'interpreter failed: Error(StepError(UnknownFunction(fn0)))', [wasmtime/fuzz/fuzz_targets/cranelift-fuzzgen.rs:93](https://github.com/bytecodealliance/wasmtime/blob/d620705a323e3da59bd90473b4e627c8502b1255/fuzz/fuzz_targets/cranelift-fuzzgen.rs#L93):36

input:

/////wH/////AP8B64L/

cc @afonso360

Last updated: Nov 22 2024 at 16:03 UTC