From 6e0ef3eda2611c39247b6be434b1e2814695fd17 Mon Sep 17 00:00:00 2001 From: Pete Johns Date: Sat, 2 Oct 2010 01:23:22 +1000 Subject: [PATCH] Fixed #1132 (Detection of misused scope objects in functions) Emits error in the form: [useless_lock.cpp:18]: (error) instance of "Lock" object destroyed immediately ...if an instance of a class or struct is unnamed and therefore destroyed straight after creation. Only checks for misused scope objects within functions. Optimised isIdentifierObjectType() by memoizing. --- lib/checkother.cpp | 51 ++++++++++++++++++++++ lib/checkother.h | 18 ++++++++ test/testother.cpp | 103 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 6e99e8398..cced6c76e 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3842,6 +3842,52 @@ void CheckOther::checkMathFunctions() } +bool CheckOther::isIdentifierObjectType(const Token * const tok) +{ + const std::string identifier = tok->tokAt(1)->str(); + const MemoizeIsClassResultsIterator found = isClassresults.find(identifier); + if (found != isClassresults.end()) + { + return found->second; + } + + const std::string classDef = std::string("class|struct ") + identifier; + const bool result = Token::findmatch(_tokenizer->tokens(), classDef.c_str()); + isClassresults.insert(std::make_pair(identifier, result)); + return result; +} + + +void CheckOther::checkMisusedScopedObject() +{ + bool withinFuntion = false; + unsigned int depth = 0; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + withinFuntion |= Token::Match(tok, ") const| {"); + if (withinFuntion) + { + if (tok->str() == "{") + { + ++depth; + } + else if (tok->str() == "}") + { + --depth; + withinFuntion &= depth > 0; + } + + if (withinFuntion && Token::Match(tok, "[;{}] %var% (") && isIdentifierObjectType(tok)) + { + tok = tok->next(); + misusedScopeObjectError(tok, tok->str()); + tok = tok->next(); + } + } + } +} + void CheckOther::postIncrement() { @@ -4091,3 +4137,8 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam "selfAssignment", "Redundant assignment of \"" + varname + "\" to itself"); } +void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& varname) +{ + reportError(tok, Severity::error, + "unusedScopedObject", "instance of \"" + varname + "\" object destroyed immediately"); +} diff --git a/lib/checkother.h b/lib/checkother.h index 9d9bb0aa0..4c343be39 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -87,6 +87,7 @@ public: // New type of check: Check execution paths checkOther.executionPaths(); + checkOther.checkMisusedScopedObject(); } @@ -183,6 +184,9 @@ public: /** @brief %Check for assigning a variable to itself*/ void checkSelfAssignment(); + /** @brief %Check for objects that are destroyed immediately */ + void checkMisusedScopedObject(); + // Error messages.. void cstyleCastError(const Token *tok); void dangerousUsageStrtolError(const Token *tok); @@ -209,6 +213,7 @@ public: void fflushOnInputStreamError(const Token *tok, const std::string &varname); void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname); void selfAssignmentError(const Token *tok, const std::string &varname); + void misusedScopeObjectError(const Token *tok, const std::string &varname); void getErrorMessages() { @@ -222,6 +227,7 @@ public: zerodivError(0); mathfunctionCallError(0); fflushOnInputStreamError(0, "stdin"); + misusedScopeObjectError(NULL, "varname"); // style cstyleCastError(0); @@ -337,6 +343,18 @@ private: return varname; } + + /** + * @brief query type of identifier + * @param tok Token of the identifier + * @return true if the identifier is of type 'class' or 'struct', + * false otherwise. + */ + bool isIdentifierObjectType(const Token* const tok); + + typedef std::map MemoizeIsClassResults; + typedef MemoizeIsClassResults::const_iterator MemoizeIsClassResultsIterator; + MemoizeIsClassResults isClassresults; }; /// @} //--------------------------------------------------------------------------- diff --git a/test/testother.cpp b/test/testother.cpp index 461a087df..0233b1f22 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -104,6 +104,13 @@ private: TEST_CASE(selfAssignment); TEST_CASE(testScanf1); TEST_CASE(testScanf2); + + TEST_CASE(trac1132); + TEST_CASE(testMisusedScopeObjectDoesNotPickFunction); + TEST_CASE(testMisusedScopeObjectPicksClass); + TEST_CASE(testMisusedScopeObjectPicksStruct); + TEST_CASE(testMisusedScopeObjectDoesNotPickIf); + TEST_CASE(testMisusedScopeObjectDoesNotPickConstructorDeclaration); } void check(const char code[]) @@ -134,6 +141,7 @@ private: checkOther.checkFflushOnInputStream(); checkOther.checkSelfAssignment(); checkOther.invalidScanf(); + checkOther.checkMisusedScopedObject(); } @@ -2997,6 +3005,101 @@ private: ASSERT_EQUALS("[test.cpp:6]: (style) scanf without field width limits can crash with huge input data\n" "[test.cpp:7]: (style) scanf without field width limits can crash with huge input data\n", errout.str()); } + + void trac1132() + { + errout.str(""); + + std::istringstream code("#include \n" + "class Lock\n" + "{\n" + "public:\n" + " Lock(int i)\n" + " {\n" + " std::cout << \"Lock \" << i << std::endl;\n" + " }\n" + " ~Lock()\n" + " {\n" + " std::cout << \"~Lock\" << std::endl;\n" + " }\n" + "};\n" + "int main()\n" + "{\n" + " Lock(123);\n" + " std::cout << \"hello\" << std::endl;\n" + " return 0;\n" + "}\n" + ); + + Tokenizer tokenizer; + tokenizer.tokenize(code, "trac1132.cpp"); + tokenizer.simplifyTokenList(); + + Settings settings; + + CheckOther checkOther(&tokenizer, &settings, this); + checkOther.checkMisusedScopedObject(); + + ASSERT_EQUALS("[trac1132.cpp:16]: (error) instance of \"Lock\" object destroyed immediately\n", errout.str()); + } + + void testMisusedScopeObjectDoesNotPickFunction() + { + check("int main ( )\n" + "{\n" + " CouldBeFunction ( 123 ) ;\n" + " return 0 ;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + } + + void testMisusedScopeObjectPicksClass() + { + check("class NotAFunction ;\n" + "int function ( )\n" + "{\n" + " NotAFunction ( 123 );\n" + " return 0 ;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:4]: (error) instance of \"NotAFunction\" object destroyed immediately\n", errout.str()); + } + + void testMisusedScopeObjectPicksStruct() + { + check("struct NotAClass;\n" + "bool func ( )\n" + "{\n" + " NotAClass ( 123 ) ;\n" + " return true ;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:4]: (error) instance of \"NotAClass\" object destroyed immediately\n", errout.str()); + } + + void testMisusedScopeObjectDoesNotPickIf() + { + check("bool func( int a , int b , int c )\n" + "{\n" + " if ( a > b ) return c == a ;\n" + " return b == a ;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + } + + void testMisusedScopeObjectDoesNotPickConstructorDeclaration() + { + check("class Something : public SomthingElse\n" + "{\n" + "public:\n" + "~Something ( ) ;\n" + "Something ( ) ;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestOther)