-
-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fuzz target checker/fuzz/check_project_naive
#152
Add fuzz target checker/fuzz/check_project_naive
#152
Conversation
Awesome! Yeah doing a quick Is there a reason for using naive data, rather than structured? Also didn't I ask about doing fuzzing for checking last year and heard it wasn't possible? Also as the fuzzing tests don't run on windows (right?) and the current setup is exit on first error is there a way to do print failed exceptions as it would make fixing them a lot easier. In the current CI setup I have option thing |
Maybe I can shine some light on a few of these points.
It is possible, but it's not necessarily going to find bugs with complicated pre-conditions. Mutation-based fuzzing works by exploring the program; it mutates a input, and if it sees new coverage, it keeps that input, otherwise discarding. Suppose you have a bug caused by a single block; this strategy finds bugs for which a crash/incorrectness/branch is induced unconditionally (e.g. it just doesn't work) or conditionally on only the last few blocks. As more of the program's state is encoded into bugs/branches which determine whether these blocks get executed, fuzzing becomes less useful without significantly better mutations. This is because you actually want to explore lots of different program states which are less related to the coverage when your targets encode more state into each branch/action. Concretely, parsers are very good mutation-based fuzz targets because they typically don't encode a lot of state. Typically, each block refers to one particular action (e.g. transforming some tokens to a structure) and that action doesn't have a whole lot of conditions or state attached to it (just the immediate scope). Since mutating from one input to another that is a small edit distance away (naïvely or structured) is likely to induce a bug or new coverage, we can explore the program space and the bugs therein. By comparison, a ton of information is encoded in the state at any given moment in type checkers/compilers, and is used to determine branches and may affect whether a bug is triggered. For this reason, this fuzzer will likely not find deeper bugs in the type checker: edit distance between new coverage and bugs is not guaranteed to be small. Fuzzing compilers is possible, but generally people use generative fuzzers that produce large complex (and generally near-correct!) programs that encode a lot of state, not mutation-based fuzzers like this one. This will find bugs, but it likely won't find so-called "deep bugs". TL;DR: if you want to find really complicated bugs, you're gonna need a far more specialized fuzzer than naïve or even structured mutation-based fuzzers.
It does run in parallel, it just stops all parallel processes at the same time. If you don't want to short-circuit, you can set a |
Does this mean that it is in fact finding an issue then? In that I would be able to catch those as ignorable errors for the time being, but right now they panic when they shouldn't? |
Yep the Thanks @addisoncrump! Will act on them later |
Hey, sorry have been busy so haven't had a chance to look at it. Will merge but only after #157 has been merged which clears up obvious Also looking at CI times, is the fuzzing build using the cache? It might be just because it is a release build and more dependencies or something but it is taking 90 seconds to build whereas other tasks take 30-45 seconds? Maybe because it is not in the workspace or something (because we don't want it to always be built in development). Not sure? |
For fuzzing, the fuzz crates are their own workspace, since there's some weird linking and LLVM flags set. So I don't think it caches? |
Now #157 is merged, this additional fuzzing should produce actual errors. The fuzzing doesn't seem to be compiling though, with an error Once that is fixed then will merge (even if it finds errors as would be useful to have those being highlighted now) |
Yeah, that CI error is this again: rust-fuzz/cargo-fuzz#355 Avoid binstall in CI for cargo-fuzz 😅 Here's the applicable diff (I don't have edit permissions): diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 59c0d97..ac16b2a 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -280,10 +280,9 @@ jobs:
rustup install nightly
rustup default nightly
- - uses: brndnmtthws/rust-action-cargo-binstall@v1
- if: steps.changes.outputs.checker == 'true'
- with:
- packages: cargo-fuzz
+ - name: Install cargo-fuzz
+ if: steps.changes.outputs.parser == 'true'
+ run: cargo install --git https://github.com/rust-fuzz/cargo-fuzz.git
- name: Run fuzzing
env: |
- `cargo install` from git source because current cargo-fuzz release is broken
- `HashSet` -> `Vec` for type definition files
The workflow is running okay so merging now. Currently is finding an error with the partial syntax system in the parser
Might be cyclic recursion or something. (current parser fuzzing test well formed syntax, so might have not picked it up. Partial syntax wasn't a thing when they were written). edit: Think I have fixed it in #190 |
So it begins 😀 |
This PR adds a new fuzz target testing the checker repo. It's not all that helpful at the moment, since it's just detecting
todo!()
blocks in the code, but it can be useful in the future.This also sets up the
fuzzing_parser
andfuzzing_checker
CI jobs to replace thefuzzing
CI job/cc @addisoncrump
Fixes #153