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

Deduplicate OnDisk Corpus #2827

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

BAGUVIX456
Copy link
Contributor

@BAGUVIX456 BAGUVIX456 commented Jan 11, 2025

addresses #2434

Original PR (now closed) #2802

Cargo.toml Outdated Show resolved Hide resolved
@tokatoka
Copy link
Member

this is a lot ...
can you wait a bit

@tokatoka
Copy link
Member

tokatoka commented Jan 12, 2025

Hey @BAGUVIX456.
I discussed with others, but I think the previous direction is acceptable/better.
can you force push f06ed97 to the branch then we can merge after reviewing the remaining part

sorry for the confusion!

@tokatoka
Copy link
Member

can you run ./scripts/fmt_all.sh and fix the errors in ci?

@BAGUVIX456
Copy link
Contributor Author

yep, working on it

@tokatoka
Copy link
Member

it seems it doesn't pass some tests

thread 'schedulers::queue::tests::test_queuecorpus' has overflowed its stack
error: test failed, to rerun pass `-p libafl --lib`

Caused by:
  process didn't exit successfully: `D:\a\LibAFL\LibAFL\target\debug\deps\libafl-6609fdc76bef418d.exe` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)
note: test exited abnormally; to see the full output pass --nocapture to the harness.

@BAGUVIX456
Copy link
Contributor Author

Seems like fs2 is having trouble with windows, i'll have a look

@BAGUVIX456
Copy link
Contributor Author

Any method that modifies the name of a testcase (rename_testcase() of InMemoryOnDisk corpus or Testcase::with_filename()) runs the risk of breaking deduplication. Would just adding a warning work for now or should I do something else?
The worst case scenario is that they will end up with duplicate testcases so it is not really a critical issue, but it needs to be considered nonetheless

@BAGUVIX456
Copy link
Contributor Author

@tokatoka the bulk of the work is completed, just need your prompt on what to do with the renaming of testcases and then we can merge this pr

@tokatoka
Copy link
Member

Any method that modifies the name of a testcase (rename_testcase() of InMemoryOnDisk corpus or Testcase::with_filename()) runs the risk of breaking deduplication.

there's no easy way to avoid this, no? then it's fine.

and then can you run ./scripts/fmt_all.sh and fix clippy?

@domenukk
Copy link
Member

Any method that modifies the name of a testcase (rename_testcase() of InMemoryOnDisk corpus or Testcase::with_filename()) runs the risk of breaking deduplication.

there's no easy way to avoid this, no? then it's fine.

and then can you run ./scripts/fmt_all.sh and fix clippy?

I think rename should create a new file and if the old file's counter goes to 0, remove the old file

@tokatoka
Copy link
Member

there's still problem in windows's clippy

@BAGUVIX456
Copy link
Contributor Author

Something wrong with the gdiplus fuzzer, will figure that out along with handling renaming

@BAGUVIX456
Copy link
Contributor Author

@domenukk could you review whether there are any more changes that I need to do for renaming?

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