-
Notifications
You must be signed in to change notification settings - Fork 71
Add Murch’s Inputs July 2025 (2nd attempt) #230
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
Conversation
MSan CI fails due to OOM. Since it's only the MSan job, I'd guess that the MSan instrumentation's overhead is the cause. Maybe just delete the offending inputs? |
I removed the five slow and the one oom
|
d0cb372
to
4d2a312
Compare
Squashed it, because CI was unhappy about the deletions, as I should have expected. |
There should be no need to delete pre-existing inputs from ci is trying to tell you that |
Oh, I assumed the troublesome inputs would be new ones. Did we change anything about the MSan CI or the failing harnesses that could cause existing inputs to start failing? |
the oom inputs was a different one : |
Thanks, I’ll amend the PR tomorrow, when I’m in the office. |
4d2a312
to
1480951
Compare
Used 9935aeb, to include the other pull in the meantime: vs main: |
My report still does not show any function or line coverage difference. Also, according to the CI logs, the coverage is mostly identical: https://cirrus-ci.com/task/5416573638279168?logs=ci#L4498 vs https://cirrus-ci.com/task/6084064303644672?logs=ci#L8614 So I guess there is still something wrong on your side? |
Uh, only 7,000 files after weeks of fuzzing does look wrong. o.0 |
Ok, I am thinking about going with #232 first, and then wait for you to revisit your fuzzing workflow, if this sounds good? |
1480951
to
0116b9d
Compare
Please feel free to go ahead with #232. My submission crafting is pretty quick on the new computer, and should my workflow now actually be right, it is easy for me to rebase on the pruned commit history. |
I redid my submission (with one more week of fuzzing under the belt) and pushed. Going to take a look at the CI logs and see if I can find the fuzz_coverage report, when it finishes running. If it still doesn’t improve, I’m gonna comb through my documentation of the process again to make sure I’m not setting myself up to fail in some manner, but if that still yields no explanation, I could probably use some help. |
I guess you'll have to cherry-pick your commit after the force push? |
At least compared to the CI logs we used for comparison above, I could find a few targets for which the coverage improved. Will rebase. |
0116b9d
to
10347b1
Compare
Yeah, seems fine to add inputs that trigger coverage internal to the sanitizer instrumentation. However, it would be better if there also was at least one real line of code covered additionally :) I'll go ahead and merge this nonetheless. |
Just checked for comparison on a 8-core vm, running for two weeks, the coverage increase was 35 lines and two new functions. (#233 (comment)) Happy to check your logs, if you want, to see if there is anything standing out. But maybe it was just a randomly odd run with little new coverage for you? 🤷♂️ |
That does make me think that something is wrong about my process. What data could I best provide to help? |
I think I’ll just try to craft another submission next week and see if it provides any additional code coverage. If it does, I’ll chalk it up to happenstance, if not, we could investigate any logs that seem useful. |
Yeah, in the logs I'd check how long it takes to start fuzzing. If there are too many input files, it can take a long time to iterate over all of them. |
Okay, I’ll check tomorrow. |
To clarify, I move aside the I could try to use I tried a few that I think might be among the slower targets:
|
Hmm, a few minutes look good and harmless, when you target a few hours of fuzzing. |
Following up on #229 with a second attempt to upstream my inputs.