fuzzgen: Generate compiler flags

[afonso360]

👋 Hey,

This PR allows fuzzgen to generate compiler flags and test using those. We only allow compiler flags that shouldn't change the behaviour of the code.

I wanted to submit this now so that we could fuzz PR's such as #4953 and #5004 that require optimizations enabled.

However, when testing this it has found an issue with one of the passes (I haven't yet narrowed it down), but I don't want to start fixing it if we are going to change pretty much everything once #4953 lands. I'm opening this as a draft, and lets re test this once #4953 lands.

The actual bug manifests as following:

Fails with:

target/aarch64-unknown-linux-gnu/release/cranelift-fuzzgen: Running 1 inputs 1 time(s) each.
Running: fuzz/artifacts/cranelift-fuzzgen/crash-00e37c2858c35c573ed85d618ea5d0c75ae06fda
thread '<unnamed>' panicked at 'assertion failed: `(left != right)`
  left: `v137`,
 right: `v137`: Aliasing v137 to v134 would create a loop', cranelift/codegen/src/ir/dfg.rs:322:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

And I cannot reproduce via the CLIF file since that runs into #4875 (comment) (which I kinda forgot about 😅 ). But I'm going to try and investigate that one in the mean time.

This also enables regalloc_checker as we discussed in #4979.

cc: @jameysharp

[github-actions]

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "cranelift", "cranelift:meta", "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[afonso360]

This PR had some performance issues:

• The regalloc checker started consuming a bunch of time (around 45%) so we now try to minimize runs with it (10% with the current iteration).

• I've also had to limit the amount of runs that we allow, at some point the fuzzer decided that spending 100% of the time in the interpreter was a good idea, and started generating huge inputs, that were in the order of 20k runs each. (up to 130k!)

execs/s seems way better than before, which is nice.

And for those who are interested, here's a flamegraph of where we spend time. This is about 2min at 1Khz around 190k inputs into my normal benchmark run:

Edit: I should note, this is all on top of #5034

[cfallin]

Generally looks great, thanks! Some thoughts below on specifics but I'm happy to see this landed whenever you think it's appropriate to actually start fuzzing.

[cfallin]

Does it make sense to add riscv64 here too (or are you worried it would find too many issues)?

Actually in the spirit of your enum .all() method above -- perhaps we can define a cranelift_codegen::isa::all() that returns all Triples compiled into the build, and use that here? (Feel free to drop that idea though if it's more than ~20 lines of complexity or so.)

[afonso360]

Yeah the all() idea sounds good, I've been adding them where it think they would be useful for others.

[afonso360]

This ended up not being very easy to do, I might try it again later. But either way I've added riscv64 to the target list.

[afonso360]

I've given this one round of fuzzing on AArch64 and x86, and it found the two issues with iconst, one has already been fixed in #5061 and I'm working on removing iconst.i128, once that is merged I'll give this another round of fuzzing and if everything goes right we should be able to merge this.

[afonso360]

@cfallin What do you think about merging this with the use_egraphs flag commented out and enabling that on a separate PR?

It's getting a bit annoying having to rebase my other work on this PR just to fuzz it. I've fuzzed this for about an hour on both AArch64 and x86 without use_egraphs and it seems stable.

[cfallin]

Oh, for sure, I didn't know we were waiting on something here; sorry! I was waiting for your go-ahead to merge. FWIW the PR is still marked as a draft; go ahead and transition it to "Ready for review" when you want me to merge.

I actually think maybe it's fine to include use_egraphs; better to get the fuzzbugs coming sooner than later, and I'm happy to take them as they come in. But I'm open to the other approach if you think otherwise.

[afonso360]

Oh, for sure, I didn't know we were waiting on something here; sorry!

I wasn't really, I just got a bit annoyed today at having to do a rebase for the third time, that's where that suggestion came from!

I'm okay with merging this as is and letting ossfuzz report issues as we go along.