Skip to content

Commit 2324ce7

Browse files
committed
C++ IR: Fix soundness of ConstantAnalysis
Now that `PhiInstruction.getAnInput` only has results for congruent operands, a previous optimization I made to `getConstantValue` is no longer sound. We have to check that all phi inputs give the same value, not just the congruent ones. After this change, if there are any non-congruent operands on a phi instruction, the whole aggregate will have no result.
1 parent 7fb43a5 commit 2324ce7

File tree

3 files changed

+6
-30
lines changed

3 files changed

+6
-30
lines changed

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/constant/ConstantAnalysis.qll

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,8 @@ int getConstantValue(Instruction instr) {
1010
result = getConstantValue(instr.(CopyInstruction).getSourceValue()) or
1111
exists(PhiInstruction phi |
1212
phi = instr and
13-
result = max(Instruction def | def = phi.getAnInput() | getConstantValueToPhi(def)) and
14-
result = min(Instruction def | def = phi.getAnInput() | getConstantValueToPhi(def))
15-
)
16-
}
17-
18-
pragma[noinline]
19-
int getConstantValueToPhi(Instruction def) {
20-
exists(PhiInstruction phi |
21-
result = getConstantValue(def) and
22-
def = phi.getAnInput()
13+
result = max(Operand op | op = phi.getAnInputOperand() | getConstantValue(op.getDef())) and
14+
result = min(Operand op | op = phi.getAnInputOperand() | getConstantValue(op.getDef()))
2315
)
2416
}
2517

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/constant/ConstantAnalysis.qll

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,8 @@ int getConstantValue(Instruction instr) {
1010
result = getConstantValue(instr.(CopyInstruction).getSourceValue()) or
1111
exists(PhiInstruction phi |
1212
phi = instr and
13-
result = max(Instruction def | def = phi.getAnInput() | getConstantValueToPhi(def)) and
14-
result = min(Instruction def | def = phi.getAnInput() | getConstantValueToPhi(def))
15-
)
16-
}
17-
18-
pragma[noinline]
19-
int getConstantValueToPhi(Instruction def) {
20-
exists(PhiInstruction phi |
21-
result = getConstantValue(def) and
22-
def = phi.getAnInput()
13+
result = max(Operand op | op = phi.getAnInputOperand() | getConstantValue(op.getDef())) and
14+
result = min(Operand op | op = phi.getAnInputOperand() | getConstantValue(op.getDef()))
2315
)
2416
}
2517

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/constant/ConstantAnalysis.qll

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,8 @@ int getConstantValue(Instruction instr) {
1010
result = getConstantValue(instr.(CopyInstruction).getSourceValue()) or
1111
exists(PhiInstruction phi |
1212
phi = instr and
13-
result = max(Instruction def | def = phi.getAnInput() | getConstantValueToPhi(def)) and
14-
result = min(Instruction def | def = phi.getAnInput() | getConstantValueToPhi(def))
15-
)
16-
}
17-
18-
pragma[noinline]
19-
int getConstantValueToPhi(Instruction def) {
20-
exists(PhiInstruction phi |
21-
result = getConstantValue(def) and
22-
def = phi.getAnInput()
13+
result = max(Operand op | op = phi.getAnInputOperand() | getConstantValue(op.getDef())) and
14+
result = min(Operand op | op = phi.getAnInputOperand() | getConstantValue(op.getDef()))
2315
)
2416
}
2517

0 commit comments

Comments
 (0)