-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
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
gh-107652: Set up CIFuzz to run fuzz targets continuously #107653
Conversation
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
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.
Some nits
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
I'm mindful of CI availability and note this adds three slow, ~23-28 minute jobs: Is there any caching that can be used to speed them up? Should we restrict these jobs to only trigger when C files change, and not Python/other support files? And, if we've already run this on PRs, do we also need to run on |
I'd second this, especially if given OSS Fuzz's runs (per the comment at the top of the new job). A |
I agree, but it seems that CIFuzz does not support any settings for caching and Python compilation happens in its Docker containers. There is a
This sounds like a good idea!
Running on PR updates should be enough. |
I don't understand well which Python is tested. Does the job builds Python and run the fuzzer on the freshly built Python? CIFuzz (address) pass but it see a log with an error:
|
@vstinner please check "How it works". It builds CPython from the source at a particular pull request, and it does not report crashes that occurred on older OSS-Fuzz builds. You can find that particular error in the list of known issues here. |
Could CIFuzz be run as a buildbot instead of on CI? There's been a concerted effort to reduce CI times as of late, and I do worry that this PR adds a substantial amount of work for little apparent benefit, given that OSS Fuzz runs anyway on actual commits. If we could do a "test with fuzz buildbots" label, that might be a compromise? A |
If the CI job is not mandatory and doesn't prevent to merge a PR, I think that it's acceptable to use GHA resources. |
I am not sure how often the label will be set, it is pretty likely that this approach will not solve the problem of OSS-Fuzz builds being left broken from time to time |
@ezio-melotti @hugovk is there anything I can do here to make this PR ready for merging? |
@illia-v It's the core dev sprint next week, I'll bring it up for discussion then. I think fuzzing can be extremely valuable, and we need to balance how long it runs so we're not waiting too long for PRs to complete. |
It seems @15r10nk found several issues that OSS-Fuzz missed, I assume using a different approach. If we're to adopt fuzzing, perhaps we should try his mechanism that seems to find more (release-blocking!) issues? A |
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.
Hi! Let's give this a shot! It's a bit slow, but it's about the same as the Windows jobs. We're trying to make the Windows ones faster, and if the duration fuzzing jobs end up being a problem, we can address it then.
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.
Thank you!
Is there a doc somewhere explaining how to handle a CIFuzz failure? Or this CI never fails? :-) |
I just would like to know who I should contact if it goes wrong. @hugovk if I understood correctly :-) |
If/when it fails, let's open an issue and figure it out :) |
We've been seeing CIFuzz failures on otherwise good PRs not related to the PR change at hand of late with the build failing:
these are understandably confusing people. |
|
…on#107653) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
This sets up CIFuzz to run fuzz targets with three types of sanitizers for every pull request to the main branch and every commit in the main branch.