Skip to content

Commit 1b2d23b

Browse files
aschackmullaschackmull
authored
Merge pull request #19567 from aschackmull/ssa/branchedge
SSA: Distinguish between has and controls branch edge.
2 parents 909c1bb + f4fb717 commit 1b2d23b

File tree

11 files changed

+103
-31
lines changed

11 files changed

+103
-31
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,6 +1903,10 @@ module IteratorFlow {
19031903
predicate allowFlowIntoUncertainDef(IteratorSsa::UncertainWriteDefinition def) { any() }
19041904

19051905
class Guard extends Void {
1906+
predicate hasBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
1907+
none()
1908+
}
1909+
19061910
predicate controlsBranchEdge(
19071911
SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch
19081912
) {

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,13 +991,17 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
991991
class Guard instanceof IRGuards::IRGuardCondition {
992992
string toString() { result = super.toString() }
993993

994-
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
994+
predicate hasBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
995995
exists(EdgeKind kind |
996996
super.getBlock() = bb1 and
997997
kind = getConditionalEdge(branch) and
998998
bb1.getSuccessor(kind) = bb2
999999
)
10001000
}
1001+
1002+
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
1003+
this.hasBranchEdge(bb1, bb2, branch)
1004+
}
10011005
}
10021006

10031007
predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,17 +1044,25 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
10441044

10451045
class Guard extends Guards::Guard {
10461046
/**
1047-
* Holds if the control flow branching from `bb1` is dependent on this guard,
1048-
* and that the edge from `bb1` to `bb2` corresponds to the evaluation of this
1049-
* guard to `branch`.
1047+
* Holds if the evaluation of this guard to `branch` corresponds to the edge
1048+
* from `bb1` to `bb2`.
10501049
*/
1051-
predicate controlsBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) {
1050+
predicate hasBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) {
10521051
exists(ControlFlow::SuccessorTypes::ConditionalSuccessor s |
10531052
this.getAControlFlowNode() = bb1.getLastNode() and
10541053
bb2 = bb1.getASuccessorByType(s) and
10551054
s.getValue() = branch
10561055
)
10571056
}
1057+
1058+
/**
1059+
* Holds if this guard evaluating to `branch` controls the control-flow
1060+
* branch edge from `bb1` to `bb2`. That is, following the edge from
1061+
* `bb1` to `bb2` implies that this guard evaluated to `branch`.
1062+
*/
1063+
predicate controlsBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) {
1064+
this.hasBranchEdge(bb1, bb2, branch)
1065+
}
10581066
}
10591067

10601068
/** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */

‎java/ql/lib/semmle/code/java/controlflow/Guards.qll

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,15 @@ class Guard extends ExprParent {
272272
preconditionControls(this, controlled, branch)
273273
}
274274

275+
/**
276+
* Holds if this guard evaluating to `branch` controls the control-flow
277+
* branch edge from `bb1` to `bb2`. That is, following the edge from
278+
* `bb1` to `bb2` implies that this guard evaluated to `branch`.
279+
*/
280+
predicate controlsBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) {
281+
guardControlsBranchEdge_v3(this, bb1, bb2, branch)
282+
}
283+
275284
/**
276285
* Holds if this guard evaluating to `branch` directly or indirectly controls
277286
* the block `controlled`. That is, the evaluation of `controlled` is
@@ -351,6 +360,18 @@ private predicate guardControls_v3(Guard guard, BasicBlock controlled, boolean b
351360
)
352361
}
353362

363+
pragma[nomagic]
364+
private predicate guardControlsBranchEdge_v3(
365+
Guard guard, BasicBlock bb1, BasicBlock bb2, boolean branch
366+
) {
367+
guard.hasBranchEdge(bb1, bb2, branch)
368+
or
369+
exists(Guard g, boolean b |
370+
guardControlsBranchEdge_v3(g, bb1, bb2, b) and
371+
implies_v3(g, b, guard, branch)
372+
)
373+
}
374+
354375
private predicate equalityGuard(Guard g, Expr e1, Expr e2, boolean polarity) {
355376
exists(EqualityTest eqtest |
356377
eqtest = g and

‎java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -654,16 +654,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
654654
def instanceof SsaUncertainImplicitUpdate
655655
}
656656

657-
class Guard extends Guards::Guard {
658-
/**
659-
* Holds if the control flow branching from `bb1` is dependent on this guard,
660-
* and that the edge from `bb1` to `bb2` corresponds to the evaluation of this
661-
* guard to `branch`.
662-
*/
663-
predicate controlsBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) {
664-
super.hasBranchEdge(bb1, bb2, branch)
665-
}
666-
}
657+
class Guard = Guards::Guard;
667658

668659
/** Holds if the guard `guard` directly controls block `bb` upon evaluating to `branch`. */
669660
predicate guardDirectlyControlsBlock(Guard guard, BasicBlock bb, boolean branch) {

‎javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,26 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig {
7575
Guard() { this = any(js::ConditionGuardNode g).getTest() }
7676

7777
/**
78-
* Holds if the control flow branching from `bb1` is dependent on this guard,
79-
* and that the edge from `bb1` to `bb2` corresponds to the evaluation of this
80-
* guard to `branch`.
78+
* Holds if the evaluation of this guard to `branch` corresponds to the edge
79+
* from `bb1` to `bb2`.
8180
*/
82-
predicate controlsBranchEdge(js::BasicBlock bb1, js::BasicBlock bb2, boolean branch) {
81+
predicate hasBranchEdge(js::BasicBlock bb1, js::BasicBlock bb2, boolean branch) {
8382
exists(js::ConditionGuardNode g |
8483
g.getTest() = this and
8584
bb1 = this.getBasicBlock() and
8685
bb2 = g.getBasicBlock() and
8786
branch = g.getOutcome()
8887
)
8988
}
89+
90+
/**
91+
* Holds if this guard evaluating to `branch` controls the control-flow
92+
* branch edge from `bb1` to `bb2`. That is, following the edge from
93+
* `bb1` to `bb2` implies that this guard evaluated to `branch`.
94+
*/
95+
predicate controlsBranchEdge(js::BasicBlock bb1, js::BasicBlock bb2, boolean branch) {
96+
this.hasBranchEdge(bb1, bb2, branch)
97+
}
9098
}
9199

92100
pragma[inline]

‎ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -484,17 +484,25 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
484484

485485
class Guard extends Cfg::CfgNodes::AstCfgNode {
486486
/**
487-
* Holds if the control flow branching from `bb1` is dependent on this guard,
488-
* and that the edge from `bb1` to `bb2` corresponds to the evaluation of this
489-
* guard to `branch`.
487+
* Holds if the evaluation of this guard to `branch` corresponds to the edge
488+
* from `bb1` to `bb2`.
490489
*/
491-
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
490+
predicate hasBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
492491
exists(Cfg::SuccessorTypes::ConditionalSuccessor s |
493492
this.getBasicBlock() = bb1 and
494493
bb2 = bb1.getASuccessor(s) and
495494
s.getValue() = branch
496495
)
497496
}
497+
498+
/**
499+
* Holds if this guard evaluating to `branch` controls the control-flow
500+
* branch edge from `bb1` to `bb2`. That is, following the edge from
501+
* `bb1` to `bb2` implies that this guard evaluated to `branch`.
502+
*/
503+
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
504+
this.hasBranchEdge(bb1, bb2, branch)
505+
}
498506
}
499507

500508
/** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */

‎rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,17 +363,25 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
363363

364364
class Guard extends CfgNodes::AstCfgNode {
365365
/**
366-
* Holds if the control flow branching from `bb1` is dependent on this guard,
367-
* and that the edge from `bb1` to `bb2` corresponds to the evaluation of this
368-
* guard to `branch`.
366+
* Holds if the evaluation of this guard to `branch` corresponds to the edge
367+
* from `bb1` to `bb2`.
369368
*/
370-
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
369+
predicate hasBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
371370
exists(Cfg::ConditionalSuccessor s |
372371
this = bb1.getANode() and
373372
bb2 = bb1.getASuccessor(s) and
374373
s.getValue() = branch
375374
)
376375
}
376+
377+
/**
378+
* Holds if this guard evaluating to `branch` controls the control-flow
379+
* branch edge from `bb1` to `bb2`. That is, following the edge from
380+
* `bb1` to `bb2` implies that this guard evaluated to `branch`.
381+
*/
382+
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
383+
this.hasBranchEdge(bb1, bb2, branch)
384+
}
377385
}
378386

379387
/** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */

‎shared/dataflow/codeql/dataflow/VariableCapture.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,8 @@ module Flow<LocationSig Location, InputSig<Location> Input> implements OutputSig
732732
}
733733

734734
class Guard extends Void {
735+
predicate hasBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) { none() }
736+
735737
predicate controlsBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch) { none() }
736738
}
737739

‎shared/ssa/change-notes/2025-05-23-guards-interface.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: breaking
3+
---
4+
* Adjusted the Guards interface in the SSA data flow integration to distinguish `hasBranchEdge` from `controlsBranchEdge`. Any breakage can be fixed by implementing one with the other as a reasonable fallback solution.

‎shared/ssa/codeql/ssa/Ssa.qll

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,9 +1570,23 @@ module Make<LocationSig Location, InputSig<Location> Input> {
15701570
string toString();
15711571

15721572
/**
1573-
* Holds if the control flow branching from `bb1` is dependent on this guard,
1574-
* and that the edge from `bb1` to `bb2` corresponds to the evaluation of this
1575-
* guard to `branch`.
1573+
* Holds if the evaluation of this guard to `branch` corresponds to the edge
1574+
* from `bb1` to `bb2`.
1575+
*/
1576+
predicate hasBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch);
1577+
1578+
/**
1579+
* Holds if this guard evaluating to `branch` controls the control-flow
1580+
* branch edge from `bb1` to `bb2`. That is, following the edge from
1581+
* `bb1` to `bb2` implies that this guard evaluated to `branch`.
1582+
*
1583+
* This predicate differs from `hasBranchEdge` in that it also covers
1584+
* indirect guards, such as:
1585+
* ```
1586+
* b = guard;
1587+
* ...
1588+
* if (b) { ... }
1589+
* ```
15761590
*/
15771591
predicate controlsBranchEdge(BasicBlock bb1, BasicBlock bb2, boolean branch);
15781592
}
@@ -1667,7 +1681,7 @@ module Make<LocationSig Location, InputSig<Location> Input> {
16671681
(
16681682
// The input node is relevant either if it sits directly on a branch
16691683
// edge for a guard,
1670-
exists(DfInput::Guard g | g.controlsBranchEdge(input, phi.getBasicBlock(), _))
1684+
exists(DfInput::Guard g | g.hasBranchEdge(input, phi.getBasicBlock(), _))
16711685
or
16721686
// or if the unique predecessor is not an equivalent substitute in
16731687
// terms of being controlled by the same guards.

0 commit comments

Comments
 (0)