rust-fuzz / libfuzzer

Rust bindings and utilities for LLVM’s libFuzzer
Apache License 2.0
210 stars 44 forks source link

Make RUST_LIBFUZZER_DEBUG_PATH "free" for non-users #81

Closed elichai closed 3 years ago

elichai commented 3 years ago

Currently if someone doesn't use RUST_LIBFUZZER_DEBUG_PATH they still need to pay an allocation (to convert to CString) a Mutex lock, and a getenv(3) call for every fuzzing input.

Instead we can check and load the environment variable at initialization and from then it will be almost free for users that don't set the environment variable.

(this is slightly a behavior change as before someone could attach a debugger and modify the enviroment variable while it's fuzzing but now they can't, but I don't think it matters)

elichai commented 3 years ago

However, I'd like to avoid all this unsafe. I'm not arguing that it is incorrect, its just a lot of open-coded unsafe stuff that adds that much overhead to all future maintenance. Instead let's either

* use `lazy_static`

* make `RUST_LIBFUZZER_DEBUG_PATH` an `AtomicBool` that is set during init if the env var is set, and then checked during the main function where, if it is set, we can afford to pay for the `std::env::var` call again.

Yeah I also really wanted to avoid unsafe, but couldn't find a good way to do it (too bad there's no AtomicStaticRef<T> type that can give us the same as AtomicPtr but without using unsafe)

I don't mind using lazy_static or once_cell(the latter will allow us to move to libstd provided variant in the future more easily - See https://github.com/rust-lang/rust/issues/74465) but I wanted to avoid introducing another dependency.

the 2nd option sounds interesting, it's still a little more expensive, but only for people who use this feature and they then do actual I/O so it's probably negligible compared to that.

So for me either is fine, which one do you prefer?

fitzgen commented 3 years ago

Good point about OnceCell eventually moving into std. Let's go with that: initializing the OnceCell inside initialize and then doing

if let Some(path) = the_once_cell.get() {
    // ...
}

in the main function.

elichai commented 3 years ago

Good point about OnceCell eventually moving into std. Let's go with that: initializing the OnceCell inside initialize and then doing

if let Some(path) = the_once_cell.get() {
    // ...
}

in the main function.

Done

elichai commented 3 years ago

@fitzgen I fixed the comments :)