Skip to content

Commit 39854a3

Browse files
committed
C++ IR: guard against cycles in operand graph
This doesn't fix the underlying problem that for some reason there are cycles in the operand graph on our snapshots of the Linux kernel, but it ensures that the cycles don't lead to non-termination of `ConstantAnalysis` and `ValueNumbering`.
1 parent 46d7792 commit 39854a3

File tree

3 files changed

+72
-6
lines changed

3 files changed

+72
-6
lines changed

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,37 @@ private import semmle.code.cpp.ir.internal.OperandTag
88

99
private newtype TOperand =
1010
TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
11-
defInstr = Construction::getRegisterOperandDefinition(useInstr, tag)
11+
defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
12+
not isInCycle(useInstr)
1213
} or
1314
TNonPhiMemoryOperand(Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap) {
14-
defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap)
15+
defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
16+
not isInCycle(useInstr)
1517
} or
1618
TPhiOperand(PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap) {
1719
defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
1820
}
1921

22+
/** Gets a non-phi instruction that defines an operand of `instr`. */
23+
private Instruction getNonPhiOperandDef(Instruction instr) {
24+
result = Construction::getRegisterOperandDefinition(instr, _)
25+
or
26+
result = Construction::getMemoryOperandDefinition(instr, _, _)
27+
}
28+
29+
/**
30+
* Holds if `instr` is part of a cycle in the operand graph that doesn't go
31+
* through a phi instruction and therefore should be impossible.
32+
*
33+
* If such cycles are present, either due to a programming error in the IR
34+
* generation or due to a malformed database, it can cause infinite loops in
35+
* analyses that assume a cycle-free graph of non-phi operands. Therefore it's
36+
* better to remove these operands than to leave cycles in the operand graph.
37+
*/
38+
private predicate isInCycle(Instruction instr) {
39+
getNonPhiOperandDef+(instr) = instr
40+
}
41+
2042
/**
2143
* A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
2244
*/

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,37 @@ private import semmle.code.cpp.ir.internal.OperandTag
88

99
private newtype TOperand =
1010
TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
11-
defInstr = Construction::getRegisterOperandDefinition(useInstr, tag)
11+
defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
12+
not isInCycle(useInstr)
1213
} or
1314
TNonPhiMemoryOperand(Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap) {
14-
defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap)
15+
defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
16+
not isInCycle(useInstr)
1517
} or
1618
TPhiOperand(PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap) {
1719
defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
1820
}
1921

22+
/** Gets a non-phi instruction that defines an operand of `instr`. */
23+
private Instruction getNonPhiOperandDef(Instruction instr) {
24+
result = Construction::getRegisterOperandDefinition(instr, _)
25+
or
26+
result = Construction::getMemoryOperandDefinition(instr, _, _)
27+
}
28+
29+
/**
30+
* Holds if `instr` is part of a cycle in the operand graph that doesn't go
31+
* through a phi instruction and therefore should be impossible.
32+
*
33+
* If such cycles are present, either due to a programming error in the IR
34+
* generation or due to a malformed database, it can cause infinite loops in
35+
* analyses that assume a cycle-free graph of non-phi operands. Therefore it's
36+
* better to remove these operands than to leave cycles in the operand graph.
37+
*/
38+
private predicate isInCycle(Instruction instr) {
39+
getNonPhiOperandDef+(instr) = instr
40+
}
41+
2042
/**
2143
* A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
2244
*/

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,37 @@ private import semmle.code.cpp.ir.internal.OperandTag
88

99
private newtype TOperand =
1010
TRegisterOperand(Instruction useInstr, RegisterOperandTag tag, Instruction defInstr) {
11-
defInstr = Construction::getRegisterOperandDefinition(useInstr, tag)
11+
defInstr = Construction::getRegisterOperandDefinition(useInstr, tag) and
12+
not isInCycle(useInstr)
1213
} or
1314
TNonPhiMemoryOperand(Instruction useInstr, MemoryOperandTag tag, Instruction defInstr, Overlap overlap) {
14-
defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap)
15+
defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
16+
not isInCycle(useInstr)
1517
} or
1618
TPhiOperand(PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap) {
1719
defInstr = Construction::getPhiOperandDefinition(useInstr, predecessorBlock, overlap)
1820
}
1921

22+
/** Gets a non-phi instruction that defines an operand of `instr`. */
23+
private Instruction getNonPhiOperandDef(Instruction instr) {
24+
result = Construction::getRegisterOperandDefinition(instr, _)
25+
or
26+
result = Construction::getMemoryOperandDefinition(instr, _, _)
27+
}
28+
29+
/**
30+
* Holds if `instr` is part of a cycle in the operand graph that doesn't go
31+
* through a phi instruction and therefore should be impossible.
32+
*
33+
* If such cycles are present, either due to a programming error in the IR
34+
* generation or due to a malformed database, it can cause infinite loops in
35+
* analyses that assume a cycle-free graph of non-phi operands. Therefore it's
36+
* better to remove these operands than to leave cycles in the operand graph.
37+
*/
38+
private predicate isInCycle(Instruction instr) {
39+
getNonPhiOperandDef+(instr) = instr
40+
}
41+
2042
/**
2143
* A source operand of an `Instruction`. The operand represents a value consumed by the instruction.
2244
*/

0 commit comments

Comments
 (0)