Merge pull request #1567 from jbj/ir-operand-cycles

C++ IR: guard against cycles in operand graph

Author: dave-bartolomeo

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll

@@ -9,15 +9,39 @@ private import semmle.code.cpp.ir.internal.OperandTag
 cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
-    defInstr = Construction::getRegisterOperandDefinition(useInstr, tag)
+    defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
+    not isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap) {
-    defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap)
+    defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
+    not isInCycle(useInstr)
   } or
   TPhiOperand(PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap) {
     defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
   }
 
+/** Gets a non-phi instruction that defines an operand of `instr`. */
+private Instruction getNonPhiOperandDef(Instruction instr) {
+  result = Construction::getRegisterOperandDefinition(instr, _)
+  or
+  result = Construction::getMemoryOperandDefinition(instr, _, _)
+}
+
+/**
+ * Holds if `instr` is part of a cycle in the operand graph that doesn't go
+ * through a phi instruction and therefore should be impossible.
+ *
+ * If such cycles are present, either due to a programming error in the IR
+ * generation or due to a malformed database, it can cause infinite loops in
+ * analyses that assume a cycle-free graph of non-phi operands. Therefore it's
+ * better to remove these operands than to leave cycles in the operand graph.
+ */
+pragma[noopt]
+private predicate isInCycle(Instruction instr) {
+  instr instanceof Instruction and
+  getNonPhiOperandDef+(instr) = instr
+}
+
 /**
  * A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
  */

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll

@@ -9,15 +9,39 @@ private import semmle.code.cpp.ir.internal.OperandTag
 cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
-    defInstr = Construction::getRegisterOperandDefinition(useInstr, tag)
+    defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
+    not isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap) {
-    defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap)
+    defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
+    not isInCycle(useInstr)
   } or
   TPhiOperand(PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap) {
     defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
   }
 
+/** Gets a non-phi instruction that defines an operand of `instr`. */
+private Instruction getNonPhiOperandDef(Instruction instr) {
+  result = Construction::getRegisterOperandDefinition(instr, _)
+  or
+  result = Construction::getMemoryOperandDefinition(instr, _, _)
+}
+
+/**
+ * Holds if `instr` is part of a cycle in the operand graph that doesn't go
+ * through a phi instruction and therefore should be impossible.
+ *
+ * If such cycles are present, either due to a programming error in the IR
+ * generation or due to a malformed database, it can cause infinite loops in
+ * analyses that assume a cycle-free graph of non-phi operands. Therefore it's
+ * better to remove these operands than to leave cycles in the operand graph.
+ */
+pragma[noopt]
+private predicate isInCycle(Instruction instr) {
+  instr instanceof Instruction and
+  getNonPhiOperandDef+(instr) = instr
+}
+
 /**
  * A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
  */

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll

@@ -9,15 +9,39 @@ private import semmle.code.cpp.ir.internal.OperandTag
 cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
-    defInstr = Construction::getRegisterOperandDefinition(useInstr, tag)
+    defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
+    not isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap) {
-    defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap)
+    defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
+    not isInCycle(useInstr)
   } or
   TPhiOperand(PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap) {
     defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
   }
 
+/** Gets a non-phi instruction that defines an operand of `instr`. */
+private Instruction getNonPhiOperandDef(Instruction instr) {
+  result = Construction::getRegisterOperandDefinition(instr, _)
+  or
+  result = Construction::getMemoryOperandDefinition(instr, _, _)
+}
+
+/**
+ * Holds if `instr` is part of a cycle in the operand graph that doesn't go
+ * through a phi instruction and therefore should be impossible.
+ *
+ * If such cycles are present, either due to a programming error in the IR
+ * generation or due to a malformed database, it can cause infinite loops in
+ * analyses that assume a cycle-free graph of non-phi operands. Therefore it's
+ * better to remove these operands than to leave cycles in the operand graph.
+ */
+pragma[noopt]
+private predicate isInCycle(Instruction instr) {
+  instr instanceof Instruction and
+  getNonPhiOperandDef+(instr) = instr
+}
+
 /**
  * A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
  */