C++: Make sure that nrOfBoundsNEPhi is functional

Author: paldepind

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

@@ -592,7 +592,7 @@ private module BoundsEstimate {
     not exists(def.getAPhiInput(v)) and
     // If there's different `access`es, then they refer to the same variable
     // with the same lower bounds. Hence adding these guards make no sense (the
-    // implementation will take the union but they'll be removed by
+    // implementation will take the union, but they'll be removed by
     // deduplication). Hence we use `max` as an approximation.
     result =
       max(VariableAccess access | isGuardPhiWithBound(def, v, access) | nrOfBoundsExpr(access))
@@ -624,8 +624,13 @@ private module BoundsEstimate {
    * Gets the number of bounds for `def` when `def` is an NE phi node for the
    * variable `v`.
    */
-  private float nrOfBoundsNEPhi(RangeSsaDefinition def, StackVariable v) {
-    exists(VariableAccess access | isNEPhi(v, def, access, _) and result = nrOfBoundsExpr(access))
+  language[monotonicAggregates]
+  float nrOfBoundsNEPhi(RangeSsaDefinition def, StackVariable v) {
+    // If there's different `access`es, then they refer to the same variable
+    // with the same lower bounds. Hence adding these guards make no sense (the
+    // implementation will take the union, but they'll be removed by
+    // deduplication). Hence we use `max` as an approximation.
+    result = max(VariableAccess access | isNEPhi(v, def, access, _) | nrOfBoundsExpr(access))
     or
     def.isPhiNode(v) and
     not isNEPhi(v, def, _, _) and
@@ -636,11 +641,14 @@ private module BoundsEstimate {
    * Gets the number of bounds for `def` when `def` is an unsupported guard phi
    * node for the variable `v`.
    */
+  language[monotonicAggregates]
   private float nrOfBoundsUnsupportedGuardPhi(RangeSsaDefinition def, StackVariable v) {
-    exists(VariableAccess access |
-      isUnsupportedGuardPhi(v, def, access) and
-      result = nrOfBoundsExpr(access)
-    )
+    // If there's different `access`es, then they refer to the same variable
+    // with the same lower bounds. Hence adding these guards make no sense (the
+    // implementation will take the union, but they'll be removed by
+    // deduplication). Hence we use `max` as an approximation.
+    result =
+      max(VariableAccess access | isUnsupportedGuardPhi(v, def, access) | nrOfBoundsExpr(access))
     or
     def.isPhiNode(v) and
     not isUnsupportedGuardPhi(v, def, _) and

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/nrOfBounds.expected

@@ -1456,14 +1456,10 @@ estimateNrOfBounds
 | test.c:434:7:434:7 | b | 1.0 |
 | test.c:434:7:434:12 | ... = ... | 1.0 |
 | test.c:434:11:434:12 | 10 | 1.0 |
-| test.c:439:11:439:11 | a | 3.0 |
 | test.c:439:11:439:11 | a | 4.0 |
-| test.c:439:11:439:15 | ... + ... | 12.0 |
 | test.c:439:11:439:15 | ... + ... | 16.0 |
 | test.c:439:15:439:15 | b | 4.0 |
-| test.c:440:10:440:10 | a | 3.0 |
 | test.c:440:10:440:10 | a | 4.0 |
-| test.c:440:10:440:14 | ... + ... | 12.0 |
 | test.c:440:10:440:14 | ... + ... | 16.0 |
 | test.c:440:14:440:14 | b | 4.0 |
 | test.c:447:4:449:50 | (...) | 1.0 |

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c

@@ -436,8 +436,8 @@ int ne_phi_nodes(int a, int b) {
   }
   // The statement below is an NE phi node for the access `a` in both `a == 17`
   // and `a == 18`.
-  int c = a + b; // $ nonFunctionalNrOfBounds
-  return a + b; // $ nonFunctionalNrOfBounds
+  int c = a + b;
+  return a + b;
 }
 
 unsigned int conditional_nested_guards(unsigned int ip) {