C++: Fix up uses of GuardCondition.

Author: MathiasVP

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll

@@ -171,7 +171,7 @@ private predicate bestOperandBound(Operand op, Bound b, int delta, boolean upper
 private IRGuardCondition eqFlowCond(
   ValueNumber vn, Operand bound, int delta, boolean isEq, boolean testIsTrue
 ) {
-  result.comparesEq(vn.getAUse(), bound, delta, isEq, testIsTrue)
+  result.comparesEq(vn, valueNumberOfOperand(bound), delta, isEq, testIsTrue)
 }
 
 /**
@@ -207,7 +207,7 @@ private IRGuardCondition boundFlowCond(
   ValueNumber vn, NonPhiOperand bound, int delta, boolean upper, boolean testIsTrue
 ) {
   exists(int d |
-    result.comparesLt(vn.getAUse(), bound, d, upper, testIsTrue) and
+    result.comparesLt(vn, valueNumberOfOperand(bound), d, upper, testIsTrue) and
     // `comparesLt` provides bounds of the form `x < y + k` or `x >= y + k`, but we need
     // `x <= y + k` so we strengthen here. `testIsTrue` has the same semantics in `comparesLt` as
     // it does here, so we don't need to account for it.

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/SignAnalysis.qll

@@ -297,16 +297,17 @@ private predicate unknownSign(Instruction i) {
 private predicate lowerBound(
   IRGuardCondition comp, Operand lowerbound, Operand bounded, boolean isStrict
 ) {
-  exists(int adjustment, Operand compared |
-    valueNumberOfOperand(bounded) = valueNumberOfOperand(compared) and
+  exists(int adjustment, ValueNumber compared |
+    valueNumberOfOperand(bounded) = compared and
     (
       isStrict = true and
       adjustment = 0
       or
       isStrict = false and
       adjustment = 1
     ) and
-    comp.ensuresLt(lowerbound, compared, adjustment, bounded.getUse().getBlock(), true)
+    comp.ensuresLt(valueNumberOfOperand(lowerbound), compared, adjustment,
+      bounded.getUse().getBlock(), true)
   )
 }
 
@@ -317,16 +318,17 @@ private predicate lowerBound(
 private predicate upperBound(
   IRGuardCondition comp, Operand upperbound, Operand bounded, boolean isStrict
 ) {
-  exists(int adjustment, Operand compared |
-    valueNumberOfOperand(bounded) = valueNumberOfOperand(compared) and
+  exists(int adjustment, ValueNumber compared |
+    valueNumberOfOperand(bounded) = compared and
     (
       isStrict = true and
       adjustment = 0
       or
       isStrict = false and
       adjustment = 1
     ) and
-    comp.ensuresLt(compared, upperbound, adjustment, bounded.getUse().getBlock(), true)
+    comp.ensuresLt(compared, valueNumberOfOperand(upperbound), adjustment,
+      bounded.getUse().getBlock(), true)
   )
 }
 
@@ -338,9 +340,9 @@ private predicate upperBound(
  *  - `isEq = false` : `bounded != eqbound`
  */
 private predicate eqBound(IRGuardCondition guard, Operand eqbound, Operand bounded, boolean isEq) {
-  exists(Operand compared |
-    valueNumberOfOperand(bounded) = valueNumberOfOperand(compared) and
-    guard.ensuresEq(compared, eqbound, 0, bounded.getUse().getBlock(), isEq)
+  exists(ValueNumber compared |
+    valueNumberOfOperand(bounded) = compared and
+    guard.ensuresEq(compared, valueNumberOfOperand(eqbound), 0, bounded.getUse().getBlock(), isEq)
   )
 }
 

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

@@ -8,12 +8,13 @@ import semmle.code.cpp.controlflow.BasicBlocks
 import semmle.code.cpp.controlflow.SSA
 import semmle.code.cpp.controlflow.Dominance
 import IRGuards
+private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 
 /** An `SsaDefinition` with an additional predicate `isLt`. */
 class GuardedSsa extends SsaDefinition {
   /** Holds if this `SsaDefinition` is guarded such that `this(var) < gt + k` is `testIsTrue` in `block`. */
-  predicate isLt(StackVariable var, Expr gt, int k, BasicBlock block, boolean testIsTrue) {
-    exists(Expr luse, GuardCondition test | this.getAUse(var) = luse |
+  predicate isLt(StackVariable var, GVN gt, int k, BasicBlock block, boolean testIsTrue) {
+    exists(GVN luse, GuardCondition test | this.getAUse(var) = luse.getAnExpr() |
       test.ensuresLt(luse, gt, k, block, testIsTrue)
     )
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

@@ -1238,12 +1238,16 @@ module IsUnreachableInCall {
   }
 
   pragma[nomagic]
-  private predicate ensuresEq(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
+  private predicate ensuresEq(
+    ValueNumber left, ValueNumber right, int k, IRBlock block, boolean areEqual
+  ) {
     any(G::IRGuardCondition guard).ensuresEq(left, right, k, block, areEqual)
   }
 
   pragma[nomagic]
-  private predicate ensuresLt(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
+  private predicate ensuresLt(
+    ValueNumber left, ValueNumber right, int k, IRBlock block, boolean areEqual
+  ) {
     any(G::IRGuardCondition guard).ensuresLt(left, right, k, block, areEqual)
   }
 
@@ -1256,13 +1260,13 @@ module IsUnreachableInCall {
   predicate isUnreachableInCall(NodeRegion block, DataFlowCall call) {
     exists(
       InstructionDirectParameterNode paramNode, ConstantIntegralTypeArgumentNode arg,
-      IntegerConstantInstruction constant, int k, Operand left, Operand right, int argval
+      IntegerConstantInstruction constant, int k, ValueNumber left, ValueNumber right, int argval
     |
       // arg flows into `paramNode`
       DataFlowImplCommon::viableParamArg(call, pragma[only_bind_into](paramNode),
         pragma[only_bind_into](arg)) and
-      left = constant.getAUse() and
-      right = valueNumber(paramNode.getInstruction()).getAUse() and
+      left.getAnInstruction() = constant and
+      right.getAnInstruction() = paramNode.getInstruction() and
       argval = arg.getValue()
     |
       // and there's a guard condition which ensures that the result of `left == right + k` is `areEqual`

cpp/ql/lib/semmle/code/cpp/ir/internal/ASTValueNumbering.qll

@@ -109,6 +109,9 @@ class GVN extends TValueNumber {
 
   /** Gets an expression that has this GVN. */
   Expr getAConvertedExpr() { result = this.getAnInstruction().getConvertedResultExpression() }
+
+  pragma[nomagic]
+  Function getEnclosingFunction() { result = this.getAnExpr().getEnclosingFunction() }
 }
 
 /** Gets the global value number of expression `e`. */

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll

@@ -240,10 +240,10 @@ module SemanticExprConfig {
   Expr getGuardAsExpr(Guard guard) { result = getSemanticExpr(guard) }
 
   predicate equalityGuard(Guard guard, Expr e1, Expr e2, boolean polarity) {
-    exists(IR::Instruction left, IR::Instruction right |
-      getSemanticExpr(left) = e1 and
-      getSemanticExpr(right) = e2 and
-      guard.comparesEq(left.getAUse(), right.getAUse(), 0, true, polarity)
+    exists(ValueNumber left, ValueNumber right |
+      getSemanticExpr(left.getAnInstruction()) = e1 and
+      getSemanticExpr(right.getAnInstruction()) = e2 and
+      guard.comparesEq(left, right, 0, true, polarity)
     )
   }
 

cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll

@@ -134,21 +134,23 @@ private module SizeBarrier {
      * Holds if `small <= large + k` holds if `g` evaluates to `testIsTrue`.
      */
     additional predicate isSink(
-      DataFlow::Node small, DataFlow::Node large, IRGuardCondition g, int k, boolean testIsTrue
+      ValueNumber small, ValueNumber large, IRGuardCondition g, int k, boolean testIsTrue
     ) {
       // The sink is any "large" side of a relational comparison. i.e., the `large` expression
       // in a guard such as `small <= large + k`.
-      g.comparesLt(small.asOperand(), large.asOperand(), k + 1, true, testIsTrue)
+      g.comparesLt(small, large, k + 1, true, testIsTrue)
     }
 
-    predicate isSink(DataFlow::Node sink) { isSink(_, sink, _, _, _) }
+    predicate isSink(DataFlow::Node sink) {
+      isSink(_, valueNumberOfOperand(sink.asOperand()), _, _, _)
+    }
   }
 
   module SizeBarrierFlow = DataFlow::Global<SizeBarrierConfig>;
 
-  private int getASizeAddend(DataFlow::Node node) {
+  private int getASizeAddend(ValueNumber node) {
     exists(DataFlow::Node source |
-      SizeBarrierFlow::flow(source, node) and
+      SizeBarrierFlow::flow(source, DataFlow::operandNode(node.getAUse())) and
       hasSize(_, source, result)
     )
   }
@@ -157,10 +159,10 @@ private module SizeBarrier {
    * Holds if `small <= large + k` holds if `g` evaluates to `edge`.
    */
   private predicate operandGuardChecks(
-    IRGuardCondition g, Operand small, DataFlow::Node large, int k, boolean edge
+    IRGuardCondition g, ValueNumber small, ValueNumber large, int k, boolean edge
   ) {
-    SizeBarrierFlow::flowTo(large) and
-    SizeBarrierConfig::isSink(DataFlow::operandNode(small), large, g, k, edge)
+    SizeBarrierFlow::flowTo(DataFlow::operandNode(large.getAUse())) and
+    SizeBarrierConfig::isSink(small, large, g, k, edge)
   }
 
   /**
@@ -170,15 +172,15 @@ private module SizeBarrier {
    */
   Instruction getABarrierInstruction0(int delta, int k) {
     exists(
-      IRGuardCondition g, ValueNumber value, Operand small, boolean edge, DataFlow::Node large
+      IRGuardCondition g, ValueNumber value, ValueNumber small, boolean edge, ValueNumber large
     |
       // We know:
       // 1. result <= value + delta (by `bounded`)
       // 2. value <= large + k (by `operandGuardChecks`).
       // So:
       // result <= value + delta (by 1.)
       //        <= large + k + delta (by 2.)
-      small = value.getAUse() and
+      small = value and
       operandGuardChecks(pragma[only_bind_into](g), pragma[only_bind_into](small), large,
         pragma[only_bind_into](k), pragma[only_bind_into](edge)) and
       bounded(result, value.getAnInstruction(), delta) and

cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/InvalidPointerToDereference.qll

@@ -101,13 +101,15 @@ private module InvalidPointerToDerefBarrier {
     predicate isSource(DataFlow::Node source) { isSource(source, _) }
 
     additional predicate isSink(
-      DataFlow::Node small, DataFlow::Node large, IRGuardCondition g, int k, boolean testIsTrue
+      ValueNumber small, ValueNumber large, IRGuardCondition g, int k, boolean testIsTrue
     ) {
       // The sink is any "large" side of a relational comparison.
-      g.comparesLt(small.asOperand(), large.asOperand(), k, true, testIsTrue)
+      g.comparesLt(small, large, k, true, testIsTrue)
     }
 
-    predicate isSink(DataFlow::Node sink) { isSink(_, sink, _, _, _) }
+    predicate isSink(DataFlow::Node sink) {
+      isSink(_, valueNumberOfOperand(sink.asOperand()), _, _, _)
+    }
 
     int fieldFlowBranchLimit() { result = invalidPointerToDereferenceFieldFlowBranchLimit() }
   }
@@ -123,11 +125,11 @@ private module InvalidPointerToDerefBarrier {
   private predicate operandGuardChecks(
     PointerArithmeticInstruction pai, IRGuardCondition g, Operand small, int k, boolean edge
   ) {
-    exists(DataFlow::Node source, DataFlow::Node nSmall, DataFlow::Node nLarge |
-      nSmall.asOperand() = small and
+    exists(DataFlow::Node source, ValueNumber nSmall, DataFlow::Node nLarge |
+      nSmall.getAUse() = small and
       BarrierConfig::isSource(source, pai) and
       BarrierFlow::flow(source, nLarge) and
-      BarrierConfig::isSink(nSmall, nLarge, g, k, edge)
+      BarrierConfig::isSink(nSmall, valueNumberOfOperand(nLarge.asOperand()), g, k, edge)
     )
   }
 

cpp/ql/lib/semmle/code/cpp/security/Overflow.qll

@@ -8,14 +8,15 @@ import semmle.code.cpp.controlflow.Dominance
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
 import semmle.code.cpp.controlflow.Guards
+private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 
 /**
  * Holds if the value of `use` is guarded using `abs`.
  */
 predicate guardedAbs(Operation e, Expr use) {
   exists(FunctionCall fc | fc.getTarget().getName() = ["abs", "labs", "llabs", "imaxabs"] |
     fc.getArgument(0).getAChild*() = use and
-    exists(GuardCondition c | c.ensuresLt(fc, _, _, e.getBasicBlock(), true))
+    exists(GuardCondition c | c.ensuresLt(globalValueNumber(fc), _, _, e.getBasicBlock(), true))
   )
 }
 
@@ -25,7 +26,7 @@ predicate guardedAbs(Operation e, Expr use) {
  */
 pragma[nomagic]
 predicate guardedLesser(Operation e, Expr use) {
-  exists(GuardCondition c | c.ensuresLt(use, _, _, e.getBasicBlock(), true))
+  exists(GuardCondition c | c.ensuresLt(globalValueNumber(use), _, _, e.getBasicBlock(), true))
   or
   guardedAbs(e, use)
 }
@@ -36,7 +37,7 @@ predicate guardedLesser(Operation e, Expr use) {
  */
 pragma[nomagic]
 predicate guardedGreater(Operation e, Expr use) {
-  exists(GuardCondition c | c.ensuresLt(use, _, _, e.getBasicBlock(), false))
+  exists(GuardCondition c | c.ensuresLt(globalValueNumber(use), _, _, e.getBasicBlock(), false))
   or
   guardedAbs(e, use)
 }

cpp/ql/src/Critical/MissingCheckScanf.ql

@@ -157,14 +157,10 @@ predicate hasNonGuardedAccess(
   |
     not exists(GuardCondition guard |
       // call == k and k >= minGuard so call >= minGuard
-      guard
-          .ensuresEq(globalValueNumber(call).getAnExpr(), any(int k | minGuard <= k),
-            e.getBasicBlock(), true)
+      guard.ensuresEq(globalValueNumber(call), any(int k | minGuard <= k), e.getBasicBlock(), true)
       or
       // call >= k and k >= minGuard so call >= minGuard
-      guard
-          .ensuresLt(globalValueNumber(call).getAnExpr(), any(int k | minGuard <= k),
-            e.getBasicBlock(), false)
+      guard.ensuresLt(globalValueNumber(call), any(int k | minGuard <= k), e.getBasicBlock(), false)
     )
   )
 }