C++: Compute isInCycle only for raw IR

On wireshark/wireshark, `isInCycle` ran into a low-memory loop on the
`aliased_ssa` stage. It shouldn't be necessary to detect cycles after
the `raw` stage, so this commit moves cycle detection into the
`Construction` modules and makes it a no-op in `SSAConstruction.qll`.

Author: jbj

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

@@ -11,42 +11,20 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::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/aliased_ssa/internal/SSAConstruction.qll

@@ -133,6 +133,16 @@ private module Cached {
     overlap instanceof MustExactlyOverlap
   }
 
+  /**
+   * 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.
+   *
+   * For performance reasons, this predicate is not implemented (never holds)
+   * for the SSA stages of the IR.
+   */
+  cached
+  predicate isInCycle(Instruction instr) { none() }
+
   cached
   Language::LanguageType getInstructionOperandType(Instruction instr, TypedOperandTag tag) {
     exists(OldInstruction oldInstruction, OldIR::TypedOperand oldOperand |

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

@@ -11,42 +11,20 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::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/internal/IRConstruction.qll

@@ -93,6 +93,29 @@ private module Cached {
     overlap instanceof MustTotallyOverlap
   }
 
+  /** Gets a non-phi instruction that defines an operand of `instr`. */
+  private Instruction getNonPhiOperandDef(Instruction instr) {
+    result = getRegisterOperandDefinition(instr, _)
+    or
+    result = 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]
+  cached
+  predicate isInCycle(Instruction instr) {
+    instr instanceof Instruction and
+    getNonPhiOperandDef+(instr) = instr
+  }
+
   cached
   CppType getInstructionOperandType(Instruction instruction, TypedOperandTag tag) {
     // For all `LoadInstruction`s, the operand type of the `LoadOperand` is the same as

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

@@ -11,42 +11,20 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::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/internal/SSAConstruction.qll

@@ -133,6 +133,16 @@ private module Cached {
     overlap instanceof MustExactlyOverlap
   }
 
+  /**
+   * 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.
+   *
+   * For performance reasons, this predicate is not implemented (never holds)
+   * for the SSA stages of the IR.
+   */
+  cached
+  predicate isInCycle(Instruction instr) { none() }
+
   cached
   Language::LanguageType getInstructionOperandType(Instruction instr, TypedOperandTag tag) {
     exists(OldInstruction oldInstruction, OldIR::TypedOperand oldOperand |

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

@@ -11,42 +11,20 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::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.
  */

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/IRConstruction.qll

@@ -95,6 +95,29 @@ private module Cached {
           .getInstructionOperand(getInstructionTag(instruction), tag)
   }
 
+  /** Gets a non-phi instruction that defines an operand of `instr`. */
+  private Instruction getNonPhiOperandDef(Instruction instr) {
+    result = getRegisterOperandDefinition(instr, _)
+    or
+    result = 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]
+  cached
+  predicate isInCycle(Instruction instr) {
+    instr instanceof Instruction and
+    getNonPhiOperandDef+(instr) = instr
+  }
+
   cached
   CSharpType getInstructionOperandType(Instruction instruction, TypedOperandTag tag) {
     // For all `LoadInstruction`s, the operand type of the `LoadOperand` is the same as

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

@@ -11,42 +11,20 @@ cached
 private newtype TOperand =
   TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
     defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
-    not isInCycle(useInstr)
+    not Construction::isInCycle(useInstr)
   } or
   TNonPhiMemoryOperand(
     Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
-    not isInCycle(useInstr)
+    not Construction::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.
  */

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

@@ -133,6 +133,16 @@ private module Cached {
     overlap instanceof MustExactlyOverlap
   }
 
+  /**
+   * 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.
+   *
+   * For performance reasons, this predicate is not implemented (never holds)
+   * for the SSA stages of the IR.
+   */
+  cached
+  predicate isInCycle(Instruction instr) { none() }
+
   cached
   Language::LanguageType getInstructionOperandType(Instruction instr, TypedOperandTag tag) {
     exists(OldInstruction oldInstruction, OldIR::TypedOperand oldOperand |