Skip to content

Commit 2c29f21

Browse files
committed
Shared: Address review comments.
1 parent 109b6a1 commit 2c29f21

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

shared/controlflow/codeql/controlflow/ControlFlow.qll

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ module Make<
278278
exists(Expr e |
279279
def.getDefinition() = e and
280280
exprHasValue(e, gv) and
281-
(exists(GuardValue gv0 | exprHasValue(e, gv0) and gv0.isSingleton()) implies gv.isSingleton())
281+
(any(GuardValue gv0 | exprHasValue(e, gv0)).isSingleton() implies gv.isSingleton())
282282
)
283283
}
284284

@@ -595,7 +595,7 @@ module Make<
595595
) {
596596
ssaControlsBranchEdge(t, bb1, bb2, gv) and
597597
(
598-
exists(GuardValue gv0 | ssaControlsBranchEdge(t, bb1, bb2, gv0) and gv0.isSingleton())
598+
any(GuardValue gv0 | ssaControlsBranchEdge(t, bb1, bb2, gv0)).isSingleton()
599599
implies
600600
gv.isSingleton()
601601
) and
@@ -694,7 +694,7 @@ module Make<
694694
* `src`, and `var` is a relevant splitting variable that gets (re-)defined
695695
* in `bb2` by `t`, which is not a phi node.
696696
*
697-
* `val` is the best known value for `t` in `bb2`.
697+
* `val` is the best known value that is relatable to `condgv` for `t` in `bb2`.
698698
*/
699699
private predicate stepSsaValueRedef(
700700
SourceVariable src, BasicBlock bb1, BasicBlock bb2, SourceVariable var, GuardValue condgv,
@@ -720,7 +720,8 @@ module Make<
720720
* `t2`, in `bb2` taking input from `t1` along this edge. Furthermore,
721721
* there is no further redefinition of `var` in `bb2`.
722722
*
723-
* `val` is the best value for `t1`/`t2` implied by taking this edge.
723+
* `val` is the best value that is relatable to `condgv` for `t1`/`t2`
724+
* implied by taking this edge.
724725
*/
725726
private predicate stepSsaValuePhi(
726727
SourceVariable src, BasicBlock bb1, BasicBlock bb2, SourceVariable var, GuardValue condgv,
@@ -747,7 +748,7 @@ module Make<
747748
* redefinition along this edge nor in `bb2`.
748749
*
749750
* Additionally, this edge implies that the SSA definition `t` of `var` has
750-
* value `val`.
751+
* value `val` and that `val` is relatable to `condgv`.
751752
*/
752753
private predicate stepSsaValueNoRedef(
753754
SourceVariable src, BasicBlock bb1, BasicBlock bb2, SourceVariable var, GuardValue condgv,
@@ -763,6 +764,12 @@ module Make<
763764
/**
764765
* Holds if the source `srcDef` in `srcBb` may reach `def` in `bb`. The
765766
* taken path takes splitting based on the value of `var` into account.
767+
*
768+
* When multiple `GuardValue`s can be chosen for `var`, we prioritize those
769+
* that are relatable to `condgv`, as that will help determine whether a
770+
* particular edge may be taken or not. Singleton values are prioritized
771+
* highly as they are in principle relatable to every other `GuardValue`.
772+
*
766773
* The pair `(tracked, val)` is the current SSA definition and known value
767774
* for `var` in `bb`.
768775
*/

0 commit comments

Comments
 (0)