-
Notifications
You must be signed in to change notification settings - Fork 1.7k
SSA: Update data flow integration and BarrierGuard interface to use GuardValue. #20132
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR updates the SSA data flow integration interface to use a generic GuardValue
type instead of boolean
for guard expressions. This change refactors the internal API to support wrapper types for barrier guards while maintaining the same external behavior.
- Introduces a
GuardValue
class as an abstract value type for guard evaluations - Updates all guard-related predicates to use
GuardValue
instead ofboolean
- Replaces method names to include "Value" prefix (e.g.,
hasBranchEdge
→hasValueBranchEdge
)
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
shared/ssa/codeql/ssa/Ssa.qll | Defines the new GuardValue class and updates all guard interface predicates |
shared/dataflow/codeql/dataflow/VariableCapture.qll | Implements GuardValue as Void type for variable capture scenarios |
rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll | Updates Rust implementation to use Boolean as GuardValue type |
ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll | Updates Ruby implementation to use Boolean as GuardValue type |
javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll | Updates JavaScript SSA implementation with Boolean guard values |
javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll | Updates taint tracking to use Boolean type for guard checks |
javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll | Updates barrier guards implementation with Boolean type |
java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll | Updates Java implementation to use Guards::GuardValue type |
csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll | Updates C# implementation to use Boolean as GuardValue type |
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll | Updates C++ SSA internals to use Boolean guard values |
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll | Updates C++ iterator flow to use Void as GuardValue type |
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 (CPP, Rust). I'm on the fence about asking for DCA runs, I don't think this should affect correctness or performance but ... I've been wrong before.
Dca looks uneventful for all languages - except a weird OOM in sarif export for JS. But I would think that's unlikely to be related to this PR. |
Looks like the JS dca problem is unrelated - the same thing happened on #19854, which was approved and merged. |
This just tweaks the internal api in SSA to support BarrierGuard wrappers from #20121. Semantics and external interfaces remain unchanged.