-
-
Notifications
You must be signed in to change notification settings - Fork 435
Handle missing mutation names in LoggerScheduledMutator::post_exec, at /crates/libafl/scheduled.rs #3670
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
base: main
Are you sure you want to change the base?
Conversation
| .mutations() | ||
| .name(idx.0) | ||
| .cloned() | ||
| .ok_or_else(|| Error::unknown(format!("Missing mutation name for id {}", idx.0)))?; |
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.
Can we somehow get the actual mutation and debug print that?
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 believe that if we store the name over here then its possible to print the name as well. Shoud i proceed?
Line 363 scheduled.rs
fn scheduled_mutate(&mut self, state: &mut S, input: &mut I) -> Result<MutationResult, Error> {
let mut r = MutationResult::Skipped;
let num = self.iterations(state, input);
self.mutation_log.clear();
for _ in 0..num {
let idx = self.schedule(state, input);
self.mutation_log.push(idx); //<---------- here save the name as well ?
let outcome = self.mutations_mut().get_and_mutate(idx, state, input)?;
if outcome == MutationResult::Mutated {
r = MutationResult::Mutated;
}
}
Ok(r)
}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 it's possible to only store a ref (not clone the full string), then yes, but I highly doubt it.
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 it's possible to only store a ref (not clone the full string), then yes, but I highly doubt it.
I cannot think of a way to implement that without saving the entire string.
Could you suggest some ideas to me?
Otherwise, could we proceed without that specific change? Further if I do figure it out, I'll propose it with a new pr
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.
Can we get the actual mutation in the error message and include the debug representation? Not using name at all? In the error handler we have a lot of time, it's not a hot code path.
If not I can merge the current state, too, but it'll make debugging this harder
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.
Alternatively we can just print the complete mutations tuple, and the id
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.
Alternatively we can just print the complete mutations tuple, and the id
Hey,
For the alternative, did you mean this?
.ok_or_else(|| {
Error::unknown(format!(
"Missing mutation name for id {}. Available mutations: {:?}",
idx.0,
self.scheduled.mutations().names()
))
})?;
Description
Previously, LoggerScheduledMutator::post_exec used .unwrap() when retrieving mutation names from the scheduled mutator. This could cause a panic if a mutation index had no corresponding name.
This PR replaces the unwrap() with proper error handling, returning an Error if a mutation name is missing. This ensures post_exec fails gracefully instead of panicking, improving stability during fuzzing sessions.
Also added tests
Checklist
./scripts/precommit.shand addressed all comments