From d6be4559cdffebfcaf24b02807f5c348fa829aac Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Tue, 25 Jun 2013 06:37:51 +0200 Subject: [PATCH] Fixed #4840 (false negative: buffer access out of bounds) --- lib/checkbufferoverrun.cpp | 8 +++++++- lib/tokenize.cpp | 20 +++++++++++++------- test/testbufferoverrun.cpp | 31 +++++++++++++++++++++++++++++++ test/testtokenize.cpp | 10 ++++++++++ 4 files changed, 61 insertions(+), 8 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 0b3a17fb7..1cbb5609c 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -319,6 +319,11 @@ static bool for_condition(const Token *tok2, unsigned int varid, std::string &mi maxMinFlipped = true; max_value = min_value; min_value = tok2->str(); + } else if (Token::Match(tok2, "%varid% -- ; )", varid) || + Token::Match(tok2, "-- %varid% ; )", varid)) { + maxMinFlipped = true; + max_value = min_value; + min_value = (tok2->str() == "--") ? "1" : "0"; } else { // parse condition while (tok2 && tok2->str() != ";") { @@ -819,7 +824,8 @@ void CheckBufferOverrun::checkScopeForBody(const Token *tok, const ArrayInfo &ar } if (!tok2 || tok2->str() != ";") return; - if (!for3(tok2->next(), counter_varid, min_counter_value, max_counter_value, maxMinFlipped)) + const bool hasFor3 = tok2->next()->str() != ")"; + if (hasFor3 && !for3(tok2->next(), counter_varid, min_counter_value, max_counter_value, maxMinFlipped)) return; if (Token::Match(tok2->next(), "%var% =") && MathLib::toLongNumber(max_counter_value) < size) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 62fff2ce7..a7e7b7473 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -3382,15 +3382,22 @@ bool Tokenizer::simplifyTokenList() simplifyUndefinedSizeArray(); + simplifyCasts(); + + // Simplify simple calculations before replace constants, this allows the replacement of constants that are calculated + // e.g. const static int value = sizeof(X)/sizeof(Y); + simplifyCalculations(); + // Replace constants.. for (Token *tok = list.front(); tok; tok = tok->next()) { - if (Token::Match(tok, "const static| %type% %var% = %num% ;")) { + if (Token::Match(tok, "const static| %type% %var% = %num% ;") || + Token::Match(tok, "const static| %type% %var% ( %num% ) ;")) { unsigned int offset = 0; if (tok->strAt(1) == "static") offset = 1; const unsigned int varId(tok->tokAt(2 + offset)->varId()); if (varId == 0) { - tok = tok->tokAt(5); + tok = tok->tokAt(5 + offset); continue; } @@ -3413,8 +3420,6 @@ bool Tokenizer::simplifyTokenList() } } - simplifyCasts(); - // Simplify simple calculations.. simplifyCalculations(); @@ -5937,9 +5942,10 @@ bool Tokenizer::simplifyKnownVariables() tok = tok->previous(); goback = false; } - if (tok->isName() && Token::Match(tok, "static| const| static| %type% const| %var% = %any% ;")) { + if (tok->isName() && (Token::Match(tok, "static| const| static| %type% const| %var% = %any% ;") || + Token::Match(tok, "static| const| static| %type% const| %var% ( %any% ) ;"))) { bool isconst = false; - for (const Token *tok2 = tok; tok2->str() != "="; tok2 = tok2->next()) { + for (const Token *tok2 = tok; (tok2->str() != "=") && (tok2->str() != "("); tok2 = tok2->next()) { if (tok2->str() == "const") { isconst = true; break; @@ -5962,7 +5968,7 @@ bool Tokenizer::simplifyKnownVariables() const Token * const vartok = (tok->next() && tok->next()->str() == "const") ? tok->tokAt(2) : tok->next(); const Token * const valuetok = vartok->tokAt(2); - if (Token::Match(valuetok, "%bool%|%char%|%num%|%str% ;")) { + if (Token::Match(valuetok, "%bool%|%char%|%num%|%str% )| ;")) { //check if there's not a reference usage inside the code bool withreference = false; for (const Token *tok2 = valuetok->tokAt(2); tok2; tok2 = tok2->next()) { diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 902335e5d..9cebee125 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -121,6 +121,7 @@ private: TEST_CASE(array_index_43); // struct with array TEST_CASE(array_index_44); // #3979 TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...) + TEST_CASE(array_index_46); // #4840 - two-statement for loop TEST_CASE(array_index_multidim); TEST_CASE(array_index_switch_in_for); TEST_CASE(array_index_for_in_for); // FP: #2634 @@ -1506,6 +1507,36 @@ private: ASSERT_EQUALS("", errout.str()); } + // Two statement for-loop + void array_index_46() { + // #4840 + check("void bufferAccessOutOfBounds2() {\n" + " char *buffer[]={\"a\",\"b\",\"c\"};\n" + " for(int i=3; i--;) {\n" + " printf(\"files(%i): %s\n\", 3-i, buffer[3-i]);\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'buffer[3]' accessed at index 3, which is out of bounds.\n", errout.str()); + + check("void f() {\n" + " int buffer[9];\n" + " long int i;\n" + " for(i=10; i--;) {\n" + " buffer[i] = i;\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: buffer\n", errout.str()); + + // Correct access limits -> i from 9 to 0 + check("void f() {\n" + " int buffer[10];\n" + " for(unsigned long int i=10; i--;) {\n" + " buffer[i] = i;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void array_index_multidim() { check("void f()\n" "{\n" diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 1dc5252e9..36c2d2816 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -333,6 +333,7 @@ private: TEST_CASE(simplify_constants2); TEST_CASE(simplify_constants3); TEST_CASE(simplify_constants4); + TEST_CASE(simplify_constants5); TEST_CASE(simplify_null); TEST_CASE(simplifyMulAndParens); // Ticket #2784 + #3184 @@ -5295,6 +5296,15 @@ private: ASSERT_EQUALS("x = 4 ;\ny = 50 ;", tokenizeAndStringify(code,true)); } + void simplify_constants5() { + const char code[] = "int buffer[10];\n" + "static const int NELEMS = sizeof(buffer)/sizeof(int);\n" + "static const int NELEMS2(sizeof(buffer)/sizeof(int));\n" + "x = NELEMS;\n" + "y = NELEMS2;\n"; + ASSERT_EQUALS("int buffer [ 10 ] ;\n\n\nx = 10 ;\ny = 10 ;", tokenizeAndStringify(code,true)); + } + void simplify_null() { { const char code[] =