From 04eb9cf3052e24341aea486cfcfdc556a0de959f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 31 Dec 2010 18:07:46 +0100 Subject: [PATCH] Fixed #2378 (Refactoring: create utility function that skips redundant if/for/while) --- lib/checkbufferoverrun.cpp | 23 +++++++++++++++++++-- test/testbufferoverrun.cpp | 41 +++++++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index f436e19e9..e86c4f55a 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -222,6 +222,9 @@ static bool bailoutIfSwitch(const Token *tok, const unsigned int varid) else if (str1 == "if" && tok->str() == "break") return true; + // bailout for "return" + else if (tok->str() == "return") + return true; // bailout if varid is found else if (tok->varId() == varid) @@ -636,8 +639,24 @@ void CheckBufferOverrun::checkFunctionCall(const Token &tok, unsigned int par, c // Check the parameter usage in the function scope.. for (; ftok; ftok = ftok->next()) { - if (ftok->str() == "if") - break; + if (Token::Match(ftok, "if|for|while (")) + { + // bailout if there is buffer usage.. + if (bailoutIfSwitch(ftok, parameterVarId)) + { + break; + } + + // no bailout is needed. skip the if-block + else + { + // goto end of if block.. + ftok = ftok->next()->link()->next()->link(); + if (Token::simpleMatch(ftok, "} else {")) + ftok = ftok->tokAt(2)->link(); + continue; + } + } if (ftok->str() == "}") break; diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 94634c979..e712f50b6 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -131,6 +131,7 @@ private: TEST_CASE(buffer_overrun_14); TEST_CASE(buffer_overrun_15); // ticket #1787 TEST_CASE(buffer_overrun_16); + TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch // It is undefined behaviour to point out of bounds of an array // the address beyond the last element is in bounds @@ -1546,7 +1547,7 @@ private: " char s[3];\n" " f1(s,3);\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:3]: (possible error) Buffer access out-of-bounds\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Buffer access out-of-bounds\n", errout.str()); ASSERT_EQUALS("", errout.str()); check("void f1(char *s,int size)\n" @@ -1834,6 +1835,44 @@ private: ASSERT_EQUALS("", errout.str()); } + void buffer_overrun_bailoutIfSwitch() + { + // No false positive + check("void f1(char *s) {\n" + " if (x) s[100] = 0;\n" + "}\n" + "\n" + "void f2() {\n" + " char a[10];\n" + " f1(a);" + "}"); + ASSERT_EQUALS("", errout.str()); + + // No false positive + check("void f1(char *s) {\n" + " if (x) return;\n" + " s[100] = 0;\n" + "}\n" + "\n" + "void f2() {\n" + " char a[10];\n" + " f1(a);" + "}"); + ASSERT_EQUALS("", errout.str()); + + // No false negative + check("void f1(char *s) {\n" + " if (x) { }\n" + " s[100] = 0;\n" + "}\n" + "\n" + "void f2() {\n" + " char a[10];\n" + " f1(a);" + "}"); + ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Array 'a[10]' index 100 out of bounds\n", errout.str()); + } + void pointer_out_of_bounds_1() { check("void f() {\n"