Skip to content

Commit a4fa4c8

Browse files
committed
C++: Fix rounding for >>.
1 parent b1c32de commit a4fa4c8

File tree

3 files changed

+23
-8
lines changed

3 files changed

+23
-8
lines changed

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,22 @@ private class UnsignedBitwiseAndExpr extends BitwiseAndExpr {
140140
}
141141
}
142142

143+
/**
144+
* Gets the floor of `v`, with additional logic to work around issues with
145+
* large numbers.
146+
*/
147+
bindingset[v]
148+
float safeFloor(float v) {
149+
// return the floor of v
150+
v.abs() < 2.pow(31) and
151+
result = v.floor()
152+
or
153+
// `floor()` doesn't work correctly on large numbers (since it returns an integer),
154+
// so fall back to unrounded numbers at this scale.
155+
not v.abs() < 2.pow(31) and
156+
result = v
157+
}
158+
143159
/** Set of expressions which we know how to analyze. */
144160
private predicate analyzableExpr(Expr e) {
145161
// The type of the expression must be arithmetic. We reuse the logic in
@@ -709,7 +725,7 @@ private float getLowerBoundsImpl(Expr expr) {
709725
rsExpr = expr and
710726
left = getFullyConvertedLowerBounds(rsExpr.getLeftOperand()) and
711727
right = rsExpr.getRightOperand().getValue().toInt() and
712-
result = left / 2.pow(right)
728+
result = safeFloor(left / 2.pow(right))
713729
)
714730
}
715731

@@ -878,7 +894,7 @@ private float getUpperBoundsImpl(Expr expr) {
878894
rsExpr = expr and
879895
left = getFullyConvertedUpperBounds(rsExpr.getLeftOperand()) and
880896
right = rsExpr.getRightOperand().getValue().toInt() and
881-
result = left / 2.pow(right)
897+
result = safeFloor(left / 2.pow(right))
882898
)
883899
}
884900

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,9 @@ int shifts(void)
369369
{
370370
unsigned int x = 3;
371371

372-
if (x >> 1 >= 1) {} // always true [BAD MESSAGE]
373-
if (x >> 1 >= 2) {} // always false [BAD MESSAGE]
374-
if (x >> 1 == 1) {} // always true [INCORRECT MESSAGE]
372+
if (x >> 1 >= 1) {} // always true
373+
if (x >> 1 >= 2) {} // always false
374+
if (x >> 1 == 1) {} // always true [NOT DETECTED]
375375
}
376376

377377
int bitwise_ands()

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@
3838
| PointlessComparison.c:303:9:303:14 | ... >= ... | Comparison is always false because c <= 0. |
3939
| PointlessComparison.c:312:9:312:14 | ... >= ... | Comparison is always false because c <= 0. |
4040
| PointlessComparison.c:337:14:337:21 | ... >= ... | Comparison is always true because x >= 0. |
41-
| PointlessComparison.c:372:6:372:16 | ... >= ... | Comparison is always true because ... >> ... >= 1.5. |
42-
| PointlessComparison.c:373:6:373:16 | ... >= ... | Comparison is always false because ... >> ... <= 1.5. |
43-
| PointlessComparison.c:374:6:374:16 | ... == ... | Comparison is always false because ... >> ... >= 1.5. |
41+
| PointlessComparison.c:372:6:372:16 | ... >= ... | Comparison is always true because ... >> ... >= 1. |
42+
| PointlessComparison.c:373:6:373:16 | ... >= ... | Comparison is always false because ... >> ... <= 1. |
4443
| PointlessComparison.c:383:6:383:17 | ... >= ... | Comparison is always false because ... & ... <= 2. |
4544
| PointlessComparison.cpp:36:6:36:33 | ... >= ... | Comparison is always false because ... >> ... <= 9223372036854776000. |
4645
| PointlessComparison.cpp:41:6:41:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327.5. |

0 commit comments

Comments
 (0)