C++: Account for chi nodes in back-edge detection

Author: jbj

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -280,9 +280,18 @@ cached private module Cached {
 
   cached Instruction getInstructionBackEdgeSuccessor(Instruction instruction, EdgeKind kind) {
     exists(OldInstruction oldInstruction |
-      oldInstruction = getOldInstruction(instruction) and
-      result = getNewInstruction(oldInstruction.getBackEdgeSuccessor(kind)) and
-      not Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
+      not Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind) and
+      // There is only one case for the translation into `result` because the
+      // SSA construction never inserts extra instructions _before_ an existing
+      // instruction.
+      getOldInstruction(result) = oldInstruction.getBackEdgeSuccessor(kind) and
+      // There are two cases for the translation into `instruction` because the
+      // SSA construction might have inserted a chi node _after_
+      // `oldInstruction`, in which case the back edge should come out of the
+      // chi node instead.
+      if hasChiNode(_, oldInstruction)
+      then instruction = getChiInstruction(oldInstruction)
+      else instruction = getNewInstruction(oldInstruction)
     )
   }
 

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

@@ -280,9 +280,18 @@ cached private module Cached {
 
   cached Instruction getInstructionBackEdgeSuccessor(Instruction instruction, EdgeKind kind) {
     exists(OldInstruction oldInstruction |
-      oldInstruction = getOldInstruction(instruction) and
-      result = getNewInstruction(oldInstruction.getBackEdgeSuccessor(kind)) and
-      not Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
+      not Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind) and
+      // There is only one case for the translation into `result` because the
+      // SSA construction never inserts extra instructions _before_ an existing
+      // instruction.
+      getOldInstruction(result) = oldInstruction.getBackEdgeSuccessor(kind) and
+      // There are two cases for the translation into `instruction` because the
+      // SSA construction might have inserted a chi node _after_
+      // `oldInstruction`, in which case the back edge should come out of the
+      // chi node instead.
+      if hasChiNode(_, oldInstruction)
+      then instruction = getChiInstruction(oldInstruction)
+      else instruction = getNewInstruction(oldInstruction)
     )
   }
 

cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected

@@ -4687,5 +4687,5 @@ ir.cpp:
 # 1059|     r3_8(int)        = Constant[0]        : 
 # 1059|     r3_9(bool)       = CompareGT          : r3_4, r3_8
 # 1059|     v3_10(void)      = ConditionalBranch  : r3_9
-#-----|   False (back edge) -> Block 2
-#-----|   True (back edge) -> Block 1
+#-----|   False -> Block 2
+#-----|   True -> Block 1

cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected

@@ -8,6 +8,4 @@ operandAcrossFunctions
 instructionWithoutUniqueBlock
 containsLoopOfForwardEdges
 lostReachability
-| ir.cpp:1060:12:1060:12 | Constant: (char)... |
-| ir.cpp:1061:1:1061:1 | NoOp: return ... |
 backEdgeCountMismatch