rust-fuzz / cargo-fuzz

Command line helpers for fuzzing
https://rust-fuzz.github.io/book/cargo-fuzz.html
Apache License 2.0
1.48k stars 108 forks source link

Consider using a workspace instead of a fully separate crate #345

Closed jyn514 closed 5 months ago

jyn514 commented 1 year ago

Right now, cargo fuzz init generates the following Cargo.toml:

[package]
name = "example-fuzz"
version = "0.0.0"
publish = false
edition = "2021"

[package.metadata]
cargo-fuzz = true

[dependencies]
libfuzzer-sys = "0.4"

[dependencies.example]
path = ".."

# Prevent this from interfering with workspaces
[workspace]
members = ["."]

[profile.release]
debug = 1

[[bin]]
name = "fuzz_target_1"
path = "fuzz_targets/fuzz_target_1.rs"
test = false
doc = false

In particular, this puts all the fuzz targets in a separate workspace. I am not sure why that decision was made - it prevents reusing the build cache between the fuzzer and the rest of the code. It would be nice if they shared a workspace and therefore a cache.

I do see that this configures profile.release, so to avoid interfering with existing configs it could use a new profile.fuzzing or something like that.

fitzgen commented 1 year ago
# Prevent this from interfering with workspaces

This has been there since before I started hacking on cargo fuzz. Not sure what the "interfering" alluded to actually is.

@Manishearth any idea?

jyn514 commented 1 year ago

"interfering" is probably referring to this error you get if you remove the [workspace] declaration:

; c fuzz check   
error: current package believes it's in a workspace when it's not:
current:   /Users/jyn/src/example/fuzz/Cargo.toml
workspace: /Users/jyn/src/example/Cargo.toml

this may be fixable by ensuring that this crate is depended on by the workspace root: /Users/jyn/src/example/Cargo.toml
Alternatively, to keep it out of the workspace, add the package to the `workspace.exclude` array, or add an empty `[workspace]` table to the package's manifest.
Error: failed to build fuzz script: "cargo" "check" "--manifest-path" "/Users/jyn/src/example/fuzz/Cargo.toml" "--target" "aarch64-apple-darwin" "--release" "--bins"

so to make this part of the workspace, you'd have to also edit example/Cargo.toml, not just create the new example/fuzz/Cargo.toml.

Manishearth commented 1 year ago

Yep. We can remove that if we also fix the workspace editing. We ought to.

arvidn commented 1 year ago

I agree that fuzzers should be part of the main workspace. Sharing the Cargo.lock with the main workspace ensures that you fuzz the exact dependencies that you build against. It also includes fuzz targets in cargo fmt --all and cargo clippy --workspace invocations, which is nice.

In addition to adding the fuzz directory to the root workspace members, you should also define the fuzz targets with:

bench = false

otherwise, cargo bench --workspace will get stuck, running fuzz targets as if they were benchmarks.

kdarkhan commented 5 months ago

I believe this issue can be resolved now given that 0.11.4 version of cargo-fuzz contains changes from https://github.com/rust-fuzz/cargo-fuzz/pull/357