Repository navigation
Ssa: Refactor DataFlowIntegration - #23070
Merged
Merged
Conversation
aschackmull
added
the
no-change-note-required
github-actions
Bot
added
C#
JS
C++
Java
Ruby
Rust
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
An unresolved documentation nit in Ssa.qll leaves the renamed read-node class summary and getSourceVariable() documentation using outdated expression-node terminology.
1 open issue
What changed in this PR
Refactors shared SSA data-flow integration to represent reads by control-flow node and source variable, replacing the misleading Expr abstraction across language integrations.
Changes:
- Introduces
ReadNode,PostUpdateReadNode, andreadsAtAPIs. - Simplifies post-update inputs and updates shared flow and guard logic.
- Migrates language integrations and adds variable-aware joins for C++ iterator and variable-capture reads.
| File | Description |
|---|---|
| unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll | Adopts post-update inputs and new read-node mappings. |
| shared/ssa/codeql/ssa/Ssa.qll | Defines read-node APIs and updates SSA flow logic. |
| shared/dataflow/codeql/dataflow/VariableCapture.qll | Joins capture reads by CFG position and variable. |
| rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll | Removes the obsolete expression adapter. |
| rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll | Maps new read nodes through control-flow nodes. |
| ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll | Removes the obsolete expression adapter. |
| ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll | Updates read and post-update conversions. |
| javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll | Removes the obsolete expression adapter. |
| javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll | Converts new SSA read types to data-flow nodes. |
| java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll | Removes the obsolete expression adapter. |
| java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll | Maps reads through expression control-flow nodes. |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll | Removes the obsolete expression adapter. |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll | Updates mappings to the new read-node types. |
| cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll | Removes the obsolete instruction-based expression adapter. |
| cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll | Removes the iterator adapter and makes read joins variable-aware. |
Review effort: Max (auto)
Contributor
Author
|
Dca is uneventful. |
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
Exprtype inDataFlowIntegrationin SSA is was misleading - it wasn't really an expression, but rather a control flow node. This PR refactors the interface to get rid of this type and with it hopefully any associated mistakes.Matching the resulting read nodes to data flow nodes ought to be done with a join on both control flow node and sourcevariable, but joining on node alone can be ok if there are no coinciding reads. I've generally left this alone, as most languages probably don't have coinciding reads, but I've updated C++ and the VariableCapture library in this regard. Unified already joined on both columns.
Review is intended commit-by-commit, but the changes aren't that big, so end-to-end is also doable if preferred.