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

add support for dev-dependencies #338

Closed BurntSushi closed 1 year ago

BurntSushi commented 1 year ago

My main ask is this: I would like a way to build fuzz targets such that they bring in the dev-dependencies of the crate being tested. This issue was discussed a little bit in #256, but I wanted to try again with a more clearly defined use case.

Folks have been working on improving the fuzzers for the regex crate. One such improvement is add arbitrary::Arbitrary impls for the various Ast types in regex-syntax instead of relying on parsing a pattern out of a &[u8]. The problem that has cropped up is that this seems to require adding a proper dependency on the arbitrary crate from regex-syntax. Even if we assume I take steps to make using this optional dependency on arbitrary difficult (like gate it on cfg(fuzzing) and name the feature something weird), then I don't want to do this for a few reasons:

  1. I want regex-syntax to remain free of dependencies, even optional ones. In the past, I've seen ways for even optional dependencies to impact cargo build even if they aren't used. This tends to come up with MSRV concerns. For example, if the current version of Cargo doesn't know how to read the manifest of an optional dependency. There may be other avenues for impact. It's a little fuzzy to me. But I know one thing: if it isn't a dependency then it can't have any impact.
  2. Regardless of what I do, Arbitrary impls are likely to be found to be useful by others, and it's likely they will find a way to use them. That puts them at risk of breakage.

Now I do have some other work-arounds that avoid adding new optional dependencies, but they're pretty gross. The leading contender at the moment is probably to duplicate the Ast types in the fuzzer target, derive(Arbitrary) on them and then convert them to the regex-syntax Ast types. The Ast types don't change much, but... there are a lot of them.

Also, popping up a level, cargo fuzz is supposed to be test infrastructure, right? If so, then it seems to me that, at least semantically, it should permit bringing in dev-dependencies just like cargo test does. That is, cargo fuzz seems to me like it's a lot closer to cargo test than it is to cargo build.

Manishearth commented 1 year ago

So the problem here is less a desire to do this, but rather that there is no way to do this cleanly.

Cargo does not offer much control over its internals. cargo fuzz works in a very specific way — it creates a new crate with binaries and runs them — because that's the only way to get cargo to run arbitrary binaries with the right dependencies built, without polluting examples/. That's the only option on the menu when it comes to dependency resolution; we can't ask Cargo "oh but you should include the dev deps of that other crate". Cargo is too complicated to emulate, that's a alsonon-starter for us (The guppy family of crates are magical and help here but they do not help with build invocations).

The custom test frameworks eRFC was in part intended to help projects like cargo-fuzz, though after a lot of pressure from the discussion some of the parts needed for cargo-fuzz were removed (and the RFC grew in complexity on the rustc side). It's never been fully implemented and it's a bit of a dead end now. Personally I'd recommend anyone intererested step back and implement the original, simpler proposal which did very little fancy stuff at the rustc level but instead exposed more knobs on cargo.


The workaround I suggest is either:

If the examples route can be made to work pleasantly I'm open to adding support for creating and running fuzz targets that way to cargo-fuzz


I'm closing this because we have no clear long- or short-term route to making this work, not because we don't want this. I consider "abuse examples/ to get it to work" to be something that needs a different issue, since it has huge impacts on the rest of cargo-fuzz's UX/interface and is basically creating a Whole New Tool.

If such a route appears in the future I'm happy to reopen this, and I do not mind this issue being used to track any progress/discussion on that route.

BurntSushi commented 1 year ago

@Manishearth Thanks for the response! I was worried that this might be the case. That is, that this is less of a "we don't want this" and more of a "not feasible to support." Bummer. OK.

I'll check out the arbitrary crate in more thorough detail and reconsider that as an optional dependency. Otherwise, I'll probably just end up duplicating the Ast types within the fuzzer target. They almost never change, so it's a hopefully one-time "hold your nose and just do it" cost.

The example-abuse-hack seems neat, but I do definitely like the idea of custom test frameworks being the better option long term.