C++: Get rid of the case range constant value with and instead implement 'rangeGuard'.

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll

@@ -125,41 +125,16 @@ module GuardsInput implements SharedGuards::InputSig<Cpp::Location, Instruction,
       // In order to have an "integer constant" for a switch case
       // we misuse the first instruction (which is always a NoOp instruction)
       // as a constant with the switch case's value.
-      exists(CaseEdge edge |
-        this = any(SwitchInstruction switch).getSuccessor(edge) and
-        value = edge.getValue().toInt()
+      exists(CaseEdge edge | this = any(SwitchInstruction switch).getSuccessor(edge) |
+        value = edge.getMaxValue().toInt()
+        or
+        value = edge.getMinValue().toInt()
       )
     }
 
     override int asIntegerValue() { result = value }
   }
 
-  /**
-   * The instruction representing the constant expression in a case statement.
-   *
-   * Since the IR does not have an instruction for this (as this is represented
-   * by the edge) we use the `NoOp` instruction which is always generated.
-   */
-  private class CaseConstant extends ConstantExpr instanceof NoOpInstruction {
-    SwitchInstruction switch;
-    SwitchEdge edge;
-
-    CaseConstant() { this = switch.getSuccessor(edge) }
-
-    override ConstantValue asConstantValue() {
-      exists(string minValue, string maxValue |
-        edge.getMinValue() = minValue and
-        edge.getMaxValue() = maxValue and
-        result.isRange(minValue, maxValue)
-      )
-    }
-
-    predicate hasEdge(SwitchInstruction switch_, SwitchEdge edge_) {
-      switch_ = switch and
-      edge_ = edge
-    }
-  }
-
   private predicate nonNullExpr(Instruction i) {
     i instanceof VariableAddressInstruction
     or
@@ -234,7 +209,12 @@ module GuardsInput implements SharedGuards::InputSig<Cpp::Location, Instruction,
     /**
      * Gets the constant expression of this case.
      */
-    ConstantExpr asConstantCase() { result.(CaseConstant).hasEdge(switch, edge) }
+    ConstantExpr asConstantCase() {
+      // Note: This only has a value if there is a unique value for the case.
+      // So the will not be a result when using the GCC case range extension.
+      // Instead, we model these using the `LogicInput_v1::rangeGuard` predicate.
+      result.asIntegerValue() = this.getEdge().getValue().toInt()
+    }
   }
 
   abstract private class BinExpr extends Expr instanceof BinaryInstruction {
@@ -446,6 +426,23 @@ private module LogicInput_v1 implements GuardsImpl::LogicInputSig {
     g1.(ConditionalBranchInstruction).getCondition() = g2 and
     v1.asBooleanValue() = v2.asBooleanValue()
   }
+
+  predicate rangeGuard(
+    GuardsImpl::PreGuard guard, GuardValue val, GuardsInput::Expr e, int k, boolean upper
+  ) {
+    exists(SwitchInstruction switch, string minValue, string maxValue |
+      switch.getSuccessor(EdgeKind::caseEdge(minValue, maxValue)) = guard and
+      e = switch.getExpression() and
+      minValue != maxValue and
+      val.asBooleanValue() = true
+    |
+      upper = false and
+      k = minValue.toInt()
+      or
+      upper = true and
+      k = maxValue.toInt()
+    )
+  }
 }
 
 class GuardValue = GuardsImpl::GuardValue;
@@ -1707,15 +1704,16 @@ private module Cached {
     exists(string minValue, string maxValue |
       test.getExpressionOperand() = op and
       exists(test.getSuccessor(EdgeKind::caseEdge(minValue, maxValue))) and
-      value.asConstantValue().isRange(minValue, maxValue) and
       minValue < maxValue
     |
       // op <= k => op < k - 1
       isLt = true and
-      maxValue.toInt() = k - 1
+      maxValue.toInt() = k - 1 and
+      value.isIntRange(k - 1, true)
       or
       isLt = false and
-      minValue.toInt() = k
+      minValue.toInt() = k and
+      value.isIntRange(k, false)
     )
   }
 

cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected

@@ -490,22 +490,22 @@
 | test.cpp:69:12:69:12 | i | i == 0 when i is 0 |
 | test.cpp:69:12:69:12 | i | i == 1 when i is 1 |
 | test.cpp:69:12:69:12 | i | i == 2 when i is 2 |
-| test.cpp:73:30:73:30 | i | i < 11 when i is 0..10 |
-| test.cpp:73:30:73:30 | i | i < 21 when i is 11..20 |
-| test.cpp:73:30:73:30 | i | i >= 0 when i is 0..10 |
-| test.cpp:73:30:73:30 | i | i >= 11 when i is 11..20 |
-| test.cpp:74:10:74:10 | i | i < 11 when i is 0..10 |
-| test.cpp:74:10:74:10 | i | i < 21 when i is 11..20 |
-| test.cpp:74:10:74:10 | i | i >= 0 when i is 0..10 |
-| test.cpp:74:10:74:10 | i | i >= 11 when i is 11..20 |
-| test.cpp:76:12:76:12 | i | i < 11 when i is 0..10 |
-| test.cpp:76:12:76:12 | i | i < 21 when i is 11..20 |
-| test.cpp:76:12:76:12 | i | i >= 0 when i is 0..10 |
-| test.cpp:76:12:76:12 | i | i >= 11 when i is 11..20 |
-| test.cpp:79:12:79:12 | i | i < 11 when i is 0..10 |
-| test.cpp:79:12:79:12 | i | i < 21 when i is 11..20 |
-| test.cpp:79:12:79:12 | i | i >= 0 when i is 0..10 |
-| test.cpp:79:12:79:12 | i | i >= 11 when i is 11..20 |
+| test.cpp:73:30:73:30 | i | i < 11 when i is Upper bound 10 |
+| test.cpp:73:30:73:30 | i | i < 21 when i is Upper bound 20 |
+| test.cpp:73:30:73:30 | i | i >= 0 when i is Lower bound 0 |
+| test.cpp:73:30:73:30 | i | i >= 11 when i is Lower bound 11 |
+| test.cpp:74:10:74:10 | i | i < 11 when i is Upper bound 10 |
+| test.cpp:74:10:74:10 | i | i < 21 when i is Upper bound 20 |
+| test.cpp:74:10:74:10 | i | i >= 0 when i is Lower bound 0 |
+| test.cpp:74:10:74:10 | i | i >= 11 when i is Lower bound 11 |
+| test.cpp:76:12:76:12 | i | i < 11 when i is Upper bound 10 |
+| test.cpp:76:12:76:12 | i | i < 21 when i is Upper bound 20 |
+| test.cpp:76:12:76:12 | i | i >= 0 when i is Lower bound 0 |
+| test.cpp:76:12:76:12 | i | i >= 11 when i is Lower bound 11 |
+| test.cpp:79:12:79:12 | i | i < 11 when i is Upper bound 10 |
+| test.cpp:79:12:79:12 | i | i < 21 when i is Upper bound 20 |
+| test.cpp:79:12:79:12 | i | i >= 0 when i is Lower bound 0 |
+| test.cpp:79:12:79:12 | i | i >= 11 when i is Lower bound 11 |
 | test.cpp:93:6:93:6 | c | c != 0 when c is true |
 | test.cpp:93:6:93:6 | c | c != 1 when c is false |
 | test.cpp:93:6:93:6 | c | c == 0 when c is false |
@@ -1304,7 +1304,7 @@
 | test.cpp:330:7:330:7 | b | b != 1 when b is false |
 | test.cpp:330:7:330:7 | b | b == 0 when b is false |
 | test.cpp:330:7:330:7 | b | b == 1 when b is true |
-| test.cpp:334:11:334:11 | x | x < 51 when x is 40..50 |
-| test.cpp:334:11:334:11 | x | x >= 40 when x is 40..50 |
-| test.cpp:338:9:338:9 | x | x < 51 when x is 40..50 |
-| test.cpp:338:9:338:9 | x | x >= 40 when x is 40..50 |
+| test.cpp:334:11:334:11 | x | x < 51 when x is Upper bound 50 |
+| test.cpp:334:11:334:11 | x | x >= 40 when x is Lower bound 40 |
+| test.cpp:338:9:338:9 | x | x < 51 when x is Upper bound 50 |
+| test.cpp:338:9:338:9 | x | x >= 40 when x is Lower bound 40 |

cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected

@@ -178,12 +178,18 @@
 | test.cpp:42:13:42:20 | call to getABool | true | test.cpp:43:9:45:23 | { ... } |
 | test.cpp:60:31:60:31 | i | 0 | test.cpp:62:5:64:12 | case ...: |
 | test.cpp:60:31:60:31 | i | 1 | test.cpp:65:5:66:10 | case ...: |
+| test.cpp:60:31:60:31 | i | 10 | test.cpp:62:5:64:12 | case ...: |
 | test.cpp:61:10:61:10 | i | 0 | test.cpp:62:5:64:12 | case ...: |
 | test.cpp:61:10:61:10 | i | 1 | test.cpp:65:5:66:10 | case ...: |
-| test.cpp:73:30:73:30 | i | 0..10 | test.cpp:75:5:77:12 | case ...: |
-| test.cpp:73:30:73:30 | i | 11..20 | test.cpp:78:5:79:10 | case ...: |
-| test.cpp:74:10:74:10 | i | 0..10 | test.cpp:75:5:77:12 | case ...: |
-| test.cpp:74:10:74:10 | i | 11..20 | test.cpp:78:5:79:10 | case ...: |
+| test.cpp:61:10:61:10 | i | 10 | test.cpp:62:5:64:12 | case ...: |
+| test.cpp:73:30:73:30 | i | Lower bound 0 | test.cpp:75:5:77:12 | case ...: |
+| test.cpp:73:30:73:30 | i | Lower bound 11 | test.cpp:78:5:79:10 | case ...: |
+| test.cpp:73:30:73:30 | i | Upper bound 10 | test.cpp:75:5:77:12 | case ...: |
+| test.cpp:73:30:73:30 | i | Upper bound 20 | test.cpp:78:5:79:10 | case ...: |
+| test.cpp:74:10:74:10 | i | Lower bound 0 | test.cpp:75:5:77:12 | case ...: |
+| test.cpp:74:10:74:10 | i | Lower bound 11 | test.cpp:78:5:79:10 | case ...: |
+| test.cpp:74:10:74:10 | i | Upper bound 10 | test.cpp:75:5:77:12 | case ...: |
+| test.cpp:74:10:74:10 | i | Upper bound 20 | test.cpp:78:5:79:10 | case ...: |
 | test.cpp:92:31:92:31 | c | not null | test.cpp:93:9:94:7 | { ... } |
 | test.cpp:93:6:93:6 | c | not null | test.cpp:93:9:94:7 | { ... } |
 | test.cpp:93:6:93:6 | c | true | test.cpp:93:9:94:7 | { ... } |
@@ -328,9 +334,7 @@
 | test.cpp:318:6:318:18 | ... != ... | true | test.cpp:318:21:320:3 | { ... } |
 | test.cpp:318:7:318:12 | ... < ... | 0 | test.cpp:320:10:322:3 | { ... } |
 | test.cpp:318:7:318:12 | ... < ... | not 0 | test.cpp:318:21:320:3 | { ... } |
-| test.cpp:327:46:327:46 | b | false | test.cpp:336:3:338:7 | case ...: |
 | test.cpp:327:46:327:46 | b | true | test.cpp:331:3:332:10 | { ... } |
-| test.cpp:329:11:329:13 | call to foo | 40..50 | test.cpp:336:3:338:7 | case ...: |
-| test.cpp:330:7:330:7 | b | false | test.cpp:336:3:338:7 | case ...: |
 | test.cpp:330:7:330:7 | b | true | test.cpp:331:3:332:10 | { ... } |
-| test.cpp:334:11:334:11 | x | 40..50 | test.cpp:336:3:338:7 | case ...: |
+| test.cpp:334:11:334:11 | x | Lower bound 40 | test.cpp:336:3:338:7 | case ...: |
+| test.cpp:334:11:334:11 | x | Upper bound 50 | test.cpp:336:3:338:7 | case ...: |

cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected

@@ -1295,12 +1295,8 @@ unary
 | test.cpp:318:6:318:18 | ... != ... | test.cpp:318:7:318:12 | ... < ... | != | 0 | test.cpp:318:21:320:3 | { ... } |
 | test.cpp:318:6:318:18 | ... != ... | test.cpp:318:7:318:12 | ... < ... | == | 0 | test.cpp:320:10:322:3 | { ... } |
 | test.cpp:327:46:327:46 | b | test.cpp:330:7:330:7 | b | != | 0 | test.cpp:331:3:332:10 | { ... } |
-| test.cpp:327:46:327:46 | b | test.cpp:330:7:330:7 | b | != | 1 | test.cpp:336:3:338:7 | case ...: |
-| test.cpp:327:46:327:46 | b | test.cpp:330:7:330:7 | b | == | 0 | test.cpp:336:3:338:7 | case ...: |
 | test.cpp:327:46:327:46 | b | test.cpp:330:7:330:7 | b | == | 1 | test.cpp:331:3:332:10 | { ... } |
 | test.cpp:330:7:330:7 | b | test.cpp:330:7:330:7 | b | != | 0 | test.cpp:331:3:332:10 | { ... } |
-| test.cpp:330:7:330:7 | b | test.cpp:330:7:330:7 | b | != | 1 | test.cpp:336:3:338:7 | case ...: |
-| test.cpp:330:7:330:7 | b | test.cpp:330:7:330:7 | b | == | 0 | test.cpp:336:3:338:7 | case ...: |
 | test.cpp:330:7:330:7 | b | test.cpp:330:7:330:7 | b | == | 1 | test.cpp:331:3:332:10 | { ... } |
 | test.cpp:334:11:334:11 | x | test.cpp:334:11:334:11 | x | < | 51 | test.cpp:336:3:338:7 | case ...: |
 | test.cpp:334:11:334:11 | x | test.cpp:334:11:334:11 | x | >= | 40 | test.cpp:336:3:338:7 | case ...: |