-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
Resumable stages redux #1780
Resumable stages redux #1780
Conversation
https://github.com/AFLplusplus/LibAFL/actions/runs/7428264611/job/20215339231#step:9:193 Python is gonna be the death of me I swear 💀 Totally screws over my strategy of using the datatype as the stage index tracker. |
Found a workaround, but I don't know how we're going to implement resume for python... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like where this is going. @s1341 what do you think?
libafl/src/stages/mod.rs
Outdated
@@ -498,3 +520,49 @@ pub mod pybind { | |||
Ok(()) | |||
} | |||
} | |||
|
|||
/// Trait for status tracking of stages which stash data to resume | |||
pub trait ResumableStageStatus<S> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random question, could these stages, if implemented correctly, then be used as PushStage as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite sure. I've not done a whole lot with push stages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically instead of libafl running the fuzzing loop, libafl returns the next testcase.
So here, it would then "continue" after that
Guess it'd need some additional remodelling to be able to return instead of execute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, after reviewing, we should be able to make push stage resumeable using the same strategy as e.g. If/IfElse/WhileStage.
316b525
to
2683c9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Feel free to merge.
Maybe take a look at the pushstage stuff, some things seem similar
/// Trait for types which track the current corpus index | ||
pub trait HasCurrentCorpusIdx { | ||
/// Set the current corpus index; we have started processing this corpus entry | ||
fn set_corpus_idx(&mut self, idx: CorpusId) -> Result<(), Error>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could consider a _mut
like we do in the other Has
traits. It's not necessarily prettier, but it would be more consistent (and less to write to implement it, too?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally chose not to do this because it allows for a callback to be invoked when the corpus index is set (e.g. if there's any metadata or smth).
@@ -92,6 +92,8 @@ where | |||
E::State: HasCorpus + HasMetadata + HasNamedMetadata + HasExecutions, | |||
Z: Evaluator<E, EM, State = E::State>, | |||
{ | |||
type Progress = (); // TODO stage may be resumed, but how? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we somehow mark stages that are non-resumable yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this stage is not resumable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should at least handle the "sometimes crashes" scenario? Surely there are some targets for which the non-determinism may affect whether the testcase crashes or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i understood correctly, this is jus thte framework, the actual work of making the stages resumable is yet to be done, correct?
This attempts to reimplement #1341 with slightly different design. It is not yet complete; TODOs have been marked where changes still need to be applied.