Skip to content

Commit 1ee96a4

Browse files
committed
C++: SimpleRangeAnalysis: unsigned multiplication
1 parent 2ea25b9 commit 1ee96a4

File tree

4 files changed

+48
-26
lines changed

4 files changed

+48
-26
lines changed

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ float safeFloor(float v) {
156156
result = v
157157
}
158158

159+
private class UnsignedMulExpr extends MulExpr {
160+
UnsignedMulExpr() { this.getType().(IntegralType).isUnsigned() }
161+
}
162+
159163
/** Set of expressions which we know how to analyze. */
160164
private predicate analyzableExpr(Expr e) {
161165
// The type of the expression must be arithmetic. We reuse the logic in
@@ -178,6 +182,8 @@ private predicate analyzableExpr(Expr e) {
178182
or
179183
e instanceof SubExpr
180184
or
185+
e instanceof UnsignedMulExpr
186+
or
181187
e instanceof AssignExpr
182188
or
183189
e instanceof AssignAddExpr
@@ -278,6 +284,8 @@ private predicate exprDependsOnDef(Expr e, RangeSsaDefinition srcDef, StackVaria
278284
or
279285
exists(SubExpr subExpr | e = subExpr | exprDependsOnDef(subExpr.getAnOperand(), srcDef, srcVar))
280286
or
287+
exists(UnsignedMulExpr mulExpr | e = mulExpr | exprDependsOnDef(mulExpr.getAnOperand(), srcDef, srcVar))
288+
or
281289
exists(AssignExpr addExpr | e = addExpr | exprDependsOnDef(addExpr.getRValue(), srcDef, srcVar))
282290
or
283291
exists(AssignAddExpr addExpr | e = addExpr |
@@ -625,6 +633,13 @@ private float getLowerBoundsImpl(Expr expr) {
625633
result = addRoundingDown(xLow, -yHigh)
626634
)
627635
or
636+
exists(UnsignedMulExpr mulExpr, float xLow, float yLow |
637+
expr = mulExpr and
638+
xLow = getFullyConvertedLowerBounds(mulExpr.getLeftOperand()) and
639+
yLow = getFullyConvertedLowerBounds(mulExpr.getRightOperand()) and
640+
result = xLow * yLow
641+
)
642+
or
628643
exists(AssignExpr assign |
629644
expr = assign and
630645
result = getFullyConvertedLowerBounds(assign.getRValue())
@@ -794,6 +809,13 @@ private float getUpperBoundsImpl(Expr expr) {
794809
result = addRoundingUp(xHigh, -yLow)
795810
)
796811
or
812+
exists(UnsignedMulExpr mulExpr, float xHigh, float yHigh |
813+
expr = mulExpr and
814+
xHigh = getFullyConvertedUpperBounds(mulExpr.getLeftOperand()) and
815+
yHigh = getFullyConvertedUpperBounds(mulExpr.getRightOperand()) and
816+
result = xHigh * yHigh
817+
)
818+
or
797819
exists(AssignExpr assign |
798820
expr = assign and
799821
result = getFullyConvertedUpperBounds(assign.getRValue())

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -447,40 +447,40 @@
447447
| test.c:433:13:433:13 | a | 3 |
448448
| test.c:433:15:433:15 | b | 5 |
449449
| test.c:434:5:434:9 | total | 0 |
450-
| test.c:434:14:434:14 | r | -2147483648 |
450+
| test.c:434:14:434:14 | r | 15 |
451451
| test.c:436:12:436:12 | a | 0 |
452452
| test.c:436:17:436:17 | a | 3 |
453453
| test.c:436:33:436:33 | b | 0 |
454454
| test.c:436:38:436:38 | b | 0 |
455455
| test.c:437:13:437:13 | a | 3 |
456456
| test.c:437:15:437:15 | b | 0 |
457-
| test.c:438:5:438:9 | total | -2147483648 |
458-
| test.c:438:14:438:14 | r | -2147483648 |
457+
| test.c:438:5:438:9 | total | 0 |
458+
| test.c:438:14:438:14 | r | 0 |
459459
| test.c:440:12:440:12 | a | 0 |
460460
| test.c:440:17:440:17 | a | 3 |
461461
| test.c:440:34:440:34 | b | 0 |
462462
| test.c:440:39:440:39 | b | 13 |
463463
| test.c:441:13:441:13 | a | 3 |
464464
| test.c:441:15:441:15 | b | 13 |
465-
| test.c:442:5:442:9 | total | -2147483648 |
466-
| test.c:442:14:442:14 | r | -2147483648 |
467-
| test.c:445:10:445:14 | total | -2147483648 |
465+
| test.c:442:5:442:9 | total | 0 |
466+
| test.c:442:14:442:14 | r | 39 |
467+
| test.c:445:10:445:14 | total | 0 |
468468
| test.c:451:12:451:12 | b | 0 |
469469
| test.c:451:17:451:17 | b | 5 |
470470
| test.c:452:16:452:16 | b | 5 |
471471
| test.c:453:5:453:9 | total | 0 |
472-
| test.c:453:14:453:14 | r | -2147483648 |
472+
| test.c:453:14:453:14 | r | 55 |
473473
| test.c:455:12:455:12 | b | 0 |
474474
| test.c:455:17:455:17 | b | 0 |
475475
| test.c:456:16:456:16 | b | 0 |
476-
| test.c:457:5:457:9 | total | -2147483648 |
477-
| test.c:457:14:457:14 | r | -2147483648 |
476+
| test.c:457:5:457:9 | total | 0 |
477+
| test.c:457:14:457:14 | r | 0 |
478478
| test.c:459:13:459:13 | b | 0 |
479479
| test.c:459:18:459:18 | b | 13 |
480480
| test.c:460:16:460:16 | b | 13 |
481-
| test.c:461:5:461:9 | total | -2147483648 |
482-
| test.c:461:14:461:14 | r | -2147483648 |
483-
| test.c:464:10:464:14 | total | -2147483648 |
481+
| test.c:461:5:461:9 | total | 0 |
482+
| test.c:461:14:461:14 | r | 143 |
483+
| test.c:464:10:464:14 | total | 0 |
484484
| test.cpp:10:7:10:7 | b | -2147483648 |
485485
| test.cpp:11:5:11:5 | x | -2147483648 |
486486
| test.cpp:13:10:13:10 | x | -2147483648 |

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -447,40 +447,40 @@
447447
| test.c:433:13:433:13 | a | 11 |
448448
| test.c:433:15:433:15 | b | 23 |
449449
| test.c:434:5:434:9 | total | 0 |
450-
| test.c:434:14:434:14 | r | 2147483647 |
450+
| test.c:434:14:434:14 | r | 253 |
451451
| test.c:436:12:436:12 | a | 4294967295 |
452452
| test.c:436:17:436:17 | a | 4294967295 |
453453
| test.c:436:33:436:33 | b | 4294967295 |
454454
| test.c:436:38:436:38 | b | 4294967295 |
455455
| test.c:437:13:437:13 | a | 11 |
456456
| test.c:437:15:437:15 | b | 23 |
457-
| test.c:438:5:438:9 | total | 2147483647 |
458-
| test.c:438:14:438:14 | r | 2147483647 |
457+
| test.c:438:5:438:9 | total | 253 |
458+
| test.c:438:14:438:14 | r | 253 |
459459
| test.c:440:12:440:12 | a | 4294967295 |
460460
| test.c:440:17:440:17 | a | 4294967295 |
461461
| test.c:440:34:440:34 | b | 4294967295 |
462462
| test.c:440:39:440:39 | b | 4294967295 |
463463
| test.c:441:13:441:13 | a | 11 |
464464
| test.c:441:15:441:15 | b | 23 |
465-
| test.c:442:5:442:9 | total | 2147483647 |
466-
| test.c:442:14:442:14 | r | 2147483647 |
467-
| test.c:445:10:445:14 | total | 2147483647 |
465+
| test.c:442:5:442:9 | total | 506 |
466+
| test.c:442:14:442:14 | r | 253 |
467+
| test.c:445:10:445:14 | total | 759 |
468468
| test.c:451:12:451:12 | b | 4294967295 |
469469
| test.c:451:17:451:17 | b | 4294967295 |
470470
| test.c:452:16:452:16 | b | 23 |
471471
| test.c:453:5:453:9 | total | 0 |
472-
| test.c:453:14:453:14 | r | 2147483647 |
472+
| test.c:453:14:453:14 | r | 253 |
473473
| test.c:455:12:455:12 | b | 4294967295 |
474474
| test.c:455:17:455:17 | b | 4294967295 |
475475
| test.c:456:16:456:16 | b | 23 |
476-
| test.c:457:5:457:9 | total | 2147483647 |
477-
| test.c:457:14:457:14 | r | 2147483647 |
476+
| test.c:457:5:457:9 | total | 253 |
477+
| test.c:457:14:457:14 | r | 253 |
478478
| test.c:459:13:459:13 | b | 4294967295 |
479479
| test.c:459:18:459:18 | b | 4294967295 |
480480
| test.c:460:16:460:16 | b | 23 |
481-
| test.c:461:5:461:9 | total | 2147483647 |
482-
| test.c:461:14:461:14 | r | 2147483647 |
483-
| test.c:464:10:464:14 | total | 2147483647 |
481+
| test.c:461:5:461:9 | total | 506 |
482+
| test.c:461:14:461:14 | r | 253 |
483+
| test.c:464:10:464:14 | total | 759 |
484484
| test.cpp:10:7:10:7 | b | 2147483647 |
485485
| test.cpp:11:5:11:5 | x | 2147483647 |
486486
| test.cpp:13:10:13:10 | x | 2147483647 |

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,10 +385,10 @@ void bitwise_ands()
385385

386386
void unsigned_mult(unsigned int x, unsigned int y) {
387387
if(x < 13 && y < 35) {
388-
if(x * y > 1024) {} // always false [NOT DETECTED]
388+
if(x * y > 1024) {} // always false
389389
if(x * y < 204) {}
390390
if(x >= 3 && y >= 2) {
391-
if(x * y < 5) {} // always false [NOT DETECTED]
391+
if(x * y < 5) {} // always false
392392
}
393393
}
394394
}

0 commit comments

Comments
 (0)