Skip to content

Commit da13dc6

Browse files
committed
C++ IR: Don't propagate GVN through non-exact Copy
The `ValueNumbering` library is supposed to propagate value numberings through a `CopyInstruction` only when it's _congruent_, meaning it must have exact overlap with its source. A `CopyInstruction` can be a `LoadInstruction`, a `StoreInstruction`, or a `CopyValueInstruction`. The latter is also a `UnaryInstruction`, and the value numbering rule for `UnaryInstruction` applied to it as well. This meant that value numbering would propagate even through a non-congruent `CopyValueInstruction`. That's semantically wrong but probably only an issue in very rare circumstances, and it should get corrected when we change the definition of `getUnary` to require congruence. What's worse is the performance implications. It meant that the value numbering IPA witness could take two different paths through every `CopyValueInstruction`. If multiple `CopyValueInstruction`s were chained, this would lead to an exponential number of variable numbers for the same `Instruction`, and we would run out of time and space while performing value numbering. This fixes the performance of `ValueNumbering.qll` on https://github.com/asterisk/asterisk, although this project might also require a separate change for fixing an infinite loop in the IR constant analysis.
1 parent 46d7792 commit da13dc6

File tree

3 files changed

+6
-3
lines changed

3 files changed

+6
-3
lines changed

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private predicate numberableInstruction(Instruction instr) {
124124
instr instanceof StringConstantInstruction or
125125
instr instanceof FieldAddressInstruction or
126126
instr instanceof BinaryInstruction or
127-
instr instanceof UnaryInstruction or
127+
(instr instanceof UnaryInstruction and not instr instanceof CopyInstruction) or
128128
instr instanceof PointerArithmeticInstruction or
129129
instr instanceof CongruentCopyInstruction
130130
}
@@ -191,6 +191,7 @@ private predicate unaryValueNumber(UnaryInstruction instr, IRFunction irFunc, Op
191191
Type type, ValueNumber operand) {
192192
instr.getEnclosingIRFunction() = irFunc and
193193
(not instr instanceof InheritanceConversionInstruction) and
194+
(not instr instanceof CopyInstruction) and
194195
instr.getOpcode() = opcode and
195196
instr.getResultType() = type and
196197
valueNumber(instr.getUnary()) = operand

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private predicate numberableInstruction(Instruction instr) {
124124
instr instanceof StringConstantInstruction or
125125
instr instanceof FieldAddressInstruction or
126126
instr instanceof BinaryInstruction or
127-
instr instanceof UnaryInstruction or
127+
(instr instanceof UnaryInstruction and not instr instanceof CopyInstruction) or
128128
instr instanceof PointerArithmeticInstruction or
129129
instr instanceof CongruentCopyInstruction
130130
}
@@ -191,6 +191,7 @@ private predicate unaryValueNumber(UnaryInstruction instr, IRFunction irFunc, Op
191191
Type type, ValueNumber operand) {
192192
instr.getEnclosingIRFunction() = irFunc and
193193
(not instr instanceof InheritanceConversionInstruction) and
194+
(not instr instanceof CopyInstruction) and
194195
instr.getOpcode() = opcode and
195196
instr.getResultType() = type and
196197
valueNumber(instr.getUnary()) = operand

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private predicate numberableInstruction(Instruction instr) {
124124
instr instanceof StringConstantInstruction or
125125
instr instanceof FieldAddressInstruction or
126126
instr instanceof BinaryInstruction or
127-
instr instanceof UnaryInstruction or
127+
(instr instanceof UnaryInstruction and not instr instanceof CopyInstruction) or
128128
instr instanceof PointerArithmeticInstruction or
129129
instr instanceof CongruentCopyInstruction
130130
}
@@ -191,6 +191,7 @@ private predicate unaryValueNumber(UnaryInstruction instr, IRFunction irFunc, Op
191191
Type type, ValueNumber operand) {
192192
instr.getEnclosingIRFunction() = irFunc and
193193
(not instr instanceof InheritanceConversionInstruction) and
194+
(not instr instanceof CopyInstruction) and
194195
instr.getOpcode() = opcode and
195196
instr.getResultType() = type and
196197
valueNumber(instr.getUnary()) = operand

0 commit comments

Comments
 (0)