fuzzgen: Refactor name and signature generation

[afonso360]

👋 Hey,

This PR includes a few refactors to fuzzgen in order to make it easier to generate testcases with multiple functions.

The first part (first 2 commits) are sort of centralizing generation of some cranelift data structures into a trait for arbitrary.

It also moves name, signature and function references generation out of FunctionGenerator and into FuzzGen. This will allow us to control the call flow graph and avoid recursion and infinite loops when we do generate multiple functions.

This PR by itself gets us generating calls to LibCalls on fuzzgen since we no longer just outright block funcref generation.

Ran this in the fuzzer for a while and nothing came up.

[jameysharp]

This looks promising!

I think I'd prefer configuring a probability of selecting each libcall, rather than configuring a number of calls. I'd rather have each one appear either zero or one times, instead of potentially many times. Or perhaps we should always pass in the full list of libcalls; what test coverage do we gain by randomly removing libcalls from the list?

It might be nice to only add function declarations to builder.func if they're actually used, but that's a separate matter.

I spent a while trying to figure out if this PR makes it any easier to move the list of ALLOWED_LIBCALLS to cranelift-fuzzgen so we don't have to keep it in sync across different modules with the list of supported interpreter implementations. I didn't come up with a good answer, but it's something I'd like to see, if you happen to have any ideas.

[afonso360]

Or perhaps we should always pass in the full list of libcalls; what test coverage do we gain by randomly removing libcalls from the list?

This is what ended up happening anyway in #5765, I'm going to backport that. I don't think we lose anything by doing it that way and its a bit cleaner.

It might be nice to only add function declarations to builder.func if they're actually used, but that's a separate matter.

Now that you mention it, that sounds like a great idea! And it solves one of the issues in #5765 which is that, the function "headers" become quite verbose if we pass in too many call targets which is annoying because they currently don't get reduced so even the smallest function possible still has 6 lines of signatures + 6 lines of function references!

I'm going to give this a go. Would you prefer that as a separate PR or should I bundle it into this one?

I spent a while trying to figure out if this PR makes it any easier to move the list of ALLOWED_LIBCALLS to cranelift-fuzzgen so we don't have to keep it in sync across different modules with the list of supported interpreter implementations. I didn't come up with a good answer, but it's something I'd like to see, if you happen to have any ideas.

The only thing I can think of would be to make Config a associated constant of TestCase, doing a generic arbitrary impl for that and having something like

const CONFIG = Config {
  allowed_libcalls: &'static [
   ...etc.
  ],
  ..Config::default()
}

fuzz_target!(|testcase: TestCase<CONFIG>| ..)

I think this could work, but I'm not sure if we can const eval that?

[jameysharp]

Regarding cleaning up the function header, I'm happy whether that's in this PR or separate. I'd take this PR even if you don't get around to that. So pick whichever is more convenient for you.

When I look at the associated constant idea, I wonder: why do we export types like TestCase from the cranelift-fuzzgen crate?

If we declared that struct in the cranelift-fuzzgen target instead, we just need to provide implementations of Arbitrary and Debug along with it. The current Arbitrary implementation is very short because it mostly delegates to the FuzzGen struct, and if we put that in the fuzz target then we have the opportunity to tweak the config there. The FuzzGen::generate_host_test method probably should get inlined in the implementation of Arbitrary. But the lower-level methods (generate_flags, generate_func, generate_test_inputs) are sensible things to share across multiple fuzz targets.

I think the same plan works for FunctionWithIsa too. Its current Arbitrary implementation is more complicated but I think all the complexity is reasonable to move to the cranelift-icache fuzz target.

I'd prefer if both Debug implementations call a shared helper from the fuzzgen crate too, for printing the target/flags/function body. Really, I want that even if we don't move these structs to the fuzz targets.

Again, though, none of that needs to happen in this PR.

[afonso360]

I've changed this to always pass all libcalls.

I like that idea of having each target defining its own Arbitrary type. I'll make that change as a separate PR!

Same thing with the function header cleanup, I'll do that in a separate PR.

[jameysharp]

Looks good, thank you!