Skip to content

Commit 8c2c81d

Browse files
authored
Fix some false positive in loop forward analysis (#2669)
* Fix some false positive in loop forward analysis In cases like: ``` bool b(); void f() { int val[50]; int i, sum=0; for (i = 1; b() && i < 50; i++) sum += val[i]; for (; i < 50; i++) sum -= val[i]; } ``` The forward analysis assumed the second loop was entered, and we ended up with false positive in it: `Array 'val[50]' accessed at index 50, which is out of bounds` * Fix style
1 parent 92f95c8 commit 8c2c81d

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

lib/forwardanalyzer.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,17 @@ struct ForwardTraversal {
220220
return Progress::Break;
221221
}
222222
// Traverse condition after lowering
223-
if (condTok && updateRecursive(condTok) == Progress::Break)
224-
return Progress::Break;
223+
if (condTok) {
224+
if (updateRecursive(condTok) == Progress::Break)
225+
return Progress::Break;
226+
227+
bool checkThen, checkElse;
228+
std::tie(checkThen, checkElse) = evalCond(condTok);
229+
if (checkElse)
230+
// condition is false, we don't enter the loop
231+
return Progress::Break;
232+
}
233+
225234
forkScope(endBlock, allAnalysis.isModified());
226235
if (bodyAnalysis.isModified()) {
227236
Token* writeTok = findRange(endBlock->link(), endBlock, std::mem_fn(&ForwardAnalyzer::Action::isModified));

test/testbufferoverrun.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ class TestBufferOverrun : public TestFixture {
156156
TEST_CASE(array_index_function_parameter);
157157
TEST_CASE(array_index_enum_array); // #8439
158158
TEST_CASE(array_index_container); // #9386
159+
TEST_CASE(array_index_two_for_loops);
159160

160161
TEST_CASE(buffer_overrun_2_struct);
161162
TEST_CASE(buffer_overrun_3);
@@ -2211,6 +2212,46 @@ class TestBufferOverrun : public TestFixture {
22112212
ASSERT_EQUALS("", errout.str());
22122213
}
22132214

2215+
void array_index_two_for_loops() {
2216+
check("bool b();\n"
2217+
"void f()\n"
2218+
"{\n"
2219+
" int val[50];\n"
2220+
" int i, sum=0;\n"
2221+
" for (i = 1; b() && i < 50; i++)\n"
2222+
" sum += val[i];\n"
2223+
" if (i < 50)\n"
2224+
" sum -= val[i];\n"
2225+
"}");
2226+
ASSERT_EQUALS("", errout.str());
2227+
2228+
check("bool b();\n"
2229+
"void f()\n"
2230+
"{\n"
2231+
" int val[50];\n"
2232+
" int i, sum=0;\n"
2233+
" for (i = 1; b() && i < 50; i++)\n"
2234+
" sum += val[i];\n"
2235+
" for (; i < 50;) {\n"
2236+
" sum -= val[i];\n"
2237+
" break;\n"
2238+
" }\n"
2239+
"}");
2240+
ASSERT_EQUALS("", errout.str());
2241+
2242+
check("bool b();\n"
2243+
"void f()\n"
2244+
"{\n"
2245+
" int val[50];\n"
2246+
" int i, sum=0;\n"
2247+
" for (i = 1; b() && i < 50; i++)\n"
2248+
" sum += val[i];\n"
2249+
" for (; i < 50; i++)\n"
2250+
" sum -= val[i];\n"
2251+
"}");
2252+
ASSERT_EQUALS("", errout.str());
2253+
}
2254+
22142255
void buffer_overrun_2_struct() {
22152256
check("struct ABC\n"
22162257
"{\n"

0 commit comments

Comments
 (0)