Skip to content

Commit c6275bf

Browse files
authored
Merge pull request github#13293 from MathiasVP/fix-performance-of-dtt
C++: Fix result duplication on `DefaultTaintTracking`
2 parents 7361ad9 + e7f82a3 commit c6275bf

File tree

2 files changed

+15
-7
lines changed

2 files changed

+15
-7
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1640,8 +1640,15 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) {
16401640
localFlow(instructionNode(e1), instructionNode(e2))
16411641
}
16421642

1643+
/**
1644+
* INTERNAL: Do not use.
1645+
*
1646+
* Ideally this module would be private, but the `asExprInternal` predicate is
1647+
* needed in `DefaultTaintTrackingImpl`. Once `DefaultTaintTrackingImpl` is gone
1648+
* we can make this module private.
1649+
*/
16431650
cached
1644-
private module ExprFlowCached {
1651+
module ExprFlowCached {
16451652
/**
16461653
* Holds if `n` is an indirect operand of a `PointerArithmeticInstruction`, and
16471654
* `e` is the result of loading from the `PointerArithmeticInstruction`.
@@ -1692,7 +1699,8 @@ private module ExprFlowCached {
16921699
* `x[i]` steps to the expression `x[i - 1]` without traversing the
16931700
* entire chain.
16941701
*/
1695-
private Expr asExpr(Node n) {
1702+
cached
1703+
Expr asExprInternal(Node n) {
16961704
isIndirectBaseOfArrayAccess(n, result)
16971705
or
16981706
not isIndirectBaseOfArrayAccess(n, _) and
@@ -1704,7 +1712,7 @@ private module ExprFlowCached {
17041712
* dataflow step.
17051713
*/
17061714
private predicate localStepFromNonExpr(Node n1, Node n2) {
1707-
not exists(asExpr(n1)) and
1715+
not exists(asExprInternal(n1)) and
17081716
localFlowStep(n1, n2)
17091717
}
17101718

@@ -1715,7 +1723,7 @@ private module ExprFlowCached {
17151723
pragma[nomagic]
17161724
private predicate localStepsToExpr(Node n1, Node n2, Expr e2) {
17171725
localStepFromNonExpr*(n1, n2) and
1718-
e2 = asExpr(n2)
1726+
e2 = asExprInternal(n2)
17191727
}
17201728

17211729
/**
@@ -1726,7 +1734,7 @@ private module ExprFlowCached {
17261734
exists(Node mid |
17271735
localFlowStep(n1, mid) and
17281736
localStepsToExpr(mid, n2, e2) and
1729-
e1 = asExpr(n1)
1737+
e1 = asExprInternal(n1)
17301738
)
17311739
}
17321740

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ private DataFlow::Node getNodeForSource(Expr source) {
6060
}
6161

6262
private DataFlow::Node getNodeForExpr(Expr node) {
63-
result = DataFlow::exprNode(node)
63+
node = DataFlow::ExprFlowCached::asExprInternal(result)
6464
or
6565
// Some of the sources in `isUserInput` are intended to match the value of
6666
// an expression, while others (those modeled below) are intended to match
@@ -221,7 +221,7 @@ private module Cached {
221221
predicate nodeIsBarrierIn(DataFlow::Node node) {
222222
// don't use dataflow into taint sources, as this leads to duplicate results.
223223
exists(Expr source | isUserInput(source, _) |
224-
node = DataFlow::exprNode(source)
224+
source = DataFlow::ExprFlowCached::asExprInternal(node)
225225
or
226226
// This case goes together with the similar (but not identical) rule in
227227
// `getNodeForSource`.

0 commit comments

Comments
 (0)