Repository navigation
Ssa: Refactor caching - #23011
Open
aschackmull wants to merge 3 commits into
Open
Ssa: Refactor caching#23011aschackmull wants to merge 3 commits into
aschackmull wants to merge 3 commits into
Conversation
aschackmull
force-pushed
the
ssa/caching
branch
from
November 4, 2026 11:55
907afdd to
88c24ec
Compare
Contributor
Author
|
Dca is uneventful. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Removing the cached overrides leaves getARead uncached, so query-specific barrier guards may recompute the reaching-definition/read/CFG join instead of reusing cached results.
2 open issues
What changed in this PR
Refactors static single assignment (SSA) caching into the shared library, reducing cache management in language-specific data-flow implementations.
Changes:
- Adds an explicitly named cached stage for core SSA computation.
- Links
Make,MakeSsa, andDataFlowIntegrationthrough reverse-reference hooks. - Removes redundant language-specific caching and moves read lookup into the shared integration module.
| File | Description |
|---|---|
unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll |
Connects local SSA to the integration cache hook. |
shared/ssa/codeql/ssa/Ssa.qll |
Centralizes SSA caching, adds hooks, and internalizes read lookup. |
shared/dataflow/codeql/dataflow/VariableCapture.qll |
Connects capture SSA to the integration cache hook. |
rust/ql/lib/codeql/rust/dataflow/Ssa.qll |
Calls shared phi-input and prior-definition predicates directly. |
rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll |
Adds cache wiring and removes redundant wrappers. |
ruby/ql/lib/codeql/ruby/dataflow/SSA.qll |
Calls shared prior-definition and phi-input predicates directly. |
ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll |
Adds cache wiring and removes redundant wrappers. |
javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll |
Names the integration module and removes its read override. |
java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll |
Connects both cache hooks and removes the read override. |
java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll |
Connects base SSA to the MakeSsa cache hook. |
go/ql/lib/semmle/go/dataflow/SsaImpl.qll |
Removes redundant caching annotations and related notes. |
csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll |
Connects both cache hooks and removes the read override. |
csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll |
Connects base SSA to the MakeSsa cache hook. |
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll |
Replaces the local cache module with shared caching. |
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll |
Connects iterator SSA to the integration cache hook. |
Review effort: Max (auto)
Comment on lines
+68
to
+70
| * If `MakeSsa` and/or `DataFlowIntegration` is used, then this predicate | ||
| * should be instantiated with (the union of) their respective | ||
| * `ssaCachedStageRevRef` predicates. |
hvitved
reviewed
Nov 5, 2026
| } | ||
|
|
||
| /** Reverse references to the cached predicates in this module. */ | ||
| predicate ssaCachedStageRevRef2() { |
Contributor
There was a problem hiding this comment.
Should this call ssaCachedStageRevRef1?
Contributor
Author
There was a problem hiding this comment.
No, it should just be a list of the cached predicates in this module, so they can be referenced from SsaCachedStage::revRef().
hvitved
approved these changes
Nov 5, 2026
This branch has not been deployed
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 refactors SSA caching in two steps (2 separate commits). The SSA library consists of three main modules:
Make,MakeSsa, andDataFlowIntegration. First, the main SSA computation ofMakeis cached directly in a newly constructed explicitly named cached stage. This means that all the languages that aren't usingMakeSsano longer need to do this caching. Second, the caching in all three modules is tied together via a cached-stage-reverse-reference predicate from the nested modules passed intoMake. This means that the cachedgetAReadno longer needs to be threaded through on the language-specific side.The
ssaCachedStageRevRefpredicates were initially meant to be named without the 1,2 suffixes, but they were accidentally imported in common scopes in several places in Java and C#, which broke compilation, so that would have required a much bigger refactoring. Since they're just meant as internal names, the distinguishing suffixes was the simplest solution.