C++: One Unreached per function

dave-bartolomeo · dave-bartolomeo · commit fda8605aaeee · 2018-12-17T11:03:15.000-08:00
diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll
@@ -25,11 +25,7 @@ cached private module Cached {
       not oldInstruction instanceof OldIR::PhiInstruction and
       hasChiNode(_, oldInstruction)
     } or
-    UnreachedTag(OldInstruction oldInstruction, EdgeKind kind) {
-      // We need an `Unreached` instruction for the destination of each infeasible edge whose
-      // predecessor is reachable.
-      Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
-    }
+    UnreachedTag()
 
   cached class InstructionTagType extends TInstructionTag {
     cached final string toString() {
@@ -129,11 +125,12 @@ cached private module Cached {
       resultType = vvar.getType() and
       isGLValue = false
     ) or
-    exists(OldInstruction oldInstruction, EdgeKind kind |
-      oldInstruction.getFunction() = func and
-      tag = UnreachedTag(oldInstruction, kind) and
+    exists(OldInstruction oldInstruction |
+      func = oldInstruction.getFunction() and
+      Reachability::isInfeasibleInstructionSuccessor(oldInstruction, _) and
+      tag = UnreachedTag() and
       opcode instanceof Opcode::Unreached and
-      ast = oldInstruction.getSuccessor(kind).getAST() and
+      ast = func and
       resultType instanceof VoidType and
       isGLValue = false
     )
@@ -265,10 +262,12 @@ cached private module Cached {
       exists(OldInstruction oldInstruction |
         oldInstruction = getOldInstruction(instruction) and
         (
-          result.getTag() = UnreachedTag(oldInstruction, kind) or
-          (
-            result = getNewInstruction(oldInstruction.getSuccessor(kind)) and
-            not exists(UnreachedTag(oldInstruction, kind))
+          if Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind) then (
+            result.getTag() = UnreachedTag() and
+            result.getFunction() = instruction.getFunction()
+          )
+          else (
+            result = getNewInstruction(oldInstruction.getSuccessor(kind))
           )
         )
       ) or
@@ -521,9 +520,9 @@ cached private module CachedForDebugging {
       instr.getTag() = PhiTag(vvar, phiBlock) and
       result = "Phi Block(" + phiBlock.getUniqueId() + "): " + vvar.getUniqueId() 
     ) or
-    exists(OldInstruction oldInstr, EdgeKind kind |
-      instr.getTag() = UnreachedTag(oldInstr, kind) and
-      result = "Unreached(" + oldInstr.getUniqueId() + ":" + kind.toString() + ")"
+    (
+      instr.getTag() = UnreachedTag() and
+      result = "Unreached"
     )
   }
 
diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll
@@ -25,11 +25,7 @@ cached private module Cached {
       not oldInstruction instanceof OldIR::PhiInstruction and
       hasChiNode(_, oldInstruction)
     } or
-    UnreachedTag(OldInstruction oldInstruction, EdgeKind kind) {
-      // We need an `Unreached` instruction for the destination of each infeasible edge whose
-      // predecessor is reachable.
-      Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind)
-    }
+    UnreachedTag()
 
   cached class InstructionTagType extends TInstructionTag {
     cached final string toString() {
@@ -129,11 +125,12 @@ cached private module Cached {
       resultType = vvar.getType() and
       isGLValue = false
     ) or
-    exists(OldInstruction oldInstruction, EdgeKind kind |
-      oldInstruction.getFunction() = func and
-      tag = UnreachedTag(oldInstruction, kind) and
+    exists(OldInstruction oldInstruction |
+      func = oldInstruction.getFunction() and
+      Reachability::isInfeasibleInstructionSuccessor(oldInstruction, _) and
+      tag = UnreachedTag() and
       opcode instanceof Opcode::Unreached and
-      ast = oldInstruction.getSuccessor(kind).getAST() and
+      ast = func and
       resultType instanceof VoidType and
       isGLValue = false
     )
@@ -265,10 +262,12 @@ cached private module Cached {
       exists(OldInstruction oldInstruction |
         oldInstruction = getOldInstruction(instruction) and
         (
-          result.getTag() = UnreachedTag(oldInstruction, kind) or
-          (
-            result = getNewInstruction(oldInstruction.getSuccessor(kind)) and
-            not exists(UnreachedTag(oldInstruction, kind))
+          if Reachability::isInfeasibleInstructionSuccessor(oldInstruction, kind) then (
+            result.getTag() = UnreachedTag() and
+            result.getFunction() = instruction.getFunction()
+          )
+          else (
+            result = getNewInstruction(oldInstruction.getSuccessor(kind))
           )
         )
       ) or
@@ -521,9 +520,9 @@ cached private module CachedForDebugging {
       instr.getTag() = PhiTag(vvar, phiBlock) and
       result = "Phi Block(" + phiBlock.getUniqueId() + "): " + vvar.getUniqueId() 
     ) or
-    exists(OldInstruction oldInstr, EdgeKind kind |
-      instr.getTag() = UnreachedTag(oldInstr, kind) and
-      result = "Unreached(" + oldInstr.getUniqueId() + ":" + kind.toString() + ")"
+    (
+      instr.getTag() = UnreachedTag() and
+      result = "Unreached"
     )
   }
 
diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected
@@ -1214,8 +1214,8 @@ ir.cpp:
 #  281|     v2_0(void) = NoOp : 
 #-----|   Goto -> Block 1
 
-#  283|   Block 3
-#  283|     v3_0(void) = Unreached : 
+#  278|   Block 3
+#  278|     v3_0(void) = Unreached : 
 
 #  285| For_Update() -> void
 #  285|   Block 0
@@ -1260,8 +1260,8 @@ ir.cpp:
 #  294|     v2_0(void) = NoOp : 
 #-----|   Goto -> Block 1
 
-#  296|   Block 3
-#  296|     v3_0(void) = Unreached : 
+#  292|   Block 3
+#  292|     v3_0(void) = Unreached : 
 
 #  298| For_InitUpdate() -> void
 #  298|   Block 0
@@ -1465,7 +1465,7 @@ ir.cpp:
 #  334|     r1_2(int)        = Constant[10]       : 
 #  334|     r1_3(bool)       = CompareLT          : r1_1, r1_2
 #  334|     v1_4(void)       = ConditionalBranch  : r1_3
-#-----|   False -> Block 5
+#-----|   False -> Block 4
 #-----|   True -> Block 2
 
 #  335|   Block 2
@@ -1481,11 +1481,8 @@ ir.cpp:
 #  334|     v3_0(void) = NoOp : 
 #-----|   Goto -> Block 1
 
-#  336|   Block 4
-#  336|     v4_0(void) = Unreached : 
-
-#  339|   Block 5
-#  339|     v5_0(void) = Unreached : 
+#  333|   Block 4
+#  333|     v4_0(void) = Unreached : 
 
 #  341| Dereference(int *) -> int
 #  341|   Block 0
@@ -3225,8 +3222,8 @@ ir.cpp:
 #  724|     v11_1(void) = ReturnVoid : 
 #-----|   Goto -> Block 1
 
-#  731|   Block 12
-#  731|     v12_0(void) = Unreached : 
+#  724|   Block 12
+#  724|     v12_0(void) = Unreached : 
 
 #  745| Base::Base(const Base &) -> void
 #  745|   Block 0
@@ -4101,8 +4098,8 @@ ir.cpp:
 #  904|     v1_9(void)       = UnmodeledUse                 : mu*
 #  904|     v1_10(void)      = ExitFunction                 : 
 
-#  906|   Block 2
-#  906|     v2_0(void) = Unreached : 
+#  904|   Block 2
+#  904|     v2_0(void) = Unreached : 
 
 #  940| OperatorNew() -> void
 #  940|   Block 0
@@ -4607,7 +4604,7 @@ ir.cpp:
 # 1040|     r4_3(int)        = Load               : r4_2, m0_10
 # 1040|     r4_4(bool)       = CompareLT          : r4_1, r4_3
 # 1040|     v4_5(void)       = ConditionalBranch  : r4_4
-#-----|   False -> Block 7
+#-----|   False -> Block 6
 #-----|   True -> Block 5
 
 # 1041|   Block 5
@@ -4616,11 +4613,8 @@ ir.cpp:
 # 1041|     m5_2(int)        = Store                    : r5_0, r5_1
 #-----|   Goto -> Block 1
 
-# 1033|   Block 6
-# 1033|     v6_0(void) = Unreached : 
-
-# 1044|   Block 7
-# 1044|     v7_0(void) = Unreached : 
+# 1028|   Block 6
+# 1028|     v6_0(void) = Unreached : 
 
 # 1049| DoWhileFalse() -> int
 # 1049|   Block 0
@@ -4650,5 +4644,5 @@ ir.cpp:
 # 1049|     v1_6(void)       = UnmodeledUse             : mu*
 # 1049|     v1_7(void)       = ExitFunction             : 
 
-# 1052|   Block 2
-# 1052|     v2_0(void) = Unreached : 
+# 1049|   Block 2
+# 1049|     v2_0(void) = Unreached : 
diff --git a/cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected b/cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected
@@ -44,7 +44,7 @@
 | IR: For_Break | 6 |
 | IR: For_Condition | 4 |
 | IR: For_ConditionUpdate | 4 |
-| IR: For_Continue_NoUpdate | 6 |
+| IR: For_Continue_NoUpdate | 5 |
 | IR: For_Continue_Update | 6 |
 | IR: For_Empty | 2 |
 | IR: For_Init | 2 |
@@ -94,7 +94,7 @@
 | IR: TryCatch | 13 |
 | IR: UninitializedVariables | 1 |
 | IR: UnionInit | 1 |
-| IR: UnreachableIf | 8 |
+| IR: UnreachableIf | 7 |
 | IR: UnreachableViaGoto | 1 |
 | IR: VarArgUsage | 1 |
 | IR: VarArgs | 1 |
diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected
@@ -3991,8 +3991,8 @@ ir.cpp:
 #  904|     v1_9(void)       = UnmodeledUse                 : mu*
 #  904|     v1_10(void)      = ExitFunction                 : 
 
-#  906|   Block 2
-#  906|     v2_0(void) = Unreached : 
+#  904|   Block 2
+#  904|     v2_0(void) = Unreached : 
 
 #  940| OperatorNew() -> void
 #  940|   Block 0
@@ -4515,5 +4515,5 @@ ir.cpp:
 # 1049|     v1_6(void)       = UnmodeledUse             : mu*
 # 1049|     v1_7(void)       = ExitFunction             : 
 
-# 1052|   Block 2
-# 1052|     v2_0(void) = Unreached : 
+# 1049|   Block 2
+# 1049|     v2_0(void) = Unreached : 
