Skip to content

Commit f5ddb1d

Browse files
committed
C++: Remove safeFloor in simple range analysis
1 parent bd24fb0 commit f5ddb1d

File tree

5 files changed

+12
-28
lines changed

5 files changed

+12
-28
lines changed

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

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -158,22 +158,6 @@ private class UnsignedBitwiseAndExpr extends BitwiseAndExpr {
158158
}
159159
}
160160

161-
/**
162-
* Gets the floor of `v`, with additional logic to work around issues with
163-
* large numbers.
164-
*/
165-
bindingset[v]
166-
float safeFloor(float v) {
167-
// return the floor of v
168-
v.abs() < 2.pow(31) and
169-
result = v.floor()
170-
or
171-
// `floor()` doesn't work correctly on large numbers (since it returns an integer),
172-
// so fall back to unrounded numbers at this scale.
173-
not v.abs() < 2.pow(31) and
174-
result = v
175-
}
176-
177161
/** A `MulExpr` where exactly one operand is constant. */
178162
private class MulByConstantExpr extends MulExpr {
179163
float constant;
@@ -1266,7 +1250,7 @@ private float getLowerBoundsImpl(Expr expr) {
12661250
rsExpr = expr and
12671251
left = getFullyConvertedLowerBounds(rsExpr.getLeftOperand()) and
12681252
right = getValue(rsExpr.getRightOperand().getFullyConverted()).toInt() and
1269-
result = safeFloor(left / 2.pow(right))
1253+
result = (left / 2.pow(right)).floorFloat()
12701254
)
12711255
// Not explicitly modeled by a SimpleRangeAnalysisExpr
12721256
) and
@@ -1475,7 +1459,7 @@ private float getUpperBoundsImpl(Expr expr) {
14751459
rsExpr = expr and
14761460
left = getFullyConvertedUpperBounds(rsExpr.getLeftOperand()) and
14771461
right = getValue(rsExpr.getRightOperand().getFullyConverted()).toInt() and
1478-
result = safeFloor(left / 2.pow(right))
1462+
result = (left / 2.pow(right)).floorFloat()
14791463
)
14801464
// Not explicitly modeled by a SimpleRangeAnalysisExpr
14811465
) and

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@
351351
| test.c:330:14:330:14 | r | -2147483648 |
352352
| test.c:333:10:333:14 | total | -2147483648 |
353353
| test.c:341:32:341:34 | odd | 9007199254740991 |
354-
| test.c:343:10:343:16 | shifted | 4503599627370495.5 |
354+
| test.c:343:10:343:16 | shifted | 4503599627370495 |
355355
| test.c:348:7:348:7 | x | -2147483648 |
356356
| test.c:352:10:352:10 | i | 0 |
357357
| test.c:353:5:353:5 | i | 0 |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@
351351
| test.c:330:14:330:14 | r | 2147483647 |
352352
| test.c:333:10:333:14 | total | 2147483647 |
353353
| test.c:341:32:341:34 | odd | 9007199254740991 |
354-
| test.c:343:10:343:16 | shifted | 4503599627370495.5 |
354+
| test.c:343:10:343:16 | shifted | 4503599627370495 |
355355
| test.c:348:7:348:7 | x | 2147483647 |
356356
| test.c:352:10:352:10 | i | 7 |
357357
| test.c:353:5:353:5 | i | 2 |

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ int extreme_values(void)
3838
if (x >> 1 >= 0x7FFFFFFFFFFFFFFF) {} // always true [NOT DETECTED]
3939
if (x >> 1 >= 0xFFFFFFFFFFFFFFF) {} // always true [NOT DETECTED]
4040

41-
if (y >> 1 >= 0xFFFFFFFFFFFF) {} // always false [INCORRECT MESSAGE]
42-
if (y >> 1 >= 0x800000000000) {} // always false [INCORRECT MESSAGE]
43-
if (y >> 1 >= 0x7FFFFFFFFFFF) {} // always true [INCORRECT MESSAGE]
44-
if (y >> 1 >= 0xFFFFFFFFFFF) {} // always true [INCORRECT MESSAGE]
41+
if (y >> 1 >= 0xFFFFFFFFFFFF) {} // always false
42+
if (y >> 1 >= 0x800000000000) {} // always false
43+
if (y >> 1 >= 0x7FFFFFFFFFFF) {} // always true
44+
if (y >> 1 >= 0xFFFFFFFFFFF) {} // always true
4545
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@
4545
| PointlessComparison.c:391:12:391:20 | ... < ... | Comparison is always false because ... * ... >= 6. |
4646
| PointlessComparison.c:414:7:414:16 | ... == ... | Comparison is always false because ... * ... >= 18446744073709551616. |
4747
| PointlessComparison.cpp:36:6:36:33 | ... >= ... | Comparison is always false because ... >> ... <= 9223372036854775808. |
48-
| PointlessComparison.cpp:41:6:41:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327.5. |
49-
| PointlessComparison.cpp:42:6:42:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327.5. |
50-
| PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. |
51-
| PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. |
48+
| PointlessComparison.cpp:41:6:41:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327. |
49+
| PointlessComparison.cpp:42:6:42:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327. |
50+
| PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327. |
51+
| PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327. |
5252
| RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. |
5353
| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. |

0 commit comments

Comments
 (0)