Skip to content

Commit

Permalink
Fix #13109 FP arrayIndexOutOfBounds with infinite loop and break in b…
Browse files Browse the repository at this point in the history 
…ody (#6986)
  • Loading branch information
@chrchr-github
chrchr-github authored Dec 4, 2024
1 parent 0cbf73c commit f7d0c25
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 10 deletions.
35 changes: 35 additions & 0 deletions lib/astutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2833,6 +2833,41 @@ const Token* findExpression(const Token* start, const nonneg int exprid)
return nullptr;
}

const Token* findEscapeStatement(const Scope* scope, const Library* library)
{
if (!scope)
return nullptr;
for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
const Scope* escapeScope = tok->scope();
if (!escapeScope->isExecutable()) { // skip type definitions
tok = escapeScope->bodyEnd;
continue;
}
if (const Token* lambdaEnd = findLambdaEndToken(tok)) { // skip lambdas
tok = lambdaEnd;
continue;
}
if (!tok->isName())
continue;
if (isEscapeFunction(tok, library))
return tok;
if (!tok->isKeyword())
continue;
if (Token::Match(tok, "goto|return|throw")) // TODO: check try/catch, labels?
return tok;
if (!Token::Match(tok, "break|continue"))
continue;
const bool isBreak = tok->str()[0] == 'b';
while (escapeScope && escapeScope != scope) {
if (escapeScope->isLoopScope() || (isBreak && escapeScope->type == Scope::ScopeType::eSwitch))
return nullptr;
escapeScope = escapeScope->nestedIn;
}
return tok;
}
return nullptr;
}

// Thread-unsafe memoization
template<class F, class R=decltype(std::declval<F>()())>
static std::function<R()> memoize(F f)
Expand Down
3 changes: 3 additions & 0 deletions lib/astutils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ const Token* findExpression(nonneg int exprid,
const std::function<bool(const Token*)>& pred);
const Token* findExpression(const Token* start, nonneg int exprid);

/** Does code execution escape from the given scope? */
const Token* findEscapeStatement(const Scope* scope, const Library* library);

std::vector<const Token*> astFlatten(const Token* tok, const char* op);
std::vector<Token*> astFlatten(Token* tok, const char* op);

Expand Down
16 changes: 6 additions & 10 deletions lib/valueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5128,6 +5128,12 @@ static void valueFlowForLoopSimplify(Token* const bodyStart,
if (isVariableChanged(bodyStart, bodyEnd, expr->varId(), globalvar, settings))
return;

if (const Token* escape = findEscapeStatement(bodyStart->scope(), &settings.library)) {
if (settings.debugwarnings)
bailout(tokenlist, errorLogger, escape, "For loop variable bailout on escape statement");
return;
}

for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) {
if (tok2->varId() == expr->varId()) {
const Token * parent = tok2->astParent();
Expand Down Expand Up @@ -5189,20 +5195,10 @@ static void valueFlowForLoopSimplify(Token* const bodyStart,

if (Token::simpleMatch(tok2, ") {")) {
if (vartok->varId() && Token::findmatch(tok2->link(), "%varid%", tok2, vartok->varId())) {
if (Token::findmatch(tok2, "continue|break|return", tok2->linkAt(1), vartok->varId())) {
if (settings.debugwarnings)
bailout(tokenlist, errorLogger, tok2, "For loop variable bailout on conditional continue|break|return");
break;
}
if (settings.debugwarnings)
bailout(tokenlist, errorLogger, tok2, "For loop variable skipping conditional scope");
tok2 = tok2->linkAt(1);
if (Token::simpleMatch(tok2, "} else {")) {
if (Token::findmatch(tok2, "continue|break|return", tok2->linkAt(2), vartok->varId())) {
if (settings.debugwarnings)
bailout(tokenlist, errorLogger, tok2, "For loop variable bailout on conditional continue|break|return");
break;
}
tok2 = tok2->linkAt(2);
}
}
Expand Down
13 changes: 13 additions & 0 deletions test/testbufferoverrun.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2579,6 +2579,19 @@ class TestBufferOverrun : public TestFixture {
" return buf[0];\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Array 'buf[10]' accessed at index 10, which is out of bounds.\n", errout_str());

check("void f() {\n"
" const int a[10] = {};\n"
" for (int n = 0; 1; ++n) {\n"
" if (a[n] < 1) {\n"
" switch (a[n]) {\n"
" case 0:\n"
" break;\n"
" }\n"
" }\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index 9998, which is out of bounds.\n", errout_str());
}

void array_index_for_neq() {
Expand Down
80 changes: 80 additions & 0 deletions test/testvalueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4584,6 +4584,86 @@ class TestValueFlow : public TestFixture {
ASSERT_EQUALS(true, values.empty());
values = tokenValues(code, "[ f . b");
ASSERT_EQUALS(true, values.empty());

code = "void f() {\n" // #13109
" const int a[10] = {};\n"
" for (int n = 0; 1; ++n) {\n"
" (void)a[n];\n"
" break;\n"
" }\n"
"}\n";
values = tokenValues(code, "n ]");
ASSERT_EQUALS(2, values.size());
auto it = values.begin();
ASSERT_EQUALS(-1, it->intvalue);
ASSERT(it->isImpossible());
++it;
ASSERT_EQUALS(0, it->intvalue);
ASSERT(it->isPossible());

code = "void f() {\n"
" const int a[10] = {};\n"
" for (int n = 0; 1; ++n) {\n"
" if (a[n] < 1)\n"
" break;\n"
" }\n"
"}\n";
values = tokenValues(code, "n ]");
ASSERT_EQUALS(2, values.size());
it = values.begin();
ASSERT_EQUALS(-1, it->intvalue);
ASSERT(it->isImpossible());
++it;
ASSERT_EQUALS(0, it->intvalue);
ASSERT(it->isPossible());

code = "void f() {\n"
" const int a[10] = {};\n"
" for (int n = 0; 1; ++n) {\n"
" if (a[n] < 1)\n"
" throw 0;\n"
" }\n"
"}\n";
values = tokenValues(code, "n ]");
ASSERT_EQUALS(2, values.size());
it = values.begin();
ASSERT_EQUALS(-1, it->intvalue);
ASSERT(it->isImpossible());
++it;
ASSERT_EQUALS(0, it->intvalue);
ASSERT(it->isPossible());

code = "void f() {\n"
" const int a[10] = {};\n"
" for (int n = 0; 1; ++n) {\n"
" (void)a[n];\n"
" exit(1);\n"
" }\n"
"}\n";
values = tokenValues(code, "n ]");
ASSERT_EQUALS(2, values.size());
it = values.begin();
ASSERT_EQUALS(-1, it->intvalue);
ASSERT(it->isImpossible());
++it;
ASSERT_EQUALS(0, it->intvalue);
ASSERT(it->isPossible());

code = "void f() {\n"
" const int a[10] = {};\n"
" for (int n = 0; 1; ++n) {\n"
" if (a[n] < 1)\n"
" exit(1);\n"
" }\n"
"}\n";
values = tokenValues(code, "n ]");
ASSERT_EQUALS(2, values.size());
it = values.begin();
ASSERT_EQUALS(-1, it->intvalue);
ASSERT(it->isImpossible());
++it;
ASSERT_EQUALS(0, it->intvalue);
ASSERT(it->isPossible());
}

void valueFlowSubFunction() {
Expand Down

0 comments on commit f7d0c25

Please sign in to comment.