Skip to content

Commit b3f3f6d

Browse files
committed
C++: Fix edge case.
1 parent cbf30e3 commit b3f3f6d

File tree

4 files changed

+13
-9
lines changed

4 files changed

+13
-9
lines changed

cpp/ql/src/Likely Bugs/Likely Typos/inconsistentLoopDirection.ql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,11 @@ predicate illDefinedDecrForStmt(
5454
or
5555
(forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue())
5656
)
57-
) and
58-
// exclude cases where the loop counter is `unsigned` (where wrapping behaviour can be used deliberately)
59-
not v.getUnspecifiedType().(IntegralType).isUnsigned()
57+
) and (
58+
// exclude cases where the loop counter is `unsigned` (where wrapping behaviour can be used deliberately)
59+
v.getUnspecifiedType().(IntegralType).isSigned() or
60+
initialCondition.getValue().toInt() = 0
61+
)
6062
}
6163

6264
pragma[noinline]

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/inconsistentLoopDirection/inconsistentLoopDirection.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ void Unsigned()
2424
{
2525
unsigned long i;
2626

27-
for (i = 0; i < 100; i--) //BUG [NOT DETECTED]
27+
for (i = 0; i < 100; i--) //BUG
2828
{
2929
}
3030

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/inconsistentLoopDirection/inconsistentLoopDirection.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void InvalidConditionUnsignedCornerCase()
130130
unsigned char min = 0;
131131
unsigned char max = 100;
132132

133-
for (i = 100; i < 0; i--) //BUG
133+
for (i = 100; i < 0; i--) //BUG [NOT DETECTED]
134134
{
135135
}
136136

@@ -185,13 +185,13 @@ void IntendedOverflow()
185185
unsigned char i;
186186
signed char s;
187187

188-
for (i = 63; i < 64; i--) {} // GOOD (legitimate way to count down with an unsigned) [FALSE POSITIVE]
188+
for (i = 63; i < 64; i--) {} // GOOD (legitimate way to count down with an unsigned)
189189
for (i = 63; i < 128; i--) {} // DUBIOUS (could still be a typo?)
190-
for (i = 63; i < 255; i--) {} // GOOD [FALSE POSITIVE]
190+
for (i = 63; i < 255; i--) {} // GOOD
191191

192-
for (i = m - 1; i < m; i--) {} // GOOD [FALSE POSITIVE]
192+
for (i = m - 1; i < m; i--) {} // GOOD
193193
for (i = m - 1; i < m; i--) {} // DUBIOUS
194-
for (i = m - 1; i < m; i--) {} // GOOD [FALSE POSITIVE]
194+
for (i = m - 1; i < m; i--) {} // GOOD
195195

196196
for (s = 63; s < 64; s--) {} // BAD (signed numbers don't wrap at 0 / at all)
197197
}

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/inconsistentLoopDirection/inconsistentLoopDirection.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
| inconsistentLoopDirection.c:5:5:7:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
22
| inconsistentLoopDirection.c:13:5:15:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
3+
| inconsistentLoopDirection.c:27:5:29:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
34
| inconsistentLoopDirection.c:35:5:37:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
45
| inconsistentLoopDirection.c:48:5:50:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
56
| inconsistentLoopDirection.c:58:5:60:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
67
| inconsistentLoopDirection.cpp:5:5:7:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
78
| inconsistentLoopDirection.cpp:13:5:15:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
9+
| inconsistentLoopDirection.cpp:27:5:29:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
810
| inconsistentLoopDirection.cpp:35:5:37:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
911
| inconsistentLoopDirection.cpp:46:5:48:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
1012
| inconsistentLoopDirection.cpp:54:5:56:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |

0 commit comments

Comments
 (0)