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

C++: Don't generate dataflow nodes for functions with summaries #18592

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MathiasVP
Copy link
Contributor

Not generating dataflow nodes for instructions/operands inside summarized callables has a big impact when using STL since the bodies of the instantiations are always in the database. So by excluding these we will avoid generating dataflow nodes in all of the instantiations which may be pretty significant if there are many instantiations.

Additionally, when we have flow through a function both through MaD and through the source code we may unnecessarily hit the field flow branch limit by duplicating flow. Staying below the field flow branch limit means we allow field flow in more functions.

DCA results looks good. Here's my breakdown of them:

cpp/path-injection

We gain 1 new result on cpp/path-injection. I've confirmed that this is because we now stay below the default field flow branch limit (i.e., 2) for flow out of this call to push_back. Before, we had out flow from both the MaD summary and the source code which resulted in going over the limit. But now we only have the MaD summary-proided out flow which keeps us below the threshold. So, unlike on main, field flow is now permitted in the enclosing function.

cpp/non-constant-format

We lose 60 results on SAMATE for this query. They all appear to be false positives that happen because of the generous isSource in the query that makes us start flow at some random output parameter of a call to delete deep inside the destructor of an iterator inside the libstdc++. Obviously, that's not what the query is supposed to be finding and I doubt that any of our queries will benefit from starting flow deep inside the implementation of a MaD summarized function.

@github-actions github-actions bot added the C++ label Jan 25, 2025
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Jan 25, 2025
@MathiasVP MathiasVP marked this pull request as ready for review January 26, 2025 21:13
@Copilot Copilot bot review requested due to automatic review settings January 26, 2025 21:13
@MathiasVP MathiasVP requested a review from a team as a code owner January 26, 2025 21:13

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant