Merge pull request #879 from rdmarsh2/rdmarsh/cpp/ir-guards-edges

C++: Add edge-based predicates to IRGuards

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

@@ -239,9 +239,10 @@ private class GuardConditionFromIR extends GuardCondition {
  * IRGuardConditions.
  */
 class IRGuardCondition extends Instruction {
-
+    ConditionalBranchInstruction branch;
+    
     IRGuardCondition() {
-        is_condition(this)
+        branch = get_branch_for_condition(this)
     }
 
     /**
@@ -280,6 +281,16 @@ class IRGuardCondition extends Instruction {
           ne.controls(controlled, testIsTrue.booleanNot())) 
     }
 
+    predicate controlsEdge(IRBlock pred, IRBlock succ, boolean testIsTrue) {
+      pred.getASuccessor() = succ and
+      controls(pred, testIsTrue)
+      or
+      hasBranchEdge(succ, testIsTrue) and
+      branch.getCondition() = this and
+      branch.getBlock() = pred
+    }
+
+
     /**
      * Holds if `branch` jumps directly to `succ` when this condition is `testIsTrue`.
      * 
@@ -295,7 +306,7 @@ class IRGuardCondition extends Instruction {
      * return x;
      * ```
      */
-    predicate hasBranchEdge(ConditionalBranchInstruction branch, IRBlock succ, boolean testIsTrue) {
+    private predicate hasBranchEdge(IRBlock succ, boolean testIsTrue) {
       branch.getCondition() = this and
       (
         testIsTrue = true and
@@ -319,6 +330,14 @@ class IRGuardCondition extends Instruction {
         )
     }
 
+    /** Holds if (determined by this guard) `left < right + k` must be `isLessThan` on the edge from
+     * `pred` to `succ`. If `isLessThan = false` then this implies `left >= right + k`.  */
+    cached predicate ensuresLtEdge(Operand left, Operand right, int k, IRBlock pred, IRBlock succ, boolean isLessThan) {
+        exists(boolean testIsTrue |
+            compares_lt(this, left, right, k, isLessThan, testIsTrue) and this.controlsEdge(pred, succ, testIsTrue)
+        )
+    }
+
     /** Holds if (determined by this guard) `left == right + k` evaluates to `areEqual` if this expression evaluates to `testIsTrue`. */
     cached predicate comparesEq(Operand left, Operand right, int k, boolean areEqual, boolean testIsTrue) {
         compares_eq(this, left, right, k, areEqual, testIsTrue)
@@ -331,6 +350,13 @@ class IRGuardCondition extends Instruction {
             compares_eq(this, left, right, k, areEqual, testIsTrue) and this.controls(block, testIsTrue)
         )
     }
+    /** Holds if (determined by this guard) `left == right + k` must be `areEqual` on the edge from
+     * `pred` to `succ`. If `areEqual = false` then this implies `left != right + k`.  */
+    cached predicate ensuresEqEdge(Operand left, Operand right, int k, IRBlock pred, IRBlock succ, boolean areEqual) {
+      exists(boolean testIsTrue |
+        compares_eq(this, left, right, k, areEqual, testIsTrue) and this.controlsEdge(pred, succ, testIsTrue)
+      )
+    }
 
     /**
      * Holds if this condition controls `block`, meaning that `block` is only
@@ -340,9 +366,9 @@ class IRGuardCondition extends Instruction {
      */
     private predicate controlsBlock(IRBlock controlled, boolean testIsTrue) {
         not isUnreachedBlock(controlled) and
-        exists(IRBlock thisblock
+        exists(IRBlock branchBlock
-        | thisblock.getAnInstruction() = this
+        | branchBlock.getAnInstruction() = branch
-        | exists(IRBlock succ, ConditionalBranchInstruction branch
+        | exists(IRBlock succ
           | testIsTrue = true and succ.getFirstInstruction() = branch.getTrueSuccessor()
             or
             testIsTrue = false and succ.getFirstInstruction() = branch.getFalseSuccessor()
@@ -350,16 +376,16 @@ class IRGuardCondition extends Instruction {
             succ.dominates(controlled) and
             forall(IRBlock pred
             | pred.getASuccessor() = succ
-            | pred = thisblock or succ.dominates(pred) or not pred.isReachableFromFunctionEntry())))
+            | pred = branchBlock or succ.dominates(pred) or not pred.isReachableFromFunctionEntry())))
     }
 }
 
-private predicate is_condition(Instruction guard) {
+private ConditionalBranchInstruction get_branch_for_condition(Instruction guard) {
   exists(ConditionalBranchInstruction branch|
     branch.getCondition() = guard
   )
   or
-  exists(LogicalNotInstruction cond | is_condition(cond) and cond.getUnary() = guard)
+  exists(LogicalNotInstruction cond | result = get_branch_for_condition(cond) and cond.getUnary() = guard)
 }
 
 /**

cpp/ql/src/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll

@@ -386,11 +386,7 @@ private predicate boundFlowStepPhi(
   or
   exists(IRGuardCondition guard, boolean testIsTrue |
     guard = boundFlowCond(valueNumberOfOperand(op2), op1, delta, upper, testIsTrue) and
-    (
+    guard.controlsEdge(op2.getPredecessorBlock(), op2.getUseInstruction().getBlock(), testIsTrue) and
-      guard.hasBranchEdge(op2.getPredecessorBlock().getLastInstruction(), op2.getUseInstruction().getBlock(), testIsTrue)
-      or
-      guard.controls(op2.getPredecessorBlock(), testIsTrue)
-    ) and
     reason = TCondReason(guard)
   )
 }

cpp/ql/test/library-tests/controlflow/guards-ir/test.c

@@ -151,4 +151,3 @@ void test5(int x) {
 void test6(int x, int y) {
   return x && y;
 }
-