diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll index d7a728f36e1..b29b1aa6382 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll +++ b/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. */ diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll index d7a728f36e1..b29b1aa6382 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll +++ b/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. */ diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll index d7a728f36e1..b29b1aa6382 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll +++ b/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. */