From 45f8560537545bb6e9ba279e21be6d27be9a2682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 21 Jul 2009 12:09:58 +0200 Subject: [PATCH] invalid pointer usage: broke out CheckOther::invalidPointer from the CheckOther::nullPointer (#485) --- src/checkother.cpp | 39 +++++++++++++--- src/checkother.h | 7 +++ test/testother.cpp | 114 +++++++++++++++++++++++++++++---------------- 3 files changed, 113 insertions(+), 47 deletions(-) diff --git a/src/checkother.cpp b/src/checkother.cpp index f831a9fb6..e78069dfb 100644 --- a/src/checkother.cpp +++ b/src/checkother.cpp @@ -1028,7 +1028,11 @@ void CheckOther::nullPointer() tok2 = tok2->next(); } } +} + +void CheckOther::invalidPointer() +{ // Dereferencing a pointer and then checking if it's NULL.. for (const Token *tok1 = _tokenizer->tokens(); tok1; tok1 = tok1->next()) { @@ -1037,11 +1041,16 @@ void CheckOther::nullPointer() if (std::string(tok1->strAt(1)) == tok1->strAt(3)) continue; + const unsigned int varid1(tok1->next()->varId()); + tok1 = tok1->tokAt(3); - const unsigned int varid1(tok1->varId()); - if (varid1 == 0) + const unsigned int varid2(tok1->varId()); + if (varid1 == 0 || varid2 == 0) continue; + // token where the variable is dereferenced.. + const Token * dereferenced = 0; + unsigned int indentlevel2 = 0; for (const Token *tok2 = tok1->tokAt(3); tok2; tok2 = tok2->next()) { @@ -1055,13 +1064,26 @@ void CheckOther::nullPointer() --indentlevel2; } - else if (Token::Match(tok2, "%varid% =", varid1)) + else if (Token::Match(tok2, "* %varid%", varid1)) + dereferenced = tok2; + + // Reassignment of the struct + else if (tok2->varId() == varid1) + { + if (Token::Match(tok2->next(), "=")) + break; + if (Token::Match(tok2->tokAt(-2), "[,(] &")) + break; + } + + // return at base level => stop checking + else if (indentlevel2 == 0 && tok2->str() == "return") break; - else if (tok2->str() == "if") + else if (dereferenced && tok2->str() == "if") { - if (Token::Match(tok2, "if ( !| %varid% )", varid1)) - nullPointerError(tok1); + if (Token::Match(tok2, "if ( !| %varid% )", varid2)) + invalidPointerError(dereferenced); } } } @@ -1167,6 +1189,11 @@ void CheckOther::nullPointerError(const Token *tok) reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference"); } +void CheckOther::invalidPointerError(const Token *tok) +{ + reportError(tok, Severity::error, "invalidPointer", "Possible invalid pointer dereference"); +} + void CheckOther::zerodivError(const Token *tok) { reportError(tok, Severity::error, "zerodiv", "Division by zero"); diff --git a/src/checkother.h b/src/checkother.h index a684d3158..0fce6db0d 100644 --- a/src/checkother.h +++ b/src/checkother.h @@ -72,6 +72,7 @@ public: checkOther.strPlusChar(); checkOther.invalidFunctionUsage(); checkOther.nullPointer(); + checkOther.invalidPointer(); checkOther.checkZeroDivision(); } @@ -111,6 +112,9 @@ public: /** possible null pointer dereference */ void nullPointer(); + /** possible invalid pointer dereference */ + void invalidPointer(); + /** Check zero division*/ void checkZeroDivision(); @@ -141,6 +145,7 @@ private: void conditionAlwaysTrueFalse(const Token *tok, const std::string &truefalse); void strPlusChar(const Token *tok); void nullPointerError(const Token *tok); + void invalidPointerError(const Token *tok); void zerodivError(const Token *tok); void getErrorMessages() @@ -163,6 +168,7 @@ private: conditionAlwaysTrueFalse(0, "true/false"); strPlusChar(0); nullPointerError(0); + invalidPointerError(0); zerodivError(0); } @@ -186,6 +192,7 @@ private: " * condition that is always true/false\n" " * unusal pointer arithmetic. For example: \"abc\" + 'd'\n" " * dereferencing a null pointer\n" + " * dereferencing an invalid pointer\n" " * [[IncompleteStatement|Incomplete statement]]\n"; } }; diff --git a/test/testother.cpp b/test/testother.cpp index dbf2aaa23..bf9adcf97 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -58,9 +58,8 @@ private: TEST_CASE(nullpointer1); TEST_CASE(nullpointer2); - TEST_CASE(nullpointer3); - TEST_CASE(nullpointer4); - TEST_CASE(nullpointer5); + + TEST_CASE(invalidpointer); TEST_CASE(oldStylePointerCast); } @@ -447,56 +446,89 @@ private: ASSERT_EQUALS("", errout.str()); } - void nullpointer3() + + + void checkInvalidPointer(const char code[]) { - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int *a = abc->a;\n" - " if (!abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference\n", errout.str()); + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.setVarId(); + + // Clear the error buffer.. + errout.str(""); + + // Check for redundant code.. + Settings settings; + settings._checkCodingStyle = true; + CheckOther checkOther(&tokenizer, &settings, this); + checkOther.invalidPointer(); } - void nullpointer4() + void invalidpointer() { - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " int *a = abc->a;\n" - " if (abc)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference\n", errout.str()); - } + // errors.. + checkInvalidPointer("void foo(struct ABC *abc)\n" + "{\n" + " int *a = abc->a;\n" + " *a;\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible invalid pointer dereference\n", errout.str()); - void nullpointer5() - { // ok dereferencing in a condition - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " if (abc && abc->a);\n" - " if (!abc)\n" - " ;\n" - "}\n"); + checkInvalidPointer("void foo(struct ABC *abc)\n" + "{\n" + " if (abc && abc->a);\n" + " if (!abc)\n" + " ;\n" + "}\n"); ASSERT_EQUALS("", errout.str()); // ok to use a linked list.. - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " abc = abc->next;\n" - " if (!abc)\n" - " ;\n" - "}\n"); + checkInvalidPointer("void foo(struct ABC *abc)\n" + "{\n" + " abc = abc->next;\n" + " if (!abc)\n" + " ;\n" + "}\n"); ASSERT_EQUALS("", errout.str()); // reassign struct.. - checkNullPointer("void foo(struct ABC *abc)\n" - "{\n" - " a = abc->a;\n" - " abc = abc->next;\n" - " if (!abc)\n" - " ;\n" - "}\n"); + checkInvalidPointer("void foo(struct ABC *abc)\n" + "{\n" + " a = abc->a;\n" + " *a;\n" + " abc = abc->next;\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + checkInvalidPointer("void foo(struct ABC *abc)\n" + "{\n" + " a = abc->a;\n" + " *a;\n" + " f(&abc);\n" + " if (!abc)\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // goto.. + checkInvalidPointer("void foo(struct ABC *abc)\n" + "{\n" + " if (!abc)\n" + " goto out;" + " a = abc->a;\n" + " *a;\n" + " return;\n" + "out:\n" + " if (!abc)\n" + " ;\n" + "}\n"); ASSERT_EQUALS("", errout.str()); }