Skip to content

Commit b247433

Browse files
committed
C++: Handle guard phi nodes differently
1 parent 5be6470 commit b247433

File tree

2 files changed

+1354
-1330
lines changed

2 files changed

+1354
-1330
lines changed

cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,30 @@ private module BoundsEstimate {
565565
/** Gets the number of bounds for `def` and `v` as guard phi node. */
566566
language[monotonicAggregates]
567567
private float nrOfBoundsPhiGuard(RangeSsaDefinition def, StackVariable v) {
568+
// If we have
569+
//
570+
// if (x < c) { e1 }
571+
// e2
572+
//
573+
// then `e2` is both a guard phi node (guarded by `x < c`) and a normal
574+
// phi node (control is merged after the `if` statement).
575+
//
576+
// Assume `x` has `n` bounds. Then `n` bounds are propagated to the guard
577+
// phi node `{ e1 }` and, since `{ e1 }` is input to `e2` as a normal phi
578+
// node, `n` bounds are propagated to `e2`. If we also propagate the `n`
579+
// bounds to `e2` as a guard phi node, then we square the number of
580+
// bounds.
581+
//
582+
// However in practice `x < c` is going to cut down the number of bounds:
583+
// The tracked bounds can't flow to both branches as that would require
584+
// them to simultaneously be greater and smaller than `c`. To approximate
585+
// this better, the contribution from a guard phi node that is also a
586+
// normal phi node is 1.
587+
exists(def.getAPhiInput(v)) and
588+
isGuardPhiWithBound(def, v, _) and
589+
result = 1
590+
or
591+
not exists(def.getAPhiInput(v)) and
568592
// If there's different `access`es, then they refer to the same variable
569593
// with the same lower bounds. Hence adding these guards make no sense (the
570594
// implementation will take the union but they'll be removed by

0 commit comments

Comments
 (0)