C++: IR sanity query unnecessaryPhiInstruction

Have `Instruction.getResultSize()` return zero for `void`.

Author: dave-bartolomeo

cpp/ql/src/semmle/code/cpp/ir/internal/Instruction.qll

@@ -68,17 +68,14 @@ module InstructionSanity {
   }
 
   /**
-   * Holds if `Phi` instruction `instr` has fewer than two operands.
+   * Holds if `Phi` instruction `instr` is missing an operand corresponding to
+   * the predecessor block `pred`.
    */
-  query predicate missingPhiOperands(PhiInstruction instr, int predIndex, Location predLoc) {
-    exists(IRBlock pred |
-      pred = instr.getBlock().getAPredecessor() and
-      predLoc = pred.getLocation() and
-      predIndex = pred.getDisplayIndex() and
-      not exists(PhiOperand operand |
-        exists(instr.getOperand(operand)) and
-        operand.getPredecessorBlock() = pred
-      )
+  query predicate missingPhiOperand(PhiInstruction instr, IRBlock pred) {
+    pred = instr.getBlock().getAPredecessor() and
+    not exists(PhiOperand operand |
+      exists(instr.getOperand(operand)) and
+      operand.getPredecessorBlock() = pred
     )
   }
 
@@ -92,6 +89,18 @@ module InstructionSanity {
     not instr instanceof PhiInstruction
   }
 
+  /**
+   * Holds if a `Phi` instruction is present in a block with fewer than two
+   * predecessors.
+   */
+  query predicate unnecessaryPhiInstruction(PhiInstruction instr) {
+    count(instr.getBlock().getAPredecessor()) < 2
+  }
+
+  /**
+   * Holds if instruction `op` consumes an operand `operand` that was defined in
+   * a different function.
+   */
   query predicate operandAcrossFunctions(
     Instruction op, Instruction operand, OperandTag tag
   ) {
@@ -321,8 +330,7 @@ class Instruction extends Construction::TInstruction {
 
   /**
    * Gets the size of the result produced by this instruction, in bytes. If the
-   * instruction does not produce a result, or if the result does not have a
-   * known constant size, this predicate does not hold.
+   * result does not have a known constant size, this predicate does not hold.
    *
    * If `this.isGLValue()` holds for this instruction, the value of
    * `getResultSize()` will always be the size of a pointer.
@@ -337,7 +345,6 @@ class Instruction extends Construction::TInstruction {
     else if resultType instanceof UnknownType then
       result = Construction::getInstructionResultSize(this)
     else (
-      not resultType instanceof VoidType and
       result = resultType.getSize()
     )
   }

cpp/ql/src/semmle/code/cpp/ssa/internal/aliased_ssa/Instruction.qll

@@ -68,17 +68,14 @@ module InstructionSanity {
   }
 
   /**
-   * Holds if `Phi` instruction `instr` has fewer than two operands.
+   * Holds if `Phi` instruction `instr` is missing an operand corresponding to
+   * the predecessor block `pred`.
    */
-  query predicate missingPhiOperand(PhiInstruction instr, int predIndex, Location predLoc) {
-    exists(IRBlock pred |
-      pred = instr.getBlock().getAPredecessor() and
-      predLoc = pred.getLocation() and
-      predIndex = pred.getDisplayIndex() and
-      not exists(PhiOperand operand |
-        exists(instr.getOperand(operand)) and
-        operand.getPredecessorBlock() = pred
-      )
+  query predicate missingPhiOperand(PhiInstruction instr, IRBlock pred) {
+    pred = instr.getBlock().getAPredecessor() and
+    not exists(PhiOperand operand |
+      exists(instr.getOperand(operand)) and
+      operand.getPredecessorBlock() = pred
     )
   }
 
@@ -92,6 +89,18 @@ module InstructionSanity {
     not instr instanceof PhiInstruction
   }
 
+  /**
+   * Holds if a `Phi` instruction is present in a block with fewer than two
+   * predecessors.
+   */
+  query predicate unnecessaryPhiInstruction(PhiInstruction instr) {
+    count(instr.getBlock().getAPredecessor()) < 2
+  }
+
+  /**
+   * Holds if instruction `op` consumes an operand `operand` that was defined in
+   * a different function.
+   */
   query predicate operandAcrossFunctions(
     Instruction op, Instruction operand, OperandTag tag
   ) {
@@ -321,8 +330,7 @@ class Instruction extends Construction::TInstruction {
 
   /**
    * Gets the size of the result produced by this instruction, in bytes. If the
-   * instruction does not produce a result, or if the result does not have a
-   * known constant size, this predicate does not hold.
+   * result does not have a known constant size, this predicate does not hold.
    *
    * If `this.isGLValue()` holds for this instruction, the value of
    * `getResultSize()` will always be the size of a pointer.
@@ -337,7 +345,6 @@ class Instruction extends Construction::TInstruction {
     else if resultType instanceof UnknownType then
       result = Construction::getInstructionResultSize(this)
     else (
-      not resultType instanceof VoidType and
       result = resultType.getSize()
     )
   }

cpp/ql/src/semmle/code/cpp/ssa/internal/ssa/Instruction.qll

@@ -68,17 +68,14 @@ module InstructionSanity {
   }
 
   /**
-   * Holds if `Phi` instruction `instr` has fewer than two operands.
+   * Holds if `Phi` instruction `instr` is missing an operand corresponding to
+   * the predecessor block `pred`.
    */
-  query predicate missingPhiOperands(PhiInstruction instr, int predIndex, Location predLoc) {
-    exists(IRBlock pred |
-      pred = instr.getBlock().getAPredecessor() and
-      predLoc = pred.getLocation() and
-      predIndex = pred.getDisplayIndex() and
-      not exists(PhiOperand operand |
-        exists(instr.getOperand(operand)) and
-        operand.getPredecessorBlock() = pred
-      )
+  query predicate missingPhiOperand(PhiInstruction instr, IRBlock pred) {
+    pred = instr.getBlock().getAPredecessor() and
+    not exists(PhiOperand operand |
+      exists(instr.getOperand(operand)) and
+      operand.getPredecessorBlock() = pred
     )
   }
 
@@ -92,6 +89,18 @@ module InstructionSanity {
     not instr instanceof PhiInstruction
   }
 
+  /**
+   * Holds if a `Phi` instruction is present in a block with fewer than two
+   * predecessors.
+   */
+  query predicate unnecessaryPhiInstruction(PhiInstruction instr) {
+    count(instr.getBlock().getAPredecessor()) < 2
+  }
+
+  /**
+   * Holds if instruction `op` consumes an operand `operand` that was defined in
+   * a different function.
+   */
   query predicate operandAcrossFunctions(
     Instruction op, Instruction operand, OperandTag tag
   ) {
@@ -321,8 +330,7 @@ class Instruction extends Construction::TInstruction {
 
   /**
    * Gets the size of the result produced by this instruction, in bytes. If the
-   * instruction does not produce a result, or if the result does not have a
-   * known constant size, this predicate does not hold.
+   * result does not have a known constant size, this predicate does not hold.
    *
    * If `this.isGLValue()` holds for this instruction, the value of
    * `getResultSize()` will always be the size of a pointer.
@@ -337,7 +345,6 @@ class Instruction extends Construction::TInstruction {
     else if resultType instanceof UnknownType then
       result = Construction::getInstructionResultSize(this)
     else (
-      not resultType instanceof VoidType and
       result = resultType.getSize()
     )
   }

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

@@ -3,4 +3,5 @@ unexpectedOperand
 duplicateOperand
 missingPhiOperand
 instructionWithoutSuccessor
+unnecessaryPhiInstruction
 operandAcrossFunctions

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

@@ -1,6 +1,7 @@
 missingOperand
 unexpectedOperand
 duplicateOperand
-missingPhiOperands
+missingPhiOperand
 instructionWithoutSuccessor
+unnecessaryPhiInstruction
 operandAcrossFunctions

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

@@ -1,6 +1,7 @@
 missingOperand
 unexpectedOperand
 duplicateOperand
-missingPhiOperands
+missingPhiOperand
 instructionWithoutSuccessor
+unnecessaryPhiInstruction
 operandAcrossFunctions