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

error handeling in libafl/src/corpus #2990

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SWASTIC-7
Copy link

Changed all .unwrap with appropriate libafl_bolts::Error
#1908

@tokatoka
Copy link
Member

i think most of these unwrap() will never happen.

@@ -38,7 +38,9 @@ where
self.load_input_into(&mut testcase.borrow_mut())?;
let mut borrowed_num = 0;
while self.cached_indexes.borrow().len() >= self.cache_max_len {
let removed = self.cached_indexes.borrow_mut().pop_front().unwrap();
let removed = self.cached_indexes.borrow_mut().pop_front().ok_or_else(|| {
Copy link
Member

Choose a reason for hiding this comment

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

this will never happen. the thing we really need is to make cache_max_len to NonZeroUsize

@tokatoka
Copy link
Member

i don't know. the changes here are either correcting unwrap() that either

  1. will never happen 2) comes from errors for which user can do nothing after receiving the error.

I perfer for the panic to stay because at least i can see the backtraces.

@SWASTIC-7
Copy link
Author

Ok so i will can make some unwrap to be as it, and only chnage which are necessary

@tokatoka
Copy link
Member

see this
https://github.com/AFLplusplus/LibAFL/pull/2990/files#r1957188382

this is what we really want then panic will never happen from this unwrap() will never happen

if let Some(item) = self.map.remove(&id) {
self.remove_key(id);
if let Some(prev) = item.prev {
self.map.get_mut(&prev).unwrap().next = item.next;
self.map.get_mut(&prev).ok_or_else(|| Error::illegal_state("Unable to fetch previous CorpusId"))?.next = item.next;
Copy link
Member

Choose a reason for hiding this comment

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

next CorpusId

@@ -198,7 +198,7 @@ where
let mut removed = Vec::with_capacity(state.corpus().count());
for (seed, (id, _)) in seed_exprs {
// if the model says the seed isn't there, mark it for deletion
if !model.eval(&seed, true).unwrap().as_bool().unwrap() {
if !model.eval(&seed, true).ok_or_else(|| Error::illegal_state("Error Evaluating Modal"))?.as_bool().ok_or_else(|| Error::illegal_state("Error converting condition to bool"))? {
Copy link
Member

Choose a reason for hiding this comment

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

evaluating model

@domenukk
Copy link
Member

This also needs cargo fmt, etc., to make CI happy

@SWASTIC-7
Copy link
Author

I have done cargo fmt, etc and now no errors are comming

@domenukk
Copy link
Member

CI is still unhappy

   --> libafl/src/corpus/inmemory.rs:109:9
    |
107 |     pub fn remove(&mut self, id: CorpusId) -> Result<RefCell<Testcase<I>>, Error> {
    |                                               ----------------------------------- expected `Result<RefCell<Testcase<I>>, libafl_bolts::Error>` because of return type
108 |         self.remove_key(id);
109 |         self.map.remove(&id)
    |         ^^^^^^^^^^^^^^^^^^^^ expected `Result<RefCell<Testcase<I>>, Error>`, found `Option<RefCell<Testcase<I>>>`
    | 

@tokatoka
Copy link
Member

can you address my comment too?

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.

4 participants