-
Notifications
You must be signed in to change notification settings - Fork 1.7k
C++: Add more MaD summaries #19753
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
C++: Add more MaD summaries #19753
Conversation
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.
Pull Request Overview
This PR adds new MaD flow summaries for several C++ libraries, updates the test expectations to use the newly generated model IDs, and adjusts the bulk generation targets list to include the new libraries.
- Updated expected IDs in
flow.expected
to match the latest bulk generation run. - Added a new change-note entry documenting the added MaD summaries.
- Expanded
bulk_generation_targets.yml
to configure generation for the listed libraries.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
File | Description |
---|---|
cpp/ql/test/library-tests/dataflow/external-models/flow.expected | Bumped MaD summary IDs to align with the new bulk generator output. |
cpp/ql/src/change-notes/2025-06-13-mad-summaries.md | Documented the addition of new MaD flow models. |
cpp/bulk_generation_targets.yml | Added targets for zlib, brotli, libidn2, libssh2, nghttp2, libuv, and curl. |
Comments suppressed due to low confidence (3)
cpp/bulk_generation_targets.yml:12
- The target name for
libssh2
was removed but itswith-sinks
/with-sources
lines remain, making the YAML invalid. Add back- name: libssh2
before these lines to restore a proper list item.
with-sinks: false
cpp/bulk_generation_targets.yml:13
- The
sqlite
entry appears to have been removed, but itswith-sinks
/with-sources
lines are still present. Remove these orphan lines or re-add- name: sqlite
if the target should be kept.
with-sources: false
cpp/bulk_generation_targets.yml:20
- [nitpick] The
openssl
target is reintroduced here but not mentioned in the PR description. Confirm whether this was intentional or remove it to keep the target list aligned with the documented libraries.
- name: openssl
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.
Changes LGTM, as do your choice of projects. Bit of a shame nothing showed up on DCA.
- name: openssl | ||
- name: zlib | ||
with-sinks: false | ||
with-sources: false |
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.
Why exclude sources and sinks?
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.
See my answer to a similar question here. TLDR: We don't yet autogenerate sources and sinks for C/C++ (mostly because I didn't bother to do it yet).
This PR adds MaD summaries for the following libraries:
The models have been generated using the bulk generator script as of 1a36374 (which fixes a dependency ordering problem)