Skip to content

Implement per-iteration leak scan for fuzzer - #2564

Merged
derekbruening merged 14 commits into
DynamoRIO:masterfrom
Pastoray:drfuzz-per-iter-leak-scan
Oct 7, 2025
Merged

derekbruening merged 14 commits into
DynamoRIO:masterfrom
Pastoray:drfuzz-per-iter-leak-scan

Conversation

@Pastoray

@Pastoray Pastoray commented Sep 29, 2025 •

Copy link
Copy Markdown
Contributor

Adds a new option -fuzz_per_iter_leak_scan which writes leak scan results
after each fuzz iteration to a new output file fuzz_results.txt.

Fixes: #1797

@Pastoray

Pastoray commented Oct 1, 2025

Copy link
Copy Markdown
Contributor Author

I thought about directly calling nudge_leak_scan but that would make the number of nudges inaccurate as well as constantly dumping the stats for every iteration which is not good, if this is missing anything or am not doing anything as intended please let me know

@Pastoray
Pastoray marked this pull request as draft October 1, 2025 12:07

@derekbruening derekbruening left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for contributing. Looks reasonable overall.

Comment thread drmemory/optionsx.h
Comment thread drmemory/fuzzer.c Outdated
Comment thread drmemory/optionsx.h
Comment thread drmemory/drmemory.c Outdated
@Pastoray

Pastoray commented Oct 2, 2025

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback and your patience. I've implemented the majority of the changes you pointed out in the review. However, I noticed that the ci-clang / clang build is currently failing. Although i don't think it relates to my changes. Regardless let me know if there's anything else I might be missing.

@derekbruening

Copy link
Copy Markdown
Contributor

Thank you for the feedback and your patience. I've implemented the majority of the changes you pointed out in the review. However, I noticed that the ci-clang / clang build is currently failing. Although i don't think it relates to my changes. Regardless let me know if there's anything else I might be missing.

Unfortunately some of the automated testing needs some maintenance: more developers are needed to help. Clicking on re-run may solve in the short term. Filing an issue and temporarily committing removal/ignoring of the problematic test config if it keeps happening; long term trying to fix.

@Pastoray
Pastoray marked this pull request as ready for review October 2, 2025 13:42
@Pastoray Pastoray changed the title WIP: Fuzz per iteration leak scan implementation Implement per-iteration leak scan for fuzzer Oct 2, 2025
@Pastoray

Pastoray commented Oct 2, 2025 •

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback and your patience. I've implemented the majority of the changes you pointed out in the review. However, I noticed that the ci-clang / clang build is currently failing. Although i don't think it relates to my changes. Regardless let me know if there's anything else I might be missing.

Unfortunately some of the automated testing needs some maintenance: more developers are needed to help. Clicking on re-run may solve in the short term. Filing an issue and temporarily committing removal/ignoring of the problematic test config if it keeps happening; long term trying to fix.

Thanks for the clarification. I've gone ahead and marked the PR as ready for review and cleaned up the title, since all the requested code changes you requested are finished.

Regarding that ci-clang build, since I can't hit the re-run button, I'll leave it to you or someone else to either re-run it or ignore it. If it messes up again after a try, i'll file an issue.

@derekbruening

Copy link
Copy Markdown
Contributor

Please resolve comments that are addressed, following https://dynamorio.org/page_code_reviews.html#autotoc_md118

@Pastoray
Pastoray requested a review from derekbruening October 2, 2025 17:22

@derekbruening derekbruening left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks again for the contribution.

Comment thread drmemory/docs/fuzzer.dox
@Pastoray

Pastoray commented Oct 2, 2025

Copy link
Copy Markdown
Contributor Author

Thanks again for the contribution.

My pleasure. Glad i could help.

@Pastoray

Pastoray commented Oct 3, 2025 •

Copy link
Copy Markdown
Contributor Author

Actually before you make your final review the current implementation logs previous errors under the current inputs. Essentially reporting everything from this latest iteration and backwards, maybe i should implement it so that it actually only includes errors about the current iteration. This is identical to nudging a leak scan for every iteration, but i don't know if that's the intended behavior. maybe a more granular report would be better ?

What do you think ?
Thank you for your time

@derekbruening

Copy link
Copy Markdown
Contributor

Actually before you make your final review the current implementation logs previous errors under the current inputs. Essentially reporting everything from this latest iteration and backwards, maybe i should implement it so that it actually only includes errors about the current iteration. This is identical to nudging a leak scan for every iteration, but i don't know if that's the intended behavior. maybe a more granular report would be better ?

What do you think ? Thank you for your time

Do you mean you would add logic that identifies leak reports that are identical to ones reported in the last iteration and removes those from the report? I would say separate that into a different PR: in this PR I would vote for updating the docs to say all leaks are shown each iteration and then if you wanted to add the dup removal do that in a separate PR.

@Pastoray

Pastoray commented Oct 3, 2025

Copy link
Copy Markdown
Contributor Author

Actually before you make your final review the current implementation logs previous errors under the current inputs. Essentially reporting everything from this latest iteration and backwards, maybe i should implement it so that it actually only includes errors about the current iteration. This is identical to nudging a leak scan for every iteration, but i don't know if that's the intended behavior. maybe a more granular report would be better ?
What do you think ? Thank you for your time

Do you mean you would add logic that identifies leak reports that are identical to ones reported in the last iteration and removes those from the report? I would say separate that into a different PR: in this PR I would vote for updating the docs to say all leaks are shown each iteration and then if you wanted to add the dup removal do that in a separate PR.

Yeah that should probably be it's own PR i think that settles this one though.

Comment thread drmemory/docs/fuzzer.dox
Comment thread drmemory/fuzzer.c
Comment thread tests/fuzz/CMakeLists.txt
@Pastoray

Pastoray commented Oct 5, 2025

Copy link
Copy Markdown
Contributor Author

The test doesn't respect results.txt now it's kinda irrelevant it just cares about the details in fuzz_results.txt, i don't think that's a problem though considering the test is specifically checking for the existence of fuzz_results.txt and it's content, could have two separate tests to test both files but i think that's unnecessary.

Comment thread tests/fuzz/fuzz_buffer.leak_iter.res Outdated
@derekbruening
derekbruening merged commit 80cae11 into DynamoRIO:master Oct 7, 2025
7 checks passed
@derekbruening

Copy link
Copy Markdown
Contributor

Merged. Thanks again for contributing.

@Pastoray

Pastoray commented Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

Merged. Thanks again for contributing.

My pleasure.

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.

Add per-iteration leak scan in Dr. Fuzz

2 participants