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

Rewrite InProcessExecutors #2880

Open
tokatoka opened this issue Jan 21, 2025 · 6 comments
Open

Rewrite InProcessExecutors #2880

tokatoka opened this issue Jan 21, 2025 · 6 comments
Milestone

Comments

@tokatoka
Copy link
Member

tokatoka commented Jan 21, 2025

  • We should use builder pattern instead of current constructors that has 90% same code
  • Redesign crash, timeout hooks.
@tokatoka tokatoka added this to the LibAFL 1.0 milestone Jan 21, 2025
@addisoncrump
Copy link
Collaborator

Can we change this to "rewrite executors"? There is too much technical debt here.

@tokatoka tokatoka changed the title Bring builder patterns to Executors Rewrite Executors Jan 21, 2025
@tokatoka tokatoka changed the title Rewrite Executors Rewrite InProcessExecutors Jan 21, 2025
@riesentoaster
Copy link
Contributor

While we're at it, I'm not sure why run_target gets a reference to the Fuzzer. Last I checked, this was only used in some InProcessExecutors to retrieve observers, and I think you already have/can get a reference to those by directly passing the observers to the executor during creation. This is just a cyclical dependency, which is never great. Stuff like passing self to a function on another struct raises some alarm bells for me:

let exit_kind = executor.run_target(self, state, event_mgr, input)?;

@tokatoka
Copy link
Member Author

tokatoka commented Jan 24, 2025

While we're at it, I'm not sure why run_target gets a reference to the Fuzzer

this is for crash handler.
When you crash in a inproc handler, you have to continue execution from a signal handler.
This signal handler has to know the pointer to 4 things, the executor, the fuzzer, the state, and the event manager.

We write the pointer to these object before executions, so we need the references.

Last I checked, this was only used in some InProcessExecutors to retrieve observers, and I think you already have/can get a reference to those by directly passing the observers to the executor during creation.

In the crash handler, we need to get fuzzer.objectives() so that's why we need fuzzer.

@riesentoaster
Copy link
Contributor

In the crash handler, we need to get fuzzer.objectives() so that's why we need fuzzer.

The only thing ever needed from the fuzzer are the objectives, right? In that case: Why not just pass the OT instead of the entire fuzzer?

@riesentoaster
Copy link
Contributor

So I've tried it, and at least the libafl crate builds: #2892

@riesentoaster
Copy link
Contributor

Alright, it works. Ready to merge from my side.

This doesn't mean this issue is to be closed, but at least the executors are a bit more disentangled from fuzzer.

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

No branches or pull requests

3 participants