Skip to content

Commit c46f9e4

Browse files
committed
C++: Don't consider additional loads when reusing dataflow operands.
1 parent 50190ef commit c46f9e4

File tree

5 files changed

+14
-13
lines changed

5 files changed

+14
-13
lines changed

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ predicate storeStep(Node node1, ContentSet c, Node node2) { storeStepImpl(node1,
692692
private predicate numberOfLoadsFromOperandRec(
693693
Operand operandFrom, Operand operandTo, int ind, boolean certain
694694
) {
695-
exists(Instruction load | Ssa::isDereference(load, operandFrom) |
695+
exists(Instruction load | Ssa::isDereference(load, operandFrom, _) |
696696
operandTo = operandFrom and ind = 0 and certain = true
697697
or
698698
numberOfLoadsFromOperand(load.getAUse(), operandTo, ind - 1, certain)
@@ -716,7 +716,7 @@ private predicate numberOfLoadsFromOperand(
716716
) {
717717
numberOfLoadsFromOperandRec(operandFrom, operandTo, n, certain)
718718
or
719-
not Ssa::isDereference(_, operandFrom) and
719+
not Ssa::isDereference(_, operandFrom, _) and
720720
not conversionFlow(operandFrom, _, _, _) and
721721
operandFrom = operandTo and
722722
n = 0 and

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ private predicate indirectConversionFlowStep(Node nFrom, Node nTo) {
610610
hasOperandAndIndex(nFrom, op1, pragma[only_bind_into](indirectionIndex)) and
611611
hasOperandAndIndex(nTo, op2, indirectionIndex - 1) and
612612
instr = op2.getDef() and
613-
isDereference(instr, op1)
613+
isDereference(instr, op1, _)
614614
)
615615
)
616616
}

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -320,10 +320,12 @@ private module IteratorIndirections {
320320
}
321321
}
322322

323-
predicate isDereference(Instruction deref, Operand address) {
324-
any(Indirection ind).isAdditionalDereference(deref, address)
323+
predicate isDereference(Instruction deref, Operand address, boolean additional) {
324+
any(Indirection ind).isAdditionalDereference(deref, address) and
325+
additional = true
325326
or
326-
deref.(LoadInstruction).getSourceAddressOperand() = address
327+
deref.(LoadInstruction).getSourceAddressOperand() = address and
328+
additional = false
327329
}
328330

329331
predicate isWrite(Node0Impl value, Operand address, boolean certain) {
@@ -545,7 +547,7 @@ private module Cached {
545547
isDef(_, value, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
546548
isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
547549
iteratorBase.getResultType() instanceof Interfaces::Iterator and
548-
isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse()) and
550+
isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse(), _) and
549551
memory = read.getSideEffectOperand().getAnyDef()
550552
)
551553
}
@@ -786,7 +788,7 @@ private module Cached {
786788
) {
787789
indirectionIndex = [1 .. countIndirectionsForCppType(getLanguageType(operand))] and
788790
exists(Instruction load |
789-
isDereference(load, operand) and
791+
isDereference(load, operand, false) and
790792
operandRepr = unique( | | getAUse(load)) and
791793
indirectionIndexRepr = indirectionIndex - 1
792794
)
@@ -806,7 +808,7 @@ private module Cached {
806808
indirectionIndex = [1 .. countIndirectionsForCppType(getResultLanguageType(instr))] and
807809
exists(Instruction load, Operand address |
808810
address.getDef() = instr and
809-
isDereference(load, address) and
811+
isDereference(load, address, false) and
810812
instrRepr = load and
811813
indirectionIndexRepr = indirectionIndex - 1
812814
)
@@ -829,7 +831,7 @@ private module Cached {
829831
or
830832
exists(int ind0 |
831833
exists(Operand address |
832-
isDereference(operand.getDef(), address) and
834+
isDereference(operand.getDef(), address, _) and
833835
isUseImpl(address, base, ind0)
834836
)
835837
or
@@ -899,7 +901,7 @@ private module Cached {
899901
)
900902
or
901903
exists(Operand address, boolean certain0 |
902-
isDereference(operand.getDef(), address) and
904+
isDereference(operand.getDef(), address, _) and
903905
isDefImpl(address, base, ind - 1, certain0)
904906
|
905907
if isCertainAddress(operand) then certain = certain0 else certain = false

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ private predicate operandToInstructionTaintStep(Operand opFrom, Instruction inst
5757
)
5858
or
5959
// Taint flow from an address to its dereference.
60-
Ssa::isDereference(instrTo, opFrom)
60+
Ssa::isDereference(instrTo, opFrom, _)
6161
or
6262
// Unary instructions tend to preserve enough information in practice that we
6363
// want taint to flow through.

cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,4 @@ WARNING: Module DataFlow has been deprecated and may be removed in future (taint
44
WARNING: Module DataFlow has been deprecated and may be removed in future (taint.ql:68,25-33)
55
WARNING: Module TaintTracking has been deprecated and may be removed in future (taint.ql:73,20-33)
66
testFailures
7-
| vector.cpp:532:10:532:12 | call to operator[] | Unexpected result: ir= |
87
failures

0 commit comments

Comments
 (0)