Skip to content

Commit ebb7f28

Browse files
committed
C++: Remove workaround for missing comparisons against 0 in C code.
1 parent 9810a4f commit ebb7f28

File tree

4 files changed

+508
-45
lines changed

4 files changed

+508
-45
lines changed

cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -899,48 +899,31 @@ private module Cached {
899899
* Holds if `op` is an operand that is eventually used in a unary comparison
900900
* with a constant.
901901
*/
902-
private predicate isRelevantUnaryComparisonOperand(Operand op) {
903-
// Base case: `op` is an operand of a `CompareEQInstruction` or `CompareNEInstruction`,
904-
// and the other operand is a constant.
905-
exists(CompareInstruction eq, Instruction instr |
906-
eq.hasOperands(op, instr.getAUse()) and
907-
exists(int_value(instr))
908-
|
909-
eq instanceof CompareEQInstruction
910-
or
911-
eq instanceof CompareNEInstruction
912-
)
913-
or
914-
// C doesn't have int-to-bool conversions, so `if(x)` will just generate:
915-
// r2_1(glval<int>) = VariableAddress[x]
916-
// r2_2(int) = Load[x] : &:r2_1, m1_6
917-
// v2_3(void) = ConditionalBranch : r2_2
918-
exists(ConditionalBranchInstruction branch | branch.getConditionOperand() = op)
902+
private predicate mayBranchOn(Instruction instr) {
903+
exists(ConditionalBranchInstruction branch | branch.getCondition() = instr)
919904
or
920905
// If `!x` is a relevant unary comparison then so is `x`.
921906
exists(LogicalNotInstruction logicalNot |
922-
isRelevantUnaryComparisonOperand(unique( | | logicalNot.getAUse())) and
923-
logicalNot.getUnaryOperand() = op
907+
mayBranchOn(logicalNot) and
908+
logicalNot.getUnary() = instr
924909
)
925910
or
926911
// If `y` is a relevant unary comparison and `y = x` then so is `x`.
927-
not op.isDefinitionInexact() and
928912
exists(CopyInstruction copy |
929-
isRelevantUnaryComparisonOperand(unique( | | copy.getAUse())) and
930-
op = copy.getSourceValueOperand()
913+
mayBranchOn(copy) and
914+
instr = copy.getSourceValue()
931915
)
932916
or
933917
// If phi(x1, x2) is a relevant unary comparison then so are `x1` and `x2`.
934-
not op.isDefinitionInexact() and
935918
exists(PhiInstruction phi |
936-
isRelevantUnaryComparisonOperand(unique( | | phi.getAUse())) and
937-
op = phi.getAnInputOperand()
919+
mayBranchOn(phi) and
920+
instr = phi.getAnInput()
938921
)
939922
or
940923
// If `__builtin_expect(x)` is a relevant unary comparison then so is `x`.
941924
exists(BuiltinExpectCallInstruction call |
942-
isRelevantUnaryComparisonOperand(unique( | | call.getAUse())) and
943-
op = call.getConditionOperand()
925+
mayBranchOn(call) and
926+
instr = call.getCondition()
944927
)
945928
}
946929

@@ -956,14 +939,24 @@ private module Cached {
956939
case.getValue().toInt() = k
957940
)
958941
or
959-
isRelevantUnaryComparisonOperand(op) and
960-
op.getDef() = test.getAnInstruction() and
961-
(
962-
k = 1 and
963-
value.(BooleanValue).getValue() = true
942+
exists(Instruction const | int_value(const) = k |
943+
value.(BooleanValue).getValue() = true and
944+
test.(CompareEQValueNumber).hasOperands(op, const.getAUse())
964945
or
946+
value.(BooleanValue).getValue() = false and
947+
test.(CompareNEValueNumber).hasOperands(op, const.getAUse())
948+
)
949+
or
950+
exists(BooleanValue bv |
951+
bv = value and
952+
mayBranchOn(op.getDef()) and
953+
op = test.getAUse()
954+
|
965955
k = 0 and
966-
value.(BooleanValue).getValue() = false
956+
bv.getValue() = false
957+
or
958+
k = 1 and
959+
bv.getValue() = true
967960
)
968961
}
969962

0 commit comments

Comments
 (0)