C++: Fix up uses of GuardCondition.

Author: MathiasVP

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)
     )
   )
 }

cpp/ql/src/Critical/OverflowDestination.ql

@@ -17,6 +17,7 @@ import cpp
 import semmle.code.cpp.ir.dataflow.TaintTracking
 import semmle.code.cpp.controlflow.IRGuards
 import semmle.code.cpp.security.FlowSources
+import semmle.code.cpp.ir.ValueNumbering
 import OverflowDestination::PathGraph
 
 /**
@@ -61,7 +62,9 @@ predicate hasUpperBoundsCheck(Variable var) {
   )
 }
 
-predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
+predicate nodeIsBarrierEqualityCandidate(
+  DataFlow::Node node, ValueNumber access, Variable checkedVar
+) {
   readsVariable(node.asInstruction(), checkedVar) and
   any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
 }
@@ -77,8 +80,8 @@ module OverflowDestinationConfig implements DataFlow::ConfigSig {
       hasUpperBoundsCheck(checkedVar)
     )
     or
-    exists(Variable checkedVar, Operand access |
-      readsVariable(access.getDef(), checkedVar) and
+    exists(Variable checkedVar, ValueNumber access |
+      readsVariable(access.getAnInstruction(), checkedVar) and
       nodeIsBarrierEqualityCandidate(node, access, checkedVar)
     )
   }

cpp/ql/src/Critical/ScanfChecks.qll

@@ -7,7 +7,7 @@ private predicate exprInBooleanContext(Expr e) {
   exists(IRGuardCondition gc |
     exists(Instruction i |
       i.getUnconvertedResultExpression() = e and
-      gc.comparesEq(valueNumber(i).getAUse(), 0, _, _)
+      gc.comparesEq(valueNumber(i), 0, _, _)
     )
     or
     gc.getUnconvertedResultExpression() = e
@@ -38,15 +38,15 @@ private string getEofValue() {
 private predicate checkedForEof(ScanfFunctionCall call) {
   exists(IRGuardCondition gc |
     exists(Instruction i | i.getUnconvertedResultExpression() = call |
-      exists(int val | gc.comparesEq(valueNumber(i).getAUse(), val, _, _) |
+      exists(int val | gc.comparesEq(valueNumber(i), val, _, _) |
         // call == EOF
         val = getEofValue().toInt()
         or
         // call == [any positive number]
         val > 0
       )
       or
-      exists(int val | gc.comparesLt(valueNumber(i).getAUse(), val, true, _) |
+      exists(int val | gc.comparesLt(valueNumber(i), val, true, _) |
         // call < [any non-negative number] (EOF is guaranteed to be negative)
         val >= 0
       )