From f5ddb1d51d18eef4ffaeca1b5e6106bc39aecd95 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Wed, 7 Jan 2026 11:00:18 +0100 Subject: [PATCH] C++: Remove `safeFloor` in simple range analysis --- .../cpp/rangeanalysis/SimpleRangeAnalysis.qll | 20 ++----------------- .../SimpleRangeAnalysis/lowerBound.expected | 2 +- .../SimpleRangeAnalysis/upperBound.expected | 2 +- .../PointlessComparison.cpp | 8 ++++---- .../PointlessComparison.expected | 8 ++++---- 5 files changed, 12 insertions(+), 28 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll index 722866c512f..cc4647b54e0 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll @@ -158,22 +158,6 @@ private class UnsignedBitwiseAndExpr extends BitwiseAndExpr { } } -/** - * Gets the floor of `v`, with additional logic to work around issues with - * large numbers. - */ -bindingset[v] -float safeFloor(float v) { - // return the floor of v - v.abs() < 2.pow(31) and - result = v.floor() - or - // `floor()` doesn't work correctly on large numbers (since it returns an integer), - // so fall back to unrounded numbers at this scale. - not v.abs() < 2.pow(31) and - result = v -} - /** A `MulExpr` where exactly one operand is constant. */ private class MulByConstantExpr extends MulExpr { float constant; @@ -1266,7 +1250,7 @@ private float getLowerBoundsImpl(Expr expr) { rsExpr = expr and left = getFullyConvertedLowerBounds(rsExpr.getLeftOperand()) and right = getValue(rsExpr.getRightOperand().getFullyConverted()).toInt() and - result = safeFloor(left / 2.pow(right)) + result = (left / 2.pow(right)).floorFloat() ) // Not explicitly modeled by a SimpleRangeAnalysisExpr ) and @@ -1475,7 +1459,7 @@ private float getUpperBoundsImpl(Expr expr) { rsExpr = expr and left = getFullyConvertedUpperBounds(rsExpr.getLeftOperand()) and right = getValue(rsExpr.getRightOperand().getFullyConverted()).toInt() and - result = safeFloor(left / 2.pow(right)) + result = (left / 2.pow(right)).floorFloat() ) // Not explicitly modeled by a SimpleRangeAnalysisExpr ) and diff --git a/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected b/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected index 4c5ceeb9c55..eb1dbc7b93f 100644 --- a/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected +++ b/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected @@ -351,7 +351,7 @@ | test.c:330:14:330:14 | r | -2147483648 | | test.c:333:10:333:14 | total | -2147483648 | | test.c:341:32:341:34 | odd | 9007199254740991 | -| test.c:343:10:343:16 | shifted | 4503599627370495.5 | +| test.c:343:10:343:16 | shifted | 4503599627370495 | | test.c:348:7:348:7 | x | -2147483648 | | test.c:352:10:352:10 | i | 0 | | test.c:353:5:353:5 | i | 0 | diff --git a/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected b/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected index cb182416e38..b2591fb52f3 100644 --- a/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected +++ b/cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected @@ -351,7 +351,7 @@ | test.c:330:14:330:14 | r | 2147483647 | | test.c:333:10:333:14 | total | 2147483647 | | test.c:341:32:341:34 | odd | 9007199254740991 | -| test.c:343:10:343:16 | shifted | 4503599627370495.5 | +| test.c:343:10:343:16 | shifted | 4503599627370495 | | test.c:348:7:348:7 | x | 2147483647 | | test.c:352:10:352:10 | i | 7 | | test.c:353:5:353:5 | i | 2 | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.cpp b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.cpp index 7b67f77ad44..ce04ddcf081 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.cpp @@ -38,8 +38,8 @@ int extreme_values(void) if (x >> 1 >= 0x7FFFFFFFFFFFFFFF) {} // always true [NOT DETECTED] if (x >> 1 >= 0xFFFFFFFFFFFFFFF) {} // always true [NOT DETECTED] - if (y >> 1 >= 0xFFFFFFFFFFFF) {} // always false [INCORRECT MESSAGE] - if (y >> 1 >= 0x800000000000) {} // always false [INCORRECT MESSAGE] - if (y >> 1 >= 0x7FFFFFFFFFFF) {} // always true [INCORRECT MESSAGE] - if (y >> 1 >= 0xFFFFFFFFFFF) {} // always true [INCORRECT MESSAGE] + if (y >> 1 >= 0xFFFFFFFFFFFF) {} // always false + if (y >> 1 >= 0x800000000000) {} // always false + if (y >> 1 >= 0x7FFFFFFFFFFF) {} // always true + if (y >> 1 >= 0xFFFFFFFFFFF) {} // always true } diff --git a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected index 6c273b985ee..d00c38fda28 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected @@ -45,9 +45,9 @@ | PointlessComparison.c:391:12:391:20 | ... < ... | Comparison is always false because ... * ... >= 6. | | PointlessComparison.c:414:7:414:16 | ... == ... | Comparison is always false because ... * ... >= 18446744073709551616. | | PointlessComparison.cpp:36:6:36:33 | ... >= ... | Comparison is always false because ... >> ... <= 9223372036854775808. | -| PointlessComparison.cpp:41:6:41:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327.5. | -| PointlessComparison.cpp:42:6:42:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327.5. | -| PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. | -| PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. | +| PointlessComparison.cpp:41:6:41:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327. | +| PointlessComparison.cpp:42:6:42:29 | ... >= ... | Comparison is always false because ... >> ... <= 140737488355327. | +| PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327. | +| PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327. | | RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. | | Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. |