SSA: Add data flow integration layer#16884
Merged
hvitved merged 3 commits intogithub:mainfrom Jul 10, 2024
Merged
Conversation
4f346dd to
6d9de86
Compare
6d9de86 to
ef1fba1
Compare
3e93c31 to
59a5b34
Compare
59a5b34 to
4d0d06d
Compare
4d0d06d to
e04847a
Compare
69608af to
f8fd8a8
Compare
b1794cc to
35c9233
Compare
35c9233 to
bd303e8
Compare
a93296e to
0319e00
Compare
8a8e8fc to
76751a5
Compare
76751a5 to
aeec768
Compare
aeec768 to
d41eae6
Compare
MathiasVP
previously approved these changes
Jul 9, 2024
Contributor
MathiasVP
left a comment
There was a problem hiding this comment.
Only a few small comments, but otherwise this LGTM!
shared/ssa/codeql/ssa/Ssa.qll
Outdated
| ) | ||
| } | ||
|
|
||
| /** Holds if there is a local flow step from `nodeFrom` to `nodeTo`. */ |
Contributor
There was a problem hiding this comment.
Should probably also mention isUseStep in the QLDoc.
| /** Gets the pre-update expression node. */ | ||
| ExprNode getPreUpdateNode() { result = TExprNode(e, false) } | ||
|
|
||
| override string toString() { result = e.toString() + " [postupdate]" } |
Contributor
There was a problem hiding this comment.
Is the missing space intentional?
Suggested change
| override string toString() { result = e.toString() + " [postupdate]" } | |
| override string toString() { result = e.toString() + " [post update]" } |
Contributor
Author
There was a problem hiding this comment.
I simply copied it from the variable capture library. However, these nodes should never actually be rendered, because they will be mapped to existing DataFlow::Nodes with appropriate toStrings.
Co-authored-by: Mathias Vorreiter Pedersen <mathiasvp@github.com>
MathiasVP
approved these changes
Jul 10, 2024
Contributor
Author
|
I'm going to ignore the failing C++ test, as it cannot possibly be caused by this PR (which adds, for now, dead code). |
This was referenced Jul 12, 2024
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.
This PR adds a new module,
DataFlowIntegration, to the shared SSA library, which makes it much easier to integrate SSA into the shared data flow library. The integration layer gives you all the relevant flow steps (such as use-use), but also gives you phi-reads and phi-input barriers without having to worry about how they are implemented.The interface is inspired by the variable capture library, meaning that we construct a
Nodetype where some branches should map to existing branches in theDataFlow::Nodetype, while theSsaNodesub class should be added as a new branch toDataFlow::Node.This PR only adds the new integration layer; follow-up PRs will adopt it for C#, Ruby, and Java.
Examples
The examples below show which local flow steps are generated for a particular code snippet.
Example 1
Example 2
Example 3
Example 4
Example 5