-
Notifications
You must be signed in to change notification settings - Fork 1.8k
SSA: Add data flow integration layer #16884
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
Conversation
4f346dd to
6d9de86
Compare
6d9de86 to
ef1fba1
Compare
| * by calling `begin` (or related functions) on the variable `v`. | ||
| */ | ||
| predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) { | ||
| predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) { |
Check warning
Code scanning / CodeQL
Missing QLDoc for parameter
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
left a comment
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.
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`. */ |
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.
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]" } |
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.
Is the missing space intentional?
| override string toString() { result = e.toString() + " [postupdate]" } | |
| override string toString() { result = e.toString() + " [post update]" } |
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.
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.
MathiasVP
left a comment
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.
LGTM!
Co-authored-by: Mathias Vorreiter Pedersen <[email protected]>
|
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 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