C++: handle phi operands from unreachable blocks

Author: Robert Marsh

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

@@ -68,8 +68,22 @@ private module Cached {
     instr instanceof TUnreachedInstruction
   }
 
-  private IRBlock getNewBlock(OldBlock oldBlock) {
-    result.getFirstInstruction() = getNewInstruction(oldBlock.getFirstInstruction())
+  cached IRBlock getNewBlock(OldBlock oldBlock) {
+    exists(Instruction newEnd, OldIR::Instruction oldEnd |
+      (
+      result.getLastInstruction() = newEnd and
+      not newEnd instanceof ChiInstruction
+      or
+      newEnd = result.getLastInstruction().(ChiInstruction).getAPredecessor() // does this work?
+      ) and
+      (
+        oldBlock.getLastInstruction() = oldEnd and
+        not oldEnd instanceof OldIR::ChiInstruction
+        or
+        oldEnd = oldBlock.getLastInstruction().(OldIR::ChiInstruction).getAPredecessor() // does this work?
+      ) and
+      oldEnd = getNewInstruction(newEnd)
+    )
   }
 
   /**
@@ -164,7 +178,7 @@ private module Cached {
       (
         result = getNewInstruction(oldOperand.getAnyDef()) and
         overlap = originalOverlap
-         or
+        or
         exists(OldIR::PhiInputOperand phiOperand, Overlap phiOperandOverlap |
           phiOperand = getDegeneratePhiOperand(oldOperand.getAnyDef()) and
           result = getNewDefinitionFromOldSSA(phiOperand, phiOperandOverlap) and

cpp/ql/src/semmle/code/cpp/ir/implementation/internal/TOperand.qll

@@ -36,10 +36,9 @@ private module Internal {
       useInstr.getOpcode().hasOperand(tag)
     } or
     TUnaliasedPhiOperand(
-      Unaliased::PhiInstruction useInstr, Unaliased::Instruction defInstr,
-      Unaliased::IRBlock predecessorBlock, Overlap overlap
+      Unaliased::PhiInstruction useInstr, Unaliased::IRBlock predecessorBlock, Overlap overlap
     ) {
-      defInstr = UnaliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
+      exists(UnaliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap))
     } or
     //// ALIASED
     ////
@@ -50,10 +49,9 @@ private module Internal {
     // important that we use the same definition of "is variable aliased" across
     // the phases.
     TAliasedPhiOperand(
-      TAliasedSSAPhiInstruction useInstr, Aliased::Instruction defInstr,
-      Aliased::IRBlock predecessorBlock, Overlap overlap
+      TAliasedSSAPhiInstruction useInstr, Aliased::IRBlock predecessorBlock, Overlap overlap
     ) {
-      defInstr = AliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
+      exists(AliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap))
     } or
     TAliasedChiOperand(TAliasedSSAChiInstruction useInstr, ChiOperandTag tag) { any() }
 }
@@ -144,7 +142,8 @@ module UnaliasedSSAOperands {
     Unaliased::PhiInstruction useInstr, Unaliased::Instruction defInstr,
     Unaliased::IRBlock predecessorBlock, Overlap overlap
   ) {
-    result = Internal::TUnaliasedPhiOperand(useInstr, defInstr, predecessorBlock, overlap)
+    defInstr = UnaliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap) and
+    result = Internal::TUnaliasedPhiOperand(useInstr, predecessorBlock, overlap)
   }
 
   TPhiOperand reusedPhiOperand(
@@ -182,7 +181,8 @@ module AliasedSSAOperands {
     Aliased::PhiInstruction useInstr, Aliased::Instruction defInstr,
     Aliased::IRBlock predecessorBlock, Overlap overlap
   ) {
-    result = Internal::TAliasedPhiOperand(useInstr, defInstr, predecessorBlock, overlap)
+    defInstr = AliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap) and
+    result = Internal::TAliasedPhiOperand(useInstr, predecessorBlock, overlap)
   }
 
   /**
@@ -193,8 +193,9 @@ module AliasedSSAOperands {
     Aliased::IRBlock predecessorBlock, Overlap overlap
   ) {
     exists(Unaliased::IRBlock oldBlock |
-      predecessorBlock.getFirstInstruction() = oldBlock.getFirstInstruction() and
-      result = Internal::TUnaliasedPhiOperand(useInstr, defInstr, oldBlock, overlap)
+      predecessorBlock = AliasedConstruction::getNewBlock(oldBlock) and
+      result = Internal::TUnaliasedPhiOperand(useInstr, oldBlock, _) and
+      defInstr = AliasedConstruction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
     )
   }
 

cpp/ql/test/library-tests/ir/ssa/aliased_ssa_consistency.expected

@@ -2,7 +2,6 @@ missingOperand
 unexpectedOperand
 duplicateOperand
 missingPhiOperand
-| ssa.cpp:398:3:398:13 | Phi: return ... | Instruction 'Phi: return ...' is missing an operand for predecessor block 'VariableAddress: y' in function '$@'. | ssa.cpp:383:5:383:24 | int FusedBlockPhiOperand(int, int, int, bool) | int FusedBlockPhiOperand(int, int, int, bool) |
 missingOperandType
 duplicateChiOperand
 sideEffectWithoutPrimary

cpp/ql/test/library-tests/ir/ssa/aliased_ssa_consistency_unsound.expected

@@ -2,7 +2,6 @@ missingOperand
 unexpectedOperand
 duplicateOperand
 missingPhiOperand
-| ssa.cpp:398:3:398:13 | Phi: return ... | Instruction 'Phi: return ...' is missing an operand for predecessor block 'VariableAddress: y' in function '$@'. | ssa.cpp:383:5:383:24 | int FusedBlockPhiOperand(int, int, int, bool) | int FusedBlockPhiOperand(int, int, int, bool) |
 missingOperandType
 duplicateChiOperand
 sideEffectWithoutPrimary

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

@@ -1802,7 +1802,7 @@ ssa.cpp:
 #-----|   Goto -> Block 4
 
 #  398|   Block 4
-#  398|     m398_1(int)         = Phi                      : from 1:m388_4
+#  398|     m398_1(int)         = Phi                      : from 1:m388_4, from 3:m391_4
 #  398|     r398_2(glval<int>)  = VariableAddress[#return] : 
 #  398|     r398_3(glval<int>)  = VariableAddress[ret]     : 
 #  398|     r398_4(int)         = Load[ret]                : &:r398_3, m398_1

cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected

@@ -1792,7 +1792,7 @@ ssa.cpp:
 #-----|   Goto -> Block 4
 
 #  398|   Block 4
-#  398|     m398_1(int)         = Phi                      : from 1:m388_4
+#  398|     m398_1(int)         = Phi                      : from 1:m388_4, from 3:m391_4
 #  398|     r398_2(glval<int>)  = VariableAddress[#return] : 
 #  398|     r398_3(glval<int>)  = VariableAddress[ret]     : 
 #  398|     r398_4(int)         = Load[ret]                : &:r398_3, m398_1