Skip to content

Commit ad11f76

Browse files
committed
C++: Always normalize bounds after a computation
This stops some cases of `-0.0` from propagating through the range analysis, fixing a false positive on arvidn/libtorrent. There seems to be no need for a corresponding change in the caller of `getDefLowerBoundsImpl` since that predicate only contains computations that cannot introduce negative zero.
1 parent 0c8e06b commit ad11f76

File tree

5 files changed

+8
-11
lines changed

5 files changed

+8
-11
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ private float getTruncatedLowerBounds(Expr expr) {
570570
else (
571571
// Some of the bounds computed by getLowerBoundsImpl might
572572
// overflow, so we replace invalid bounds with exprMinVal.
573-
exists(float newLB | newLB = getLowerBoundsImpl(expr) |
573+
exists(float newLB | newLB = normalizeFloatUp(getLowerBoundsImpl(expr)) |
574574
if exprMinVal(expr) <= newLB and newLB <= exprMaxVal(expr)
575575
then result = newLB
576576
else result = exprMinVal(expr)
@@ -617,7 +617,7 @@ private float getTruncatedUpperBounds(Expr expr) {
617617
// Some of the bounds computed by `getUpperBoundsImpl`
618618
// might overflow, so we replace invalid bounds with
619619
// `exprMaxVal`.
620-
exists(float newUB | newUB = getUpperBoundsImpl(expr) |
620+
exists(float newUB | newUB = normalizeFloatUp(getUpperBoundsImpl(expr)) |
621621
if exprMinVal(expr) <= newUB and newUB <= exprMaxVal(expr)
622622
then result = newUB
623623
else result = exprMaxVal(expr)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@
136136
| test.c:183:14:183:14 | a | -7 |
137137
| test.c:184:5:184:9 | total | -45 |
138138
| test.c:184:14:184:14 | b | -7 |
139-
| test.c:184:16:184:16 | c | -0 |
139+
| test.c:184:16:184:16 | c | 0 |
140140
| test.c:186:13:186:13 | a | -2147483648 |
141141
| test.c:186:18:186:18 | a | -7 |
142142
| test.c:187:14:187:14 | a | -7 |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@
115115
| test.c:168:14:168:14 | a | 11 |
116116
| test.c:169:5:169:9 | total | 8 |
117117
| test.c:169:14:169:14 | b | 11 |
118-
| test.c:169:16:169:16 | c | -0 |
118+
| test.c:169:16:169:16 | c | 0 |
119119
| test.c:171:13:171:13 | a | 2147483647 |
120120
| test.c:171:18:171:18 | a | 2147483647 |
121121
| test.c:172:14:172:14 | a | 11 |

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
| PointlessComparison.c:126:12:126:18 | ... >= ... | Comparison is always true because a >= 20. |
3232
| PointlessComparison.c:129:12:129:16 | ... > ... | Comparison is always false because a <= 3. |
3333
| PointlessComparison.c:197:7:197:11 | ... < ... | Comparison is always false because x >= 0. |
34-
| PointlessComparison.c:264:12:264:22 | ... >= ... | Comparison is always true because dbl >= 0 and -0 >= - .... |
34+
| PointlessComparison.c:264:12:264:22 | ... >= ... | Comparison is always true because dbl >= 0 and 0 >= - .... |
3535
| PointlessComparison.c:273:9:273:18 | ... > ... | Comparison is always false because c <= 0. |
3636
| PointlessComparison.c:283:13:283:19 | ... >= ... | Comparison is always true because c >= 11. |
3737
| PointlessComparison.c:294:9:294:16 | ... >= ... | Comparison is always false because ui1 <= 0. |
@@ -50,7 +50,4 @@
5050
| PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. |
5151
| PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. |
5252
| RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. |
53-
| RegressionTests.cpp:89:7:89:14 | ... == ... | Comparison is always false because val <= -0. |
54-
| RegressionTests.cpp:107:7:107:14 | ... == ... | Comparison is always false because val <= -0. |
55-
| RegressionTests.cpp:116:7:116:14 | ... == ... | Comparison is always false because val <= -0. |
5653
| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. |

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void negativeZero1(int val) {
8686
{
8787
val = -val;
8888
}
89-
if (val == 0) // GOOD [FALSE POSITIVE]
89+
if (val == 0) // GOOD [NO LONGER REPORTED]
9090
;
9191
}
9292

@@ -104,7 +104,7 @@ void negativeZero3(int val) {
104104
{
105105
val *= -1;
106106
}
107-
if (val == 0) // GOOD [FALSE POSITIVE]
107+
if (val == 0) // GOOD [NO LONGER REPORTED]
108108
;
109109
}
110110

@@ -113,6 +113,6 @@ void negativeZero4(int val) {
113113
{
114114
val = val * -1;
115115
}
116-
if (val == 0) // GOOD [FALSE POSITIVE]
116+
if (val == 0) // GOOD [NO LONGER REPORTED]
117117
;
118118
}

0 commit comments

Comments
 (0)