Skip to content
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

Don't rely on libfuzzer-provided entrypoint #71

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Nov 22, 2020

So, this can definitely work without changes to libfuzzer itself, as it
is written today. There are a couple pieces to it:

  1. Don't compile FuzzerMain.cpp at all;
  2. Use LLVMFuzzerRunDriver defined in FuzzerDriver.cpp to kick off
    the fuzzing process in the macro.

I think with those and something like the inventory crate we also open
ourselves to having interface that's more like libtest.

That said, LLVMFuzzerRunDriver requires argc and argv, which at
that point requires one to manually convert them back into C layout from
std::env::args_os.

I also don't believe this change is meaningful as is, without an
otherwise major rework of the libfuzzer API. It doesn't achieve
anything much as it is, and only serves to complicate the implementation
of libfuzzer crate itself.

cc #46

So, this can definitely work without changes to libfuzzer itself, as it
is written today. There are a couple pieces to it:

1. Don't compile `FuzzerMain.cpp` at all;
2. Use `LLVMFuzzerRunDriver` defined in `FuzzerDriver.cpp` to kick off
the fuzzing process in the macro.

I think with those and something like the `inventory` crate we also open
ourselves to having interface that's more like `libtest`.

That said, `LLVMFuzzerRunDriver` requires `argc` and `argv`, which at
that point requires one to manually convert them back into C layout from
`std::env::args_os`.

I also don't believe this change is meaningful as is, without an
otherwise major rework of the libfuzzer API. It doesn't achieve
anything much as it is, and only serves to complicate the implementation
of libfuzzer crate itself.
@nagisa
Copy link
Member Author

nagisa commented Nov 22, 2020

(it doesn't build right now because there are some changes wrt visibility within macro, which I didn't consider a good expenditure of time, given the other observations)

@nagisa
Copy link
Member Author

nagisa commented Nov 25, 2021

Returning to this, I think it might be worthwhile to do this. Among other things that this can enable is e.g. an ability to specify certain fuzzing-related parameters within the test case itself. So e.g. they could specify sensible defaults for the fuzz target from within the source.

@jameysharp
Copy link

I think you can make this available without breaking the existing libfuzzer crate API by splitting the crate into two parts.

One crate links in the "no-main" version of libFuzzer (which is apparently a standard build configuration these days). That crate should just export bindings to functions like LLVMFuzzerRunDriver.

The other crate exposes the current libfuzzer-sys API by defining main. Maybe that can be a C-compatible main (#[no_mangle] etc) so you don't have to convert back from std::env::args_os?

I'm not actually sure how that would work. I had C-compatible main functions working in Corrode-generated Rust modules five years ago, but then I was doing the final link with gcc, so probably everything's quite different.

@addisoncrump
Copy link
Contributor

Alternatively, you can apply the strategy of #106 and feature-gate the linkage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants