Skip to content

Commit ab09ae5

Browse files
committed
C++: Handle guard phi nodes differently
1 parent 9e17cdd commit ab09ae5

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

0 commit comments

Comments
 (0)