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

Allow multiple tuneable mutational stages #1437

Merged
merged 10 commits into from
Aug 24, 2023

Conversation

wtdcode
Copy link
Contributor

@wtdcode wtdcode commented Aug 21, 2023

I can't see obvious reasons to make TuneableMutationalStage a singleton class and it's super inconvenient that every fuzzer can only have one TuneableMutationalStage. Therefore, like the observers and feedbacks, I assign a name for each TuneableMutationalStage to allow multiple mutational strages while keeping maximum backward compatibility.

@domenukk
Copy link
Member

This needs a cargo fmt I think, else it looks reasonable :) thanks

@wtdcode
Copy link
Contributor Author

wtdcode commented Aug 21, 2023

This needs a cargo fmt I think, else it looks reasonable :) thanks

I already did that and I just tred cargo fmt again while nothing changed.

@domenukk
Copy link
Member

Diff in /Users/runner/work/LibAFL/LibAFL/libafl/src/stages/tuneable.rs at line 1:
 //! A [`crate::stages::MutationalStage`] where the mutator iteration can be tuned at runtime
 
+use alloc::string::{String, ToString};
 use core::{marker::PhantomData, time::Duration};
 
-use alloc::string::{String, ToString};
 use libafl_bolts::{current_time, impl_serdeany, rands::Rand};
 use serde::{Deserialize, Serialize};

You may need cargo +nighlty fmt to reorder inputs

@wtdcode
Copy link
Contributor Author

wtdcode commented Aug 21, 2023

Turns out that I forgot to switch the branch

}

/// Get the set iterations
pub fn iters<S: HasMetadata>(state: &S) -> Result<Option<u64>, Error> {
get_iters(state)
pub fn iters<S: HasNamedMetadata>(&self, state: &S) -> Result<Option<u64>, Error> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe have a iters_named fn then?

Copy link
Member

@andreafioraldi andreafioraldi Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domenukk IMO using named is just fine, this is called once per scheduled input IIRC, so not really an hot path.

@domenukk
Copy link
Member

@andreafioraldi is there overhead for named vs non named? Assuming this is on the hot path, would it make sense to have extra functions for non-named?

@andreafioraldi
Copy link
Member

@andreafioraldi is there overhead for named vs non named? Assuming this is on the hot path, would it make sense to have extra functions for non-named?

Yes, named is two hashmap lookups, while non named just one

libafl/src/stages/tuneable.rs Outdated Show resolved Hide resolved
}

/// Get the set iterations
pub fn iters<S: HasMetadata>(state: &S) -> Result<Option<u64>, Error> {
get_iters(state)
pub fn iters<S: HasNamedMetadata>(&self, state: &S) -> Result<Option<u64>, Error> {
Copy link
Member

@andreafioraldi andreafioraldi Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domenukk IMO using named is just fine, this is called once per scheduled input IIRC, so not really an hot path.

@domenukk
Copy link
Member

I still don't like that now users have to use a name, I might prefer a second API of _named for usability sake

@wtdcode
Copy link
Contributor Author

wtdcode commented Aug 24, 2023

Done

@domenukk
Copy link
Member

Sweet, thx :)

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