cranelift-fuzzgen fuzzbug: "interpreter failed: Error(StepError(UnknownFunction(fn0)))"

[cfallin]

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

[afonso360]

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 (and ensuring that we only produce valid references).

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

[jameysharp]

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.

[jameysharp]

@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?

[bjorn3]

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.

[afonso360]

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

[cfallin]

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!

[afonso360]

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.