Skip to content

Commit b316644

Browse files
jbjjbj
committed
C++: SimpleRangeAnalysis for *= by constant
1 parent b6b7272 commit b316644

File tree

4 files changed

+108
-10
lines changed

4 files changed

+108
-10
lines changed

‎cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,29 @@ private predicate multipliesByNegative(Expr expr, Expr operand, float negative)
217217
negative = -1.0
218218
}
219219

220+
private class AssignMulByConstantExpr extends AssignMulExpr {
221+
float constant;
222+
223+
AssignMulByConstantExpr() { constant = this.getRValue().getFullyConverted().getValue().toFloat() }
224+
225+
float getConstant() { result = constant }
226+
}
227+
228+
private class AssignMulByPositiveConstantExpr extends AssignMulByConstantExpr {
229+
AssignMulByPositiveConstantExpr() { constant >= 0.0 }
230+
}
231+
232+
private class AssignMulByNegativeConstantExpr extends AssignMulByConstantExpr {
233+
AssignMulByNegativeConstantExpr() { constant < 0.0 }
234+
}
235+
220236
private class UnsignedAssignMulExpr extends AssignMulExpr {
221-
UnsignedAssignMulExpr() { this.getType().(IntegralType).isUnsigned() }
237+
UnsignedAssignMulExpr() {
238+
this.getType().(IntegralType).isUnsigned() and
239+
// Avoid overlap. It should be slightly cheaper to analyze
240+
// `AssignMulByConstantExpr`.
241+
not this instanceof AssignMulByConstantExpr
242+
}
222243
}
223244

224245
/** Set of expressions which we know how to analyze. */
@@ -253,6 +274,8 @@ private predicate analyzableExpr(Expr e) {
253274
or
254275
e instanceof UnsignedAssignMulExpr
255276
or
277+
e instanceof AssignMulByConstantExpr
278+
or
256279
e instanceof CrementOperation
257280
or
258281
e instanceof RemExpr
@@ -310,6 +333,12 @@ private predicate defDependsOnDef(
310333
exprDependsOnDef(assignMul.getAnOperand(), srcDef, srcVar)
311334
)
312335
or
336+
exists(AssignMulByConstantExpr assignMul |
337+
def = assignMul and
338+
def.getAVariable() = v and
339+
exprDependsOnDef(assignMul.getLValue(), srcDef, srcVar)
340+
)
341+
or
313342
exists(CrementOperation crem |
314343
def = crem and
315344
def.getAVariable() = v and
@@ -365,6 +394,10 @@ private predicate exprDependsOnDef(Expr e, RangeSsaDefinition srcDef, StackVaria
365394
exprDependsOnDef(mulExpr.getAnOperand(), srcDef, srcVar)
366395
)
367396
or
397+
exists(AssignMulByConstantExpr mulExpr | e = mulExpr |
398+
exprDependsOnDef(mulExpr.getLValue(), srcDef, srcVar)
399+
)
400+
or
368401
exists(CrementOperation crementExpr | e = crementExpr |
369402
exprDependsOnDef(crementExpr.getOperand(), srcDef, srcVar)
370403
)
@@ -736,6 +769,18 @@ private float getLowerBoundsImpl(Expr expr) {
736769
result = xLow * yLow
737770
)
738771
or
772+
exists(AssignMulByPositiveConstantExpr mulExpr, float xLow |
773+
expr = mulExpr and
774+
xLow = getFullyConvertedLowerBounds(mulExpr.getLValue()) and
775+
result = xLow * mulExpr.getConstant()
776+
)
777+
or
778+
exists(AssignMulByNegativeConstantExpr mulExpr, float xHigh |
779+
expr = mulExpr and
780+
xHigh = getFullyConvertedUpperBounds(mulExpr.getLValue()) and
781+
result = xHigh * mulExpr.getConstant()
782+
)
783+
or
739784
exists(PrefixIncrExpr incrExpr, float xLow |
740785
expr = incrExpr and
741786
xLow = getFullyConvertedLowerBounds(incrExpr.getOperand()) and
@@ -910,6 +955,18 @@ private float getUpperBoundsImpl(Expr expr) {
910955
result = xHigh * yHigh
911956
)
912957
or
958+
exists(AssignMulByPositiveConstantExpr mulExpr, float xHigh |
959+
expr = mulExpr and
960+
xHigh = getFullyConvertedUpperBounds(mulExpr.getLValue()) and
961+
result = xHigh * mulExpr.getConstant()
962+
)
963+
or
964+
exists(AssignMulByNegativeConstantExpr mulExpr, float xLow |
965+
expr = mulExpr and
966+
xLow = getFullyConvertedLowerBounds(mulExpr.getLValue()) and
967+
result = xLow * mulExpr.getConstant()
968+
)
969+
or
913970
exists(PrefixIncrExpr incrExpr, float xHigh |
914971
expr = incrExpr and
915972
xHigh = getFullyConvertedUpperBounds(incrExpr.getOperand()) and
@@ -1136,6 +1193,20 @@ private float getDefLowerBoundsImpl(RangeSsaDefinition def, StackVariable v) {
11361193
result = lhsLB * rhsLB
11371194
)
11381195
or
1196+
exists(AssignMulByPositiveConstantExpr assignMul, RangeSsaDefinition nextDef, float lhsLB |
1197+
def = assignMul and
1198+
assignMul.getLValue() = nextDef.getAUse(v) and
1199+
lhsLB = getDefLowerBounds(nextDef, v) and
1200+
result = lhsLB * assignMul.getConstant()
1201+
)
1202+
or
1203+
exists(AssignMulByNegativeConstantExpr assignMul, RangeSsaDefinition nextDef, float lhsUB |
1204+
def = assignMul and
1205+
assignMul.getLValue() = nextDef.getAUse(v) and
1206+
lhsUB = getDefUpperBounds(nextDef, v) and
1207+
result = lhsUB * assignMul.getConstant()
1208+
)
1209+
or
11391210
exists(IncrementOperation incr, float newLB |
11401211
def = incr and
11411212
incr.getOperand() = v.getAnAccess() and
@@ -1186,6 +1257,20 @@ private float getDefUpperBoundsImpl(RangeSsaDefinition def, StackVariable v) {
11861257
result = lhsUB * rhsUB
11871258
)
11881259
or
1260+
exists(AssignMulByPositiveConstantExpr assignMul, RangeSsaDefinition nextDef, float lhsUB |
1261+
def = assignMul and
1262+
assignMul.getLValue() = nextDef.getAUse(v) and
1263+
lhsUB = getDefUpperBounds(nextDef, v) and
1264+
result = lhsUB * assignMul.getConstant()
1265+
)
1266+
or
1267+
exists(AssignMulByNegativeConstantExpr assignMul, RangeSsaDefinition nextDef, float lhsLB |
1268+
def = assignMul and
1269+
assignMul.getLValue() = nextDef.getAUse(v) and
1270+
lhsLB = getDefLowerBounds(nextDef, v) and
1271+
result = lhsLB * assignMul.getConstant()
1272+
)
1273+
or
11891274
exists(IncrementOperation incr, float newUB |
11901275
def = incr and
11911276
incr.getOperand() = v.getAnAccess() and

‎cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -519,16 +519,19 @@
519519
| test.c:513:9:513:9 | i | -5 |
520520
| test.c:514:9:514:9 | i | -30 |
521521
| test.c:516:5:516:5 | i | -30 |
522-
| test.c:517:9:517:9 | i | -2147483648 |
523-
| test.c:519:5:519:5 | i | -2147483648 |
524-
| test.c:520:9:520:9 | i | -2147483648 |
522+
| test.c:517:9:517:9 | i | -210 |
523+
| test.c:519:5:519:5 | i | -210 |
524+
| test.c:520:9:520:9 | i | -1155 |
525525
| test.c:522:7:522:7 | i | -2147483648 |
526526
| test.c:523:5:523:5 | i | -2147483648 |
527527
| test.c:523:9:523:9 | i | -1 |
528528
| test.c:524:9:524:9 | i | 1 |
529529
| test.c:526:3:526:3 | i | -2147483648 |
530530
| test.c:526:7:526:7 | i | -2147483648 |
531531
| test.c:527:10:527:10 | i | -2147483648 |
532+
| test.c:530:3:530:3 | i | -2147483648 |
533+
| test.c:530:10:530:11 | sc | 1 |
534+
| test.c:532:7:532:7 | i | -128 |
532535
| test.cpp:10:7:10:7 | b | -2147483648 |
533536
| test.cpp:11:5:11:5 | x | -2147483648 |
534537
| test.cpp:13:10:13:10 | x | -2147483648 |

‎cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,15 +514,22 @@ int mul_by_constant(int i, int j) {
514514
out(i); // -30 .. 15
515515

516516
i *= 7;
517-
out(i); // -210 .. 105 [BUG: not supported]
517+
out(i); // -210 .. 105
518518

519519
i *= -11;
520-
out(i); // -1155 .. 2310 [BUG: not supported]
520+
out(i); // -1155 .. 2310
521521
}
522522
if (i == -1) {
523523
i = i * (int)0xffFFffFF; // fully converted literal is -1
524524
out(i); // 1 .. 1
525525
}
526526
i = i * -1;
527-
return i; // -2^31 .. 2^31-1
527+
out( i); // -2^31 .. 2^31-1
528+
529+
signed char sc = 1;
530+
i = (*&sc *= 2);
531+
out(sc); // demonstrate that we couldn't analyze the LHS of the `*=` above...
532+
out(i); // -128 .. 127 // ... but we can still bound its result by its type.
533+
534+
return 0;
528535
}

‎cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -519,16 +519,19 @@
519519
| test.c:513:9:513:9 | i | 10 |
520520
| test.c:514:9:514:9 | i | 15 |
521521
| test.c:516:5:516:5 | i | 15 |
522-
| test.c:517:9:517:9 | i | 2147483647 |
523-
| test.c:519:5:519:5 | i | 2147483647 |
524-
| test.c:520:9:520:9 | i | 2147483647 |
522+
| test.c:517:9:517:9 | i | 105 |
523+
| test.c:519:5:519:5 | i | 105 |
524+
| test.c:520:9:520:9 | i | 2310 |
525525
| test.c:522:7:522:7 | i | 2147483647 |
526526
| test.c:523:5:523:5 | i | 2147483647 |
527527
| test.c:523:9:523:9 | i | -1 |
528528
| test.c:524:9:524:9 | i | 1 |
529529
| test.c:526:3:526:3 | i | 2147483647 |
530530
| test.c:526:7:526:7 | i | 2147483647 |
531531
| test.c:527:10:527:10 | i | 2147483647 |
532+
| test.c:530:3:530:3 | i | 2147483647 |
533+
| test.c:530:10:530:11 | sc | 1 |
534+
| test.c:532:7:532:7 | i | 127 |
532535
| test.cpp:10:7:10:7 | b | 2147483647 |
533536
| test.cpp:11:5:11:5 | x | 2147483647 |
534537
| test.cpp:13:10:13:10 | x | 2147483647 |

0 commit comments

Comments
 (0)