Shared: Support flow summaries from ReturnValues#22061
Draft
MathiasVP wants to merge 14 commits into
Draft
Conversation
… from the return value of a call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
std::stringclass in C++ is mutable so you can do:This is just a regular assignment where the left-hand side is a reference to the internal buffer. Historically, to support this in C++ taint-tracking we've specified "reverse flows":
codeql/cpp/ql/lib/semmle/code/cpp/models/implementations/StdString.qll
Lines 384 to 386 in b8c78fd
So when an assignment flows from the right-hand side to the left-hand side (i.e., to the returned reference (actually to the indirections of the reference, but nevermind)) the "reverse flow" model transfers flow to the qualifier.
We'd obviously like to get rid of all these old models and replace them with flow summaries. However, flow summaries do not allow for these reverse flow summaries. This PR fixes that so that we can model this using a summary such as
ReturnValue[*] -> Argument[this].Most of the work is in bb2ec12. We add a new summary node (which is expected to be lifted to a proper
ArgumentNodeby each language) along with a new parameter/argument position to transfer the value into the summarized callable. 662f522 shows how the new predicates are instantiated for C/C++.Commit-by-commit review recommended.
This PR doesn't actually add any real models to C/C++. However, I have a local branch where I've checked on DCA that the performance of these changes are fine.