feat(fuzz): make fuzz compile/run #140
Conversation
I have no clue what this does - fuzzing was broken for a long time, or at least never re-validated it seems (not part of CI, no docs). I fixed it with AI, plus some manual cleanup to remove AI-isms. Fuzzing was added in 2021 by Marcel in e41daa8 , any guidance would be good :)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Restores the ability to compile and run the existing cargo-fuzz harness by gating/re-exporting the generated cantools C bindings behind a dedicated feature and updating the fuzz target to use those bindings safely.
Changes:
- Add a
fuzzfeature totesting/cantools-messagesand re-export generated bindgen symbols when enabled. - Update the fuzz target to pack via cantools and compare output bytes (with a small tolerance for float/encode differences).
- Add a
just fuzzhelper target and update the fuzz crate dependency/lockfile.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/cantools-messages/src/lib.rs | Compiles bindings for tests or fuzz feature; re-exports bindgen API for fuzz crate. |
| testing/cantools-messages/Cargo.toml | Introduces fuzz feature flag for binding exposure. |
| justfile | Adds a fuzz recipe to run cargo +nightly fuzz. |
| fuzz/fuzz_targets/fuzz_target_1.rs | Uses fuzz input, calls cantools pack with raw-pointer-safe address, compares packed bytes. |
| fuzz/Cargo.toml | Enables cantools-messages fuzz feature for fuzzing build. |
| fuzz/Cargo.lock | Updates fuzz subproject lockfile after dependency resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Run cargo fuzz tests. Requires nightly toolchain (install with `rustup install nightly`) | ||
| fuzz: | ||
| RUSTFLAGS='-C target-feature=-crt-static' cargo +nightly fuzz run fuzz_target_1 --target $(rustc --print host-tuple) |
There was a problem hiding this comment.
The --target value is computed using rustc from the default toolchain, but this recipe runs cargo +nightly. On systems without the stable toolchain installed (or with a different default toolchain), $(rustc --print host-tuple) can fail or disagree with nightly. Use rustc +nightly --print host-tuple (or drop the explicit --target if not needed).
| RUSTFLAGS='-C target-feature=-crt-static' cargo +nightly fuzz run fuzz_target_1 --target $(rustc --print host-tuple) | |
| RUSTFLAGS='-C target-feature=-crt-static' cargo +nightly fuzz run fuzz_target_1 --target $(rustc +nightly --print host-tuple) |
| // Cantools C uses truncation for encode (e.g. (uint8_t)(value/0.39)) while dbc-codegen | ||
| // may round; allow off-by-one in the first byte (Two signal) due to this. |
There was a problem hiding this comment.
This comment says dbc-codegen "may round", but dbc-codegen’s float encoding path truncates after a float division/cast (similar to C). If you’re seeing off-by-one, it’s more likely due to float precision differences (Rust uses f32 internally vs cantools using double) rather than rounding. Suggest updating the comment to reflect the actual cause so future readers don’t chase the wrong behavior.
| // Cantools C uses truncation for encode (e.g. (uint8_t)(value/0.39)) while dbc-codegen | |
| // may round; allow off-by-one in the first byte (Two signal) due to this. | |
| // Cantools C uses truncation for encode (e.g. (uint8_t)(value/0.39)). dbc-codegen's | |
| // encode path also truncates, but uses f32 internally (vs. cantools' double), so minor | |
| // precision differences can cause an off-by-one in the first byte (Two signal); allow that. |
1 participant
I have no clue what this does - fuzzing was broken for a long time, or at least never re-validated it seems (not part of CI, no docs). I fixed it with AI, plus some manual cleanup to remove AI-isms.
Fuzzing was added in 2021 by @marcelbuesing in e41daa8 -- any help or guidance would be good :)
The biggest question of course - is there actually any benefit from this code, or should it be scrapped and re-done from scratch, and if so, any advice on that too?