Skip to content

Commit 0badcfd

Browse files
committed
C++: Address review comments
1 parent 979b05c commit 0badcfd

File tree

1 file changed

+23
-14
lines changed

1 file changed

+23
-14
lines changed

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

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -533,10 +533,9 @@ private module BoundsEstimate {
533533
result = 2.0.pow(40)
534534
}
535535

536-
/** Gets the maximum number of bounds possible when widening is used. */
537-
private int getNrOfWideningBounds() {
538-
result =
539-
max(ArithmeticType t | | count(wideningLowerBounds(t)).maximum(count(wideningUpperBounds(t))))
536+
/** Gets the maximum number of bounds possible for `t` when widening is used. */
537+
private int getNrOfWideningBounds(ArithmeticType t) {
538+
result = strictcount(wideningLowerBounds(t)).maximum(strictcount(wideningUpperBounds(t)))
540539
}
541540

542541
/**
@@ -554,15 +553,18 @@ private module BoundsEstimate {
554553
)
555554
}
556555

557-
/** Holds if `def` and `v` is a guard phi node with a bound from a guard. */
556+
/** Holds if `def` is a guard phi node for `v` with a bound from a guard. */
558557
predicate isGuardPhiWithBound(RangeSsaDefinition def, StackVariable v, VariableAccess access) {
559558
exists(Expr guard, boolean branch |
560559
def.isGuardPhi(v, access, guard, branch) and
561560
hasBoundFromGuard(guard, access, branch)
562561
)
563562
}
564563

565-
/** Gets the number of bounds for `def` and `v` as guard phi node. */
564+
/**
565+
* Gets the number of bounds for `def` when `def` is a guard phi node for the
566+
* variable `v`.
567+
*/
566568
language[monotonicAggregates]
567569
private float nrOfBoundsPhiGuard(RangeSsaDefinition def, StackVariable v) {
568570
// If we have
@@ -601,10 +603,12 @@ private module BoundsEstimate {
601603
result = 0
602604
}
603605

604-
/** Gets the number of bounds for `def` and `v` as normal phi node. */
606+
/**
607+
* Gets the number of bounds for `def` when `def` is a normal phi node for the
608+
* variable `v`.
609+
*/
605610
language[monotonicAggregates]
606611
private float nrOfBoundsPhiNormal(RangeSsaDefinition def, StackVariable v) {
607-
// The implementation
608612
result =
609613
strictsum(RangeSsaDefinition inputDef |
610614
inputDef = def.getAPhiInput(v)
@@ -617,7 +621,10 @@ private module BoundsEstimate {
617621
result = 0
618622
}
619623

620-
/** Gets the number of bounds for `def` and `v` as an NE phi node. */
624+
/**
625+
* Gets the number of bounds for `def` when `def` is an NE phi node for the
626+
* variable `v`.
627+
*/
621628
private float nrOfBoundsNEPhi(RangeSsaDefinition def, StackVariable v) {
622629
exists(VariableAccess access | isNEPhi(v, def, access, _) and result = nrOfBoundsExpr(access))
623630
or
@@ -626,7 +633,10 @@ private module BoundsEstimate {
626633
result = 0
627634
}
628635

629-
/** Gets the number of bounds for `def` and `v` as an unsupported guard phi node. */
636+
/**
637+
* Gets the number of bounds for `def` when `def` is an unsupported guard phi
638+
* node for the variable `v`.
639+
*/
630640
private float nrOfBoundsUnsupportedGuardPhi(RangeSsaDefinition def, StackVariable v) {
631641
exists(VariableAccess access |
632642
isUnsupportedGuardPhi(v, def, access) and
@@ -644,8 +654,7 @@ private module BoundsEstimate {
644654
// we sum the contributions from the different cases.
645655
result =
646656
nrOfBoundsPhiGuard(def, v) + nrOfBoundsPhiNormal(def, v) + nrOfBoundsNEPhi(def, v) +
647-
nrOfBoundsUnsupportedGuardPhi(def, v) and
648-
result != 0
657+
nrOfBoundsUnsupportedGuardPhi(def, v)
649658
}
650659

651660
/** Gets the estimated number of bounds for `def` and `v`. */
@@ -656,7 +665,7 @@ private module BoundsEstimate {
656665
// estimate. Had that not been the case the estimate itself would be at risk
657666
// of causing performance issues and being non-functional.
658667
if isRecursiveDef(def, v)
659-
then result = getNrOfWideningBounds()
668+
then result = getNrOfWideningBounds(getVariableRangeType(v))
660669
else (
661670
// Definitions with a defining value
662671
exists(Expr defExpr | assignmentDef(def, v, defExpr) and result = nrOfBoundsExpr(defExpr))
@@ -719,7 +728,7 @@ private module BoundsEstimate {
719728
// Similarly to what we do for definitions, we do not attempt to measure the
720729
// number of bounds for recursive expressions.
721730
if isRecursiveExpr(e)
722-
then result = getNrOfWideningBounds()
731+
then result = getNrOfWideningBounds(e.getUnspecifiedType())
723732
else
724733
if analyzableExpr(e)
725734
then

0 commit comments

Comments
 (0)