From 5b940c0c7f476564d3cbefc867722be39250507f Mon Sep 17 00:00:00 2001 From: seb777 Date: Thu, 16 Jun 2011 20:26:00 +0200 Subject: [PATCH] fix #747 and #748 (incorrect use of auto_ptr - new check) --- lib/checkstl.cpp | 101 +++++++++++++++++++++++++++++++++++++++++++++++ lib/checkstl.h | 16 +++++++- test/teststl.cpp | 86 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e04a6f86a..5e5e68d7e 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1107,3 +1107,104 @@ void CheckStl::string_c_strError(const Token *tok) reportError(tok, Severity::error, "stlcstr", "Dangerous usage of c_str()"); } + +//--------------------------------------------------------------------------- +// +//--------------------------------------------------------------------------- +void CheckStl::checkAutoPointer() +{ + + std::set autoPtrVarId; + std::set::const_iterator iter; + static const char STL_CONTAINER_LIST[] = "bitset|deque|list|map|multimap|multiset|priority_queue|queue|set|stack|hash_map|hash_multimap|hash_set|vector"; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::simpleMatch(tok, "auto_ptr <")) + { + if ((Token::Match(tok->tokAt(-1), "< auto_ptr") && Token::Match(tok->tokAt(-2), STL_CONTAINER_LIST)) || (Token::Match(tok->tokAt(-3), "< std :: auto_ptr") && Token::Match(tok->tokAt(-4), STL_CONTAINER_LIST))) + { + autoPointerContainerError(tok); + } + else + { + const Token *tok2 = tok->next()->next(); + while (tok2) + { + + if (Token::Match(tok2, "> %var%")) + { + const Token *tok3 = tok2->next()->next(); + while (tok3 && ! Token::simpleMatch(tok3, ";")) + { + tok3 = tok3->next(); + } + if (Token::Match(tok3->previous()->previous(),"] )")) + { + autoPointerArrayError(tok2->next()); + } + else if (Token::Match(tok3->previous()->previous(),"%var% )")) + { + const Token *decltok = Token::findmatch(_tokenizer->tokens(), "%varid% = new %type% [", tok3->previous()->previous()->varId()); + if (decltok) + { + autoPointerArrayError(tok2->next()); + } + } + autoPtrVarId.insert(tok2->next()->varId()); + break; + } + tok2 = tok2->next(); + } + } + } + else + { + if (Token::Match(tok, "%var% = %var% ;")) + { + if (_settings->_checkCodingStyle) + { + iter = autoPtrVarId.find(tok->next()->next()->varId()); + if (iter != autoPtrVarId.end()) + { + autoPointerError(tok->next()->next()); + } + } + } + else if (Token::Match(tok, "%var% = new %type% [") || Token::Match(tok, "%var% . reset ( new %type% [")) + { + iter = autoPtrVarId.find(tok->varId()); + if (iter != autoPtrVarId.end()) + { + autoPointerArrayError(tok); + } + } + } + } +} + + +void CheckStl::autoPointerError(const Token *tok) +{ + reportError(tok, Severity::style, "useAutoPointerCopy", + "Copy 'auto_ptr' pointer to another do not create two equal objects since one has lost its ownership of the pointer.\n" + "The auto_ptr has semantics of strict ownership, meaning that the auto_ptr instance is the sole entity responsible for the object's lifetime. If an auto_ptr is copied, the source loses the reference." + ); +} + +void CheckStl::autoPointerContainerError(const Token *tok) +{ + reportError(tok, Severity::error, "useAutoPointerContainer", + "You can randomly lose access to pointers if you store 'auto_ptr' pointers in a container because the copy-semantics of 'auto_ptr' are not compatible with containers.\n" + "An element of container must be able to be copied but 'auto_ptr' does not fulfill this requirement. You should consider to use 'shared_ptr' or 'unique_ptr'. It is suitable for use in containers, because they no longer copy their values, they move them." + ); +} + +void CheckStl::autoPointerArrayError(const Token *tok) +{ + reportError(tok, Severity::error, "useAutoPointerArray", + "Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n" + "Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. This means that you should only use 'auto_ptr' for pointers obtained with operator 'new'. This excludes arrays, which are allocated by operator 'new[]' and must be deallocated by operator 'delete[]'." + ); +} + diff --git a/lib/checkstl.h b/lib/checkstl.h index 3be11421c..b9951b49f 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -56,11 +56,13 @@ public: checkStl.stlBoundries(); checkStl.if_find(); checkStl.string_c_str(); + checkStl.checkAutoPointer(); // Style check checkStl.size(); checkStl.redundantCondition(); checkStl.missingComparison(); + } @@ -134,6 +136,10 @@ public: void string_c_str(); void string_c_strError(const Token *tok); + /** @brief %Check for use and copy auto pointer */ + void checkAutoPointer(); + + private: /** @@ -154,6 +160,10 @@ private: void sizeError(const Token *tok); void redundantIfRemoveError(const Token *tok); + void autoPointerError(const Token *tok); + void autoPointerContainerError(const Token *tok); + void autoPointerArrayError(const Token *tok); + void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { CheckStl c(0, settings, errorLogger); @@ -171,6 +181,9 @@ private: c.string_c_strError(0); c.sizeError(0); c.redundantIfRemoveError(0); + c.autoPointerError(0); + c.autoPointerContainerError(0); + c.autoPointerArrayError(0); } std::string myName() const @@ -189,7 +202,8 @@ private: "* optimisation: use empty() instead of size() to guarantee fast code\n" "* suspicious condition when using find\n" "* redundant condition\n" - "* common mistakes when using string::c_str()"; + "* common mistakes when using string::c_str()\n" + "* using auto pointer (auto_ptr)"; } bool isStlContainer(unsigned int varid); diff --git a/test/teststl.cpp b/test/teststl.cpp index 180f78480..09c0ebb3c 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -108,6 +108,9 @@ private: // catch common problems when using the string::c_str() function TEST_CASE(cstr); + + TEST_CASE(autoPointer); + } void check(const std::string &code) @@ -1314,6 +1317,89 @@ private: "}"); ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous usage of c_str()\n", errout.str()); } + + void autoPointer() + { + + check("void foo()\n" + "{\n" + " auto_ptr< ns1:::MyClass > y;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f() \n" + "{\n" + " auto_ptr p2;\n" + " p2 = new T;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void foo()\n" + "{\n" + " std::vector< std::auto_ptr< ns1::MyClass> > v;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) You can randomly lose access to pointers if you store 'auto_ptr' pointers in a container because the copy-semantics of 'auto_ptr' are not compatible with containers.\n", errout.str()); + + check("void foo()\n" + "{\n" + " std::vector< auto_ptr< MyClass> > v;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) You can randomly lose access to pointers if you store 'auto_ptr' pointers in a container because the copy-semantics of 'auto_ptr' are not compatible with containers.\n", errout.str()); + + check("void foo()\n" + "{\n" + " int *i = new int;\n" + " auto_ptr x(i);\n" + " auto_ptr y;\n" + " y = x;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (style) Copy 'auto_ptr' pointer to another do not create two equal objects since one has lost its ownership of the pointer.\n", errout.str()); + + // ticket #748 + check("void f() \n" + "{\n" + " T* var = new T[10];\n" + " auto_ptr p2( var );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n", errout.str()); + + check("void f() \n" + "{\n" + " auto_ptr p2( new T[] );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n", errout.str()); + + check("void f() \n" + "{\n" + " auto_ptr p2;\n" + " p2.reset( new T[] );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n", errout.str()); + + + check("void f() \n" + "{\n" + " auto_ptr p2( new T[][] );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n", errout.str()); + + check("void f() \n" + "{\n" + " auto_ptr p2;\n" + " p2 = new T[10];\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n", errout.str()); + + check("void f() \n" + "{\n" + " auto_ptr p2;\n" + " p2.reset( new T[10] );\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with operator 'new[]'.\n", errout.str()); + + } + + }; REGISTER_TEST(TestStl)