afonso360 opened PR #5020 from fuzzgen-flags
to main
:
:wave: 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.
<details>
<summary>The actual bug manifests as following:</summary>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 https://github.com/bytecodealliance/wasmtime/issues/4875#issuecomment-1239314171 (which I kinda forgot about :sweat_smile: ). But I'm going to try and investigate that one in the mean time.
</details>This also enables regalloc_checker as we discussed in #4979.
cc: @jameysharp
afonso360 updated PR #5020 from fuzzgen-flags
to main
.
afonso360 updated PR #5020 from fuzzgen-flags
to main
.
afonso360 updated PR #5020 from fuzzgen-flags
to main
.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
It probably would make sense to leave a comment in
cranelift/codegen/meta/src/shared/settings.rs
where all of these options are defined to note "please add to fuzzing list ..." as a step when adding new otions?
cfallin created PR review comment:
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 acranelift_codegen::isa::all()
that returns allTriple
s 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.)
cfallin created PR review comment:
Some of these can probably be excluded, based on knowledge of what they do and likely payoff from fuzzing time:
machine_code_cfg_info
generates additional metadata for the embedder but this doesn't feed back into compilation anywhere; we can leave it on unconditionally to make sure the generation doesn't panicenable_simd
,enable_float
,enable_atomics
will cause panics in compilation if an input uses one of those features and it isn't enabled, but don't change compilation otherwise; so we can probably turn them on unconditionally (we're not, e.g., trying to test that a SIMD input causes panics with the feature off)
afonso360 submitted PR review.
afonso360 created PR review comment:
Yeah the
all()
idea sounds good, I've been adding them where it think they would be useful for others.
afonso360 submitted PR review.
afonso360 created PR review comment:
Agreed, let me know if any other flags come to mind!
enable_simd
,enable_float
,enable_atomics
will cause panics in compilation if an input uses one of those features and it isn't enabled,Enabling them unconditionally sounds good.
But at least for
enable_floats
it hasn't panicked yet and we almost always insert floats for larger programs, so we might be silently allowing them anyway. (Haven't tested this yet)
afonso360 updated PR #5020 from fuzzgen-flags
to main
.
afonso360 submitted PR review.
afonso360 created PR review comment:
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 has marked PR #5020 as ready for review.
cfallin merged PR #5020.
Last updated: Nov 22 2024 at 17:03 UTC