Skip to content

Commit 657c29f

Browse files
aschackmullaschackmull
committed
Java/C++: Share valueFlowStep.
1 parent b8e7e1d commit 657c29f

File tree

10 files changed

+61
-99
lines changed

10 files changed

+61
-99
lines changed

‎cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisConstantSpecific.qll

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ module CppLangImplConstant implements LangSig<Sem, FloatDelta> {
2626
*/
2727
predicate hasBound(SemExpr e, SemExpr bound, float delta, boolean upper) { none() }
2828

29-
/**
30-
* Holds if the value of `dest` is known to be `src + delta`.
31-
*/
32-
predicate additionalValueFlowStep(SemExpr dest, SemExpr src, float delta) { none() }
33-
3429
/**
3530
* Gets the type that range analysis should use to track the result of the specified expression,
3631
* if a type other than the original type of the expression is to be used.

‎cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisImpl.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ module Sem implements Semantic {
9494

9595
class SsaExplicitUpdate = SemSsaExplicitUpdate;
9696

97+
predicate additionalValueFlowStep(SemExpr dest, SemExpr src, int delta) { none() }
98+
9799
predicate conversionCannotOverflow(Type fromType, Type toType) {
98100
SemanticType::conversionCannotOverflow(fromType, toType)
99101
}

‎cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisRelativeSpecific.qll

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ module CppLangImplRelative implements LangSig<Sem, FloatDelta> {
5858
*/
5959
predicate hasBound(SemExpr e, SemExpr bound, float delta, boolean upper) { none() }
6060

61-
/**
62-
* Holds if the value of `dest` is known to be `src + delta`.
63-
*/
64-
predicate additionalValueFlowStep(SemExpr dest, SemExpr src, float delta) { none() }
65-
6661
/**
6762
* Gets the type that range analysis should use to track the result of the specified expression,
6863
* if a type other than the original type of the expression is to be used.

‎cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,33 +9,6 @@ private import RangeAnalysisImpl
99
private import ConstantAnalysis
1010

1111
module RangeUtil<DeltaSig D, LangSig<Sem, D> Lang> implements UtilSig<Sem, D> {
12-
/**
13-
* Holds if `e1 + delta` equals `e2`.
14-
*/
15-
predicate semValueFlowStep(SemExpr e2, SemExpr e1, D::Delta delta) {
16-
e2.(SemCopyValueExpr).getOperand() = e1 and delta = D::fromFloat(0)
17-
or
18-
e2.(SemStoreExpr).getOperand() = e1 and delta = D::fromFloat(0)
19-
or
20-
e2.(SemAddOneExpr).getOperand() = e1 and delta = D::fromFloat(1)
21-
or
22-
e2.(SemSubOneExpr).getOperand() = e1 and delta = D::fromFloat(-1)
23-
or
24-
Lang::additionalValueFlowStep(e2, e1, delta)
25-
or
26-
exists(SemExpr x | e2.(SemAddExpr).hasOperands(e1, x) |
27-
D::fromInt(x.(SemConstantIntegerExpr).getIntValue()) = delta
28-
)
29-
or
30-
exists(SemExpr x, SemSubExpr sub |
31-
e2 = sub and
32-
sub.getLeftOperand() = e1 and
33-
sub.getRightOperand() = x
34-
|
35-
D::fromInt(-x.(SemConstantIntegerExpr).getIntValue()) = delta
36-
)
37-
}
38-
3912
/**
4013
* Gets the type used to track the specified expression's range information.
4114
*

‎java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,8 @@ module Sem implements Semantic {
255255
Expr getDefiningExpr() { result = super.getDefiningExpr() }
256256
}
257257

258+
predicate additionalValueFlowStep = RU::additionalValueFlowStep/3;
259+
258260
predicate conversionCannotOverflow = safeCast/2;
259261
}
260262

@@ -360,8 +362,6 @@ module JavaLangImpl implements LangSig<Sem, IntDelta> {
360362

361363
predicate ignoreExprBound(Sem::Expr e) { none() }
362364

363-
predicate additionalValueFlowStep(Sem::Expr dest, Sem::Expr src, int delta) { none() }
364-
365365
Sem::Type getAlternateType(Sem::Expr e) { none() }
366366

367367
Sem::Type getAlternateTypeForSsaVariable(Sem::SsaVariable var) { none() }
@@ -370,10 +370,6 @@ module JavaLangImpl implements LangSig<Sem, IntDelta> {
370370
}
371371

372372
module Utils implements UtilSig<Sem, IntDelta> {
373-
private import RangeUtils as RU
374-
375-
predicate semValueFlowStep = RU::valueFlowStep/3;
376-
377373
Sem::Type getTrackedTypeForSsaVariable(Sem::SsaVariable var) {
378374
result = var.getSourceVariable().getType()
379375
}

‎java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll

Lines changed: 3 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ predicate ssaRead = U::ssaRead/2;
1717

1818
predicate ssaUpdateStep = U::ssaUpdateStep/3;
1919

20+
predicate valueFlowStep = U::valueFlowStep/3;
21+
2022
predicate guardDirectlyControlsSsaRead = U::guardDirectlyControlsSsaRead/3;
2123

2224
predicate guardControlsSsaRead = U::guardControlsSsaRead/3;
@@ -163,50 +165,10 @@ class ConstantStringExpr extends Expr {
163165
/**
164166
* Holds if `e1 + delta` equals `e2`.
165167
*/
166-
predicate valueFlowStep(Expr e2, Expr e1, int delta) {
167-
e2.(AssignExpr).getSource() = e1 and delta = 0
168-
or
169-
e2.(PlusExpr).getExpr() = e1 and delta = 0
170-
or
171-
e2.(PostIncExpr).getExpr() = e1 and delta = 0
172-
or
173-
e2.(PostDecExpr).getExpr() = e1 and delta = 0
174-
or
175-
e2.(PreIncExpr).getExpr() = e1 and delta = 1
176-
or
177-
e2.(PreDecExpr).getExpr() = e1 and delta = -1
178-
or
168+
predicate additionalValueFlowStep(Expr e2, Expr e1, int delta) {
179169
exists(ArrayCreationExpr a |
180170
arrayLengthDef(e2, a) and
181171
a.getDimension(0) = e1 and
182172
delta = 0
183173
)
184-
or
185-
exists(Expr x |
186-
e2.(AddExpr).hasOperands(e1, x)
187-
or
188-
exists(AssignAddExpr add | add = e2 |
189-
add.getDest() = e1 and add.getRhs() = x
190-
or
191-
add.getDest() = x and add.getRhs() = e1
192-
)
193-
|
194-
x.(ConstantIntegerExpr).getIntValue() = delta
195-
)
196-
or
197-
exists(Expr x |
198-
exists(SubExpr sub |
199-
e2 = sub and
200-
sub.getLeftOperand() = e1 and
201-
sub.getRightOperand() = x
202-
)
203-
or
204-
exists(AssignSubExpr sub |
205-
e2 = sub and
206-
sub.getDest() = e1 and
207-
sub.getRhs() = x
208-
)
209-
|
210-
x.(ConstantIntegerExpr).getIntValue() = -delta
211-
)
212174
}

‎java/ql/test/library-tests/dataflow/range-analysis/RangeAnalysis.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
| A.java:12:16:12:20 | ... + ... | SSA init(y) | 1 | upper | NoReason |
6060
| A.java:12:20:12:20 | 1 | 0 | 1 | lower | NoReason |
6161
| A.java:12:20:12:20 | 1 | 0 | 1 | upper | NoReason |
62+
| A.java:13:13:13:23 | sum | SSA init(y) | 400 | upper | NoReason |
6263
| A.java:13:19:13:19 | x | 0 | 400 | upper | ... > ... |
6364
| A.java:13:19:13:19 | x | SSA init(x) | 0 | lower | NoReason |
6465
| A.java:13:19:13:19 | x | SSA init(x) | 0 | upper | NoReason |
@@ -72,6 +73,17 @@
7273
| A.java:15:13:15:13 | y | SSA init(y) | 0 | upper | NoReason |
7374
| A.java:15:17:15:19 | 300 | 0 | 300 | lower | NoReason |
7475
| A.java:15:17:15:19 | 300 | 0 | 300 | upper | NoReason |
76+
| A.java:16:15:16:25 | sum | 0 | 603 | lower | ... > ... |
77+
| A.java:16:15:16:25 | sum | 0 | 799 | upper | ... != ... |
78+
| A.java:16:15:16:25 | sum | 0 | 799 | upper | ... > ... |
79+
| A.java:16:15:16:25 | sum | SSA init(x) | 301 | lower | ... != ... |
80+
| A.java:16:15:16:25 | sum | SSA init(x) | 301 | lower | NoReason |
81+
| A.java:16:15:16:25 | sum | SSA init(x) | 399 | upper | ... != ... |
82+
| A.java:16:15:16:25 | sum | SSA init(x) | 399 | upper | NoReason |
83+
| A.java:16:15:16:25 | sum | SSA init(y) | 302 | lower | ... != ... |
84+
| A.java:16:15:16:25 | sum | SSA init(y) | 302 | lower | NoReason |
85+
| A.java:16:15:16:25 | sum | SSA init(y) | 400 | upper | ... != ... |
86+
| A.java:16:15:16:25 | sum | SSA init(y) | 400 | upper | NoReason |
7587
| A.java:16:21:16:21 | x | 0 | 302 | lower | ... > ... |
7688
| A.java:16:21:16:21 | x | 0 | 400 | upper | ... > ... |
7789
| A.java:16:21:16:21 | x | SSA init(x) | 0 | lower | NoReason |

‎shared/rangeanalysis/codeql/rangeanalysis/ModulusAnalysis.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ module ModulusAnalysis<
260260
or
261261
exists(Sem::Expr mid, int val0, int delta |
262262
exprModulus(mid, b, val0, mod) and
263-
U::semValueFlowStep(e, mid, D::fromInt(delta)) and
263+
valueFlowStep(e, mid, D::fromInt(delta)) and
264264
val = remainder(val0 + delta, mod)
265265
)
266266
or

‎shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ signature module Semantic {
203203
Expr getDefiningExpr();
204204
}
205205

206+
/**
207+
* Holds if the value of `dest` is known to be `src + delta`.
208+
*/
209+
predicate additionalValueFlowStep(Expr dest, Expr src, int delta);
210+
206211
predicate conversionCannotOverflow(Type fromType, Type toType);
207212
}
208213

@@ -277,11 +282,6 @@ signature module LangSig<Semantic Sem, DeltaSig D> {
277282
*/
278283
predicate ignoreExprBound(Sem::Expr e);
279284

280-
/**
281-
* Holds if the value of `dest` is known to be `src + delta`.
282-
*/
283-
predicate additionalValueFlowStep(Sem::Expr dest, Sem::Expr src, D::Delta delta);
284-
285285
/**
286286
* Gets the type that range analysis should use to track the result of the specified expression,
287287
* if a type other than the original type of the expression is to be used.
@@ -304,8 +304,6 @@ signature module LangSig<Semantic Sem, DeltaSig D> {
304304
}
305305

306306
signature module UtilSig<Semantic Sem, DeltaSig DeltaParam> {
307-
predicate semValueFlowStep(Sem::Expr e2, Sem::Expr e1, DeltaParam::Delta delta);
308-
309307
/**
310308
* Gets the type used to track the specified source variable's range information.
311309
*
@@ -711,7 +709,7 @@ module RangeStage<
711709
* - `upper = false` : `e2 >= e1 + delta`
712710
*/
713711
private predicate boundFlowStep(Sem::Expr e2, Sem::Expr e1, D::Delta delta, boolean upper) {
714-
semValueFlowStep(e2, e1, delta) and
712+
valueFlowStep(e2, e1, delta) and
715713
(upper = true or upper = false)
716714
or
717715
e2.(SafeCastExpr).getOperand() = e1 and
@@ -1344,8 +1342,8 @@ module RangeStage<
13441342
Sem::AddExpr add, boolean upper, SemBound b, boolean isLeft, D::Delta delta,
13451343
boolean fromBackEdge, D::Delta origdelta, SemReason reason
13461344
) {
1347-
// `semValueFlowStep` already handles the case where one of the operands is a constant.
1348-
not semValueFlowStep(add, _, _) and
1345+
// `valueFlowStep` already handles the case where one of the operands is a constant.
1346+
not valueFlowStep(add, _, _) and
13491347
(
13501348
isLeft = true and
13511349
bounded(add.getLeftOperand(), b, delta, upper, fromBackEdge, origdelta, reason)
@@ -1364,8 +1362,8 @@ module RangeStage<
13641362
Sem::SubExpr sub, boolean upper, SemBound b, D::Delta delta, boolean fromBackEdge,
13651363
D::Delta origdelta, SemReason reason
13661364
) {
1367-
// `semValueFlowStep` already handles the case where one of the operands is a constant.
1368-
not semValueFlowStep(sub, _, _) and
1365+
// `valueFlowStep` already handles the case where one of the operands is a constant.
1366+
not valueFlowStep(sub, _, _) and
13691367
bounded(sub.getLeftOperand(), b, delta, upper, fromBackEdge, origdelta, reason)
13701368
}
13711369

@@ -1380,8 +1378,8 @@ module RangeStage<
13801378
private predicate boundedSubOperandRight(
13811379
Sem::SubExpr sub, boolean upper, D::Delta delta, boolean fromBackEdge
13821380
) {
1383-
// `semValueFlowStep` already handles the case where one of the operands is a constant.
1384-
not semValueFlowStep(sub, _, _) and
1381+
// `valueFlowStep` already handles the case where one of the operands is a constant.
1382+
not valueFlowStep(sub, _, _) and
13851383
bounded(sub.getRightOperand(), any(SemZeroBound zb), delta, upper.booleanNot(), fromBackEdge, _,
13861384
_)
13871385
}

‎shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,35 @@ module MakeUtils<Semantic Lang, DeltaSig D> {
8484
)
8585
}
8686

87+
/**
88+
* Holds if `e1 + delta` equals `e2`.
89+
*/
90+
predicate valueFlowStep(Expr e2, Expr e1, D::Delta delta) {
91+
e2.(CopyValueExpr).getOperand() = e1 and delta = D::fromFloat(0)
92+
or
93+
e2.(PostIncExpr).getOperand() = e1 and delta = D::fromFloat(0)
94+
or
95+
e2.(PostDecExpr).getOperand() = e1 and delta = D::fromFloat(0)
96+
or
97+
e2.(PreIncExpr).getOperand() = e1 and delta = D::fromFloat(1)
98+
or
99+
e2.(PreDecExpr).getOperand() = e1 and delta = D::fromFloat(-1)
100+
or
101+
additionalValueFlowStep(e2, e1, D::toInt(delta))
102+
or
103+
exists(Expr x | e2.(AddExpr).hasOperands(e1, x) |
104+
D::fromInt(x.(ConstantIntegerExpr).getIntValue()) = delta
105+
)
106+
or
107+
exists(Expr x, SubExpr sub |
108+
e2 = sub and
109+
sub.getLeftOperand() = e1 and
110+
sub.getRightOperand() = x
111+
|
112+
D::fromInt(-x.(ConstantIntegerExpr).getIntValue()) = delta
113+
)
114+
}
115+
87116
private newtype TSsaReadPosition =
88117
TSsaReadPositionBlock(BasicBlock bb) {
89118
exists(SsaVariable v | v.getAUse().getBasicBlock() = bb)

0 commit comments

Comments
 (0)