From 4eeec75b73183e373f674a03cbb09f7afb785651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 4 Sep 2012 16:29:06 +0200 Subject: [PATCH 1/4] TestPreprocessor: Encode extended ASCII characters to avoid VS compiler warnings. --- test/testpreprocessor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index ea83acf92..e3bbca0aa 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -745,8 +745,8 @@ private: void error2() { errout.str(""); - const char filedata[] = "#error ê\n" - "#warning ê\n" + const char filedata[] = "#error \xAB\n" + "#warning \xAB\n" "123"; // Read string.. From 2197b84d78c3926b432c65f4af2053fd49b185f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 4 Sep 2012 16:29:23 +0200 Subject: [PATCH 2/4] astyle formatting --- lib/checkother.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 18d0f7e10..3ce606108 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -260,7 +260,7 @@ void CheckOther::clarifyStatement() } if (Token::Match(tok, "++|-- [;,]")) //TODO: change the string in order to remove the excessive spaces between the tokens. - clarifyStatementError(tok, + clarifyStatementError(tok, tok2->next()->stringifyList(tok->tokAt(2)), "("+tok2->next()->stringifyList(tok)+")"+tok->stringifyList(tok->tokAt(2))); } From 516232e35f732eeeab9248a9e1f4d9d8363d089a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 5 Sep 2012 06:45:39 +0200 Subject: [PATCH 3/4] Travis: Enable clang compiler --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 95a70463a..f8339e896 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,5 @@ language: cpp compiler: - gcc + - clang script: make test && g++ -o cppcheck -O2 cli/*.cpp lib/*.cpp -Ilib && ./cppcheck --error-exitcode=1 -q cli gui lib From 8546bcc94e2dba616072d5b190761172aec6101a Mon Sep 17 00:00:00 2001 From: Zachary Blair Date: Tue, 4 Sep 2012 23:31:23 -0700 Subject: [PATCH 4/4] Fixed #2029 (new check: free invalid address) --- lib/checkother.cpp | 57 +++++++++++++++++++++++++++++++ lib/checkother.h | 6 ++++ test/testother.cpp | 84 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3ce606108..07e5c1468 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2409,6 +2409,63 @@ void CheckOther::checkDuplicateBranch() } } + +//----------------------------------------------------------------------------- +// Check for a free() of an invalid address +// char* p = malloc(100); +// free(p + 10); +//----------------------------------------------------------------------------- +void CheckOther::checkInvalidFree() +{ + std::set allocatedVariables; + for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) { + + // Keep track of which variables were assigned addresses to newly-allocated memory + if (Token::Match(tok, "%var% = malloc|g_malloc|new")) { + allocatedVariables.insert(tok->varId()); + } + + // If these variables assigned new values before being used to free the memory, we can't + // say anything about whether the resulting expression is valid + else if (Token::Match(tok, "%var% =")) { + allocatedVariables.erase(tok->varId()); + } + + // If a variable that was previously assigned a newly-allocated memory location is + // added or subtracted from when used to free the memory, report an error. + else if (Token::Match(tok, "free|g_free|delete ( %any% +|- %any%") || + Token::Match(tok, "delete [ ] ( %any% +|- %any%") || + Token::Match(tok, "delete %any% +|- %any%")) { + + int varIdx = tok->strAt(1) == "(" ? 2 : + tok->strAt(3) == "(" ? 4 : 1; + unsigned int var1 = tok->tokAt(varIdx)->varId(); + unsigned int var2 = tok->tokAt(varIdx + 2)->varId(); + if (allocatedVariables.find(var1) != allocatedVariables.end() || + allocatedVariables.find(var2) != allocatedVariables.end()) { + invalidFreeError(tok); + } + } + + // If the previously-allocated variable is passed in to another function + // as a parameter, it might be modified, so we shouldn't report an error + // if it is later used to free memory + else if (Token::Match(tok, "%var% (")) { + const Token* tok2 = Token::findmatch(tok->tokAt(1), "%var%", tok->linkAt(1)); + while (tok2 != NULL) { + allocatedVariables.erase(tok2->varId()); + tok2 = Token::findmatch(tok2->next(), "%var%", tok->linkAt(1)); + } + } + } +} + +void CheckOther::invalidFreeError(const Token *tok) +{ + reportError(tok, Severity::error, "invalidFree", "Invalid memory address freed."); +} + + //----------------------------------------------------------------------------- // Check for double free // free(p); free(p); diff --git a/lib/checkother.h b/lib/checkother.h index 3ab32d658..b926ae2af 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -106,6 +106,7 @@ public: checkOther.checkAssignBoolToPointer(); checkOther.checkBitwiseOnBoolean(); + checkOther.checkInvalidFree(); checkOther.checkDoubleFree(); checkOther.checkRedundantCopy(); checkOther.checkNegativeBitwiseShift(); @@ -247,6 +248,10 @@ public: /** @brief %Check for suspicious use of semicolon */ void checkSuspiciousSemicolon(); + /** @brief %Check for free() operations on invalid memory locations */ + void checkInvalidFree(); + void invalidFreeError(const Token *tok); + /** @brief %Check for double free or double close operations */ void checkDoubleFree(); void doubleFreeError(const Token *tok, const std::string &varname); @@ -407,6 +412,7 @@ private: "* sizeof for numeric given as function argument\n" "* using sizeof(pointer) instead of the size of pointed data\n" "* incorrect length arguments for 'substr' and 'strncmp'\n" + "* free() or delete of an invalid memory location\n" "* double free() or double closedir()\n" "* bitwise operation with negative right operand\n" diff --git a/test/testother.cpp b/test/testother.cpp index 44b0bad98..071b4230b 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -162,6 +162,7 @@ private: TEST_CASE(checkForSuspiciousSemicolon2); TEST_CASE(checkDoubleFree); + TEST_CASE(checkInvalidFree); TEST_CASE(checkRedundantCopy); @@ -5864,6 +5865,89 @@ private: ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); } + + void checkInvalidFree() { + check( + "void foo(char *p) {\n" + " char *a = (char *)malloc(1024);\n" + " free(a + 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = (char *)malloc(1024);\n" + " free(a - 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = (char *)malloc(1024);\n" + " free(10 + a);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = new char[1024];\n" + " delete[] (a + 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = new char;\n" + " delete (a + 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str()); + + + check( + "void foo(char *p) {\n" + " char *a = new char;\n" + " a = a - 10;\n" + " delete (a + 10);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = new char;\n" + " bar(a);\n" + " delete (a + 10);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = new char;\n" + " char *b = new char;\n" + " bar(a);\n" + " delete (a + 10);\n" + " delete (b + 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (error) Invalid memory address freed.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = new char;\n" + " char *b = new char;\n" + " bar(a, b);\n" + " delete (a + 10);\n" + " delete (b + 10);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " char *a = new char;\n" + " bar()\n" + " delete (a + 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Invalid memory address freed.\n", errout.str()); + } + void check_redundant_copy(const char code[]) { // Clear the error buffer.. errout.str("");