diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index feed600fd..dc31f4f28 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -609,88 +609,171 @@ static void valueFlowAfterAssign(TokenList *tokenlist, ErrorLogger *errorLogger, } } +static void execute(const Token *expr, + std::map * const programMemory, + MathLib::bigint *result, + bool *error) +{ + if (!expr) + *error = true; + + else if (expr->isNumber()) + *result = MathLib::toLongNumber(expr->str()); + + else if (expr->varId() > 0) { + const std::map::const_iterator var = programMemory->find(expr->varId()); + if (var == programMemory->end()) + *error = true; + else + *result = var->second; + } + + else if (expr->isComparisonOp()) { + MathLib::bigint result1, result2; + execute(expr->astOperand1(), programMemory, &result1, error); + execute(expr->astOperand2(), programMemory, &result2, error); + if (expr->str() == "<") + *result = result1 < result2; + else if (expr->str() == "<=") + *result = result1 <= result2; + else if (expr->str() == ">") + *result = result1 > result2; + else if (expr->str() == ">=") + *result = result1 >= result2; + else if (expr->str() == "==") + *result = result1 == result2; + else if (expr->str() == "!=") + *result = result1 != result2; + } + + else if (expr->str() == "=") { + execute(expr->astOperand2(), programMemory, result, error); + if (expr->astOperand1()->varId()) + (*programMemory)[expr->astOperand1()->varId()] = *result; + else + *error = true; + } + + else if (expr->str() == "++") { + if (!expr->astOperand1() || expr->astOperand1()->varId() == 0U) + *error = true; + else { + std::map::iterator var = programMemory->find(expr->astOperand1()->varId()); + if (var == programMemory->end()) + *error = true; + else { + *result = var->second + 1; + var->second = *result; + } + } + } + + else if (expr->str() == "&&") { + execute(expr->astOperand1(), programMemory, result, error); + if (*error || *result == 0) + *result = 0; + else { + execute(expr->astOperand2(), programMemory, result, error); + // If there is an error, assume the result is not important + if (*error) { + *error = false; + *result = 1; + } + } + } + + else if (expr->str() == "||") { + execute(expr->astOperand1(), programMemory, result, error); + if (*result == 0 && *error == false) + execute(expr->astOperand2(), programMemory, result, error); + } + + else + *error = true; +} + + static void valueFlowForLoop(TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { - if (!Token::simpleMatch(tok, "for (")) + if (!Token::simpleMatch(tok, "for (") || + !Token::simpleMatch(tok->next()->astOperand2(), ";") || + !Token::simpleMatch(tok->next()->astOperand2()->astOperand2(), ";")) continue; - tok = tok->tokAt(2); - if (!Token::Match(tok,"%type%| %var% = %num% ;")) { // TODO: don't use %num% - if (settings->debugwarnings) - bailout(tokenlist, errorLogger, tok, "For loop not handled"); + const Token *firstExpression = tok->next()->astOperand2()->astOperand1(); + const Token *secondExpression = tok->next()->astOperand2()->astOperand2()->astOperand1(); + const Token *thirdExpression = tok->next()->astOperand2()->astOperand2()->astOperand2(); + tok = tok->linkAt(1); + + std::map programMemory; + MathLib::bigint result; + bool error = false; + execute(firstExpression, &programMemory, &result, &error); + if (error) continue; - } - Token * const vartok = tok->tokAt(Token::Match(tok, "%var% =") ? 0 : 1); - const MathLib::bigint num1 = MathLib::toLongNumber(vartok->strAt(2)); - if (vartok->varId() == 0U) - continue; - tok = vartok->tokAt(4); - const Token *num2tok = nullptr; - if (Token::Match(tok, "%varid% <|<=|!=", vartok->varId())) { - tok = tok->next(); - num2tok = tok->astOperand2(); - if (num2tok && num2tok->str() == "(" && !num2tok->astOperand2()) - num2tok = num2tok->astOperand1(); - if (!Token::Match(num2tok, "%num%")) - num2tok = 0; - } - if (!num2tok) { - if (settings->debugwarnings) - bailout(tokenlist, errorLogger, tok, "For loop not handled"); - continue; - } - const MathLib::bigint num2 = MathLib::toLongNumber(num2tok ? num2tok->str() : "0") - ((tok->str()=="<=") ? 0 : 1); - while (tok && tok->str() != ";") - tok = tok->next(); - if (!num2tok || !Token::Match(tok, "; %varid% ++ ) {", vartok->varId())) { - if (settings->debugwarnings) - bailout(tokenlist, errorLogger, tok, "For loop not handled"); + execute(secondExpression, &programMemory, &result, &error); + if (error) continue; + const std::map startMemory(programMemory); + std::map endMemory; + + unsigned int maxcount = 10000; + while (result != 0 && !error && --maxcount) { + endMemory = programMemory; + execute(thirdExpression, &programMemory, &result, &error); + if (!error) + execute(secondExpression, &programMemory, &result, &error); } - Token * const bodyStart = tok->tokAt(4); - const Token * const bodyEnd = bodyStart->link(); + Token * const bodyStart = tok->next(); + const Token * const bodyEnd = bodyStart->link(); - // Is variable modified inside for loop - bool modified = false; - for (const Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { - if (Token::Match(tok2, "%varid% =", vartok->varId())) { - modified = true; - break; - } - } - if (modified) - continue; + for (std::map::const_iterator it = startMemory.begin(); it != startMemory.end(); ++it) { + const unsigned int varid = it->first; + const MathLib::bigint num1 = it->second; + const MathLib::bigint num2 = endMemory[varid]; - for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { - if (tok2->varId() == vartok->varId()) { - const Token * parent = tok2->astParent(); - while (parent) { - const Token * const p = parent; - parent = parent->astParent(); - if (parent && parent->str() == "?" && parent->astOperand2() == p) - break; + // Is variable modified inside for loop + bool modified = false; + for (const Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { + if (Token::Match(tok2, "%varid% =", varid)) { + modified = true; + break; } - if (parent) { + } + if (modified) + continue; + + for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { + if (tok2->varId() == varid) { + const Token * parent = tok2->astParent(); + while (parent) { + const Token * const p = parent; + parent = parent->astParent(); + if (parent && parent->str() == "?" && parent->astOperand2() == p) + break; + } + if (parent) { + if (settings->debugwarnings) + bailout(tokenlist, errorLogger, tok2, "For loop variable " + tok2->str() + " stopping on ?"); + continue; + } + + ValueFlow::Value value1(num1); + value1.varId = tok2->varId(); + setTokenValue(tok2, value1); + + ValueFlow::Value value2(num2); + value2.varId = tok2->varId(); + setTokenValue(tok2, value2); + } + + if (tok2->str() == "{") { if (settings->debugwarnings) - bailout(tokenlist, errorLogger, tok2, "For loop variable " + vartok->str() + " stopping on ?"); - continue; + bailout(tokenlist, errorLogger, tok2, "For loop variable " + tok2->str() + " stopping on {"); + break; } - - ValueFlow::Value value1(num1); - value1.varId = tok2->varId(); - setTokenValue(tok2, value1); - - ValueFlow::Value value2(num2); - value2.varId = tok2->varId(); - setTokenValue(tok2, value2); - } - - if (tok2->str() == "{") { - if (settings->debugwarnings) - bailout(tokenlist, errorLogger, tok2, "For loop variable " + vartok->str() + " stopping on {"); - break; } } } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 8882e1fad..57a2c34c5 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -130,7 +130,6 @@ private: TEST_CASE(array_index_negative2); // ticket #3063 TEST_CASE(array_index_for_decr); TEST_CASE(array_index_varnames); // FP: struct member. #1576 - TEST_CASE(array_index_for_break); // FP: for,break TEST_CASE(array_index_for_continue); // for,continue TEST_CASE(array_index_for); // FN: for,if TEST_CASE(array_index_for_neq); // #2211: Using != in condition @@ -458,7 +457,8 @@ private: " a[i] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n" + "[test.cpp:3]: (error) Array 'a[10]' accessed at index 49, which is out of bounds.\n", errout.str()); } } @@ -1045,7 +1045,8 @@ private: " for (int i = 3; 0 <= i; i--)\n" " a[i] = i;\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: a\n" + "[test.cpp:5]: (error) Array 'a[3]' accessed at index 3, which is out of bounds.\n", errout.str()); check("void f()\n" "{\n" @@ -1766,7 +1767,8 @@ private: " data[i] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: data\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: data\n" + "[test.cpp:5]: (error) Array 'data[8]' accessed at index 10, which is out of bounds.\n", errout.str()); check("void f()\n" "{\n" @@ -1844,19 +1846,6 @@ private: ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: data\n", errout.str()); } - void array_index_for_break() { - check("void f() {\n" - " int a[2];\n" - " for (int i = 0; i <= 2; ++i) {\n" - " a[i] = 0;\n" - " if (i==1) {\n" - " break;\n" - " }\n" - " }\n" - "}"); - ASSERT_EQUALS("", errout.str()); - } - void array_index_for_continue() { // #3913 check("void f() {\n" @@ -1924,7 +1913,9 @@ private: " a[i] = 0;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: a\n" + "[test.cpp:4]: (error) Array 'a[5]' accessed at index 9, which is out of bounds.\n", + errout.str()); } void array_index_for_question() { @@ -2274,7 +2265,8 @@ private: " for (i = 0; i <= 10; ++i)\n" " a[i] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Buffer is accessed out of bounds: a\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (error) Buffer is accessed out of bounds: a\n" + "[test.cpp:7]: (error) Array 'a[10]' accessed at index 10, which is out of bounds.\n", errout.str()); } @@ -2285,7 +2277,8 @@ private: " for (int i = 0; i < 8; ++i)\n" " p[i] = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: p\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: p\n" + "[test.cpp:5]: (error) Array 'p[2]' accessed at index 7, which is out of bounds.\n", errout.str()); // No false positive check("void foo(int x, int y)\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index db65c38e8..2f15a228f 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -617,14 +617,6 @@ private: ASSERT_EQUALS(true, testValueOfX(code, 3U, 9)); ASSERT_EQUALS(false, testValueOfX(code, 3U, 10)); - code = "void f() {\n" - " for (int x = 0; x < ((short)10); x++)\n" - " a[x] = 0;\n" - "}"; - ASSERT_EQUALS(true, testValueOfX(code, 3U, 0)); - ASSERT_EQUALS(true, testValueOfX(code, 3U, 9)); - ASSERT_EQUALS(false, testValueOfX(code, 3U, 10)); - code = "void f() {\n" " for (int x = 0; x < 10; x++)\n" " x<4 ?\n"