From 704f285c90197ec341496587ce3de9d67ff12cf0 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 11 Nov 2012 13:03:58 +0100 Subject: [PATCH] Refactorized CheckStl::pushback(): - insert(), reserve() and clear() also invalidate iterators - Properly support return and throw statements: Bailout after semicolon - "." is also bad usage of invalid iterator. --- lib/checkstl.cpp | 21 +++++++++++++++------ test/teststl.cpp | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 6629d5f94..271f75cea 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -537,7 +537,7 @@ void CheckStl::pushback() } // push_back on vector.. - if (Token::Match(tok2, "%varid% . push_front|push_back|resize|reserve", containerId)) { + if (Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize|clear", containerId)) { invalidPointer = true; function = tok2->strAt(2); } @@ -588,6 +588,8 @@ void CheckStl::pushback() // count { , } and parentheses for tok2 int indent = 0; + const Token* validatingToken = 0; + std::string invalidIterator; for (const Token *tok2 = tok; indent >= 0 && tok2; tok2 = tok2->next()) { if (tok2->str() == "{" || tok2->str() == "(") @@ -599,6 +601,11 @@ void CheckStl::pushback() --indent; } + if (validatingToken == tok2) { + invalidIterator.clear(); + validatingToken = 0; + } + // Using push_back or push_front inside a loop.. if (Token::simpleMatch(tok2, "for (")) { tok2 = tok2->tokAt(2); @@ -619,7 +626,7 @@ void CheckStl::pushback() if (tok3->str() == "break" || tok3->str() == "return") { pushbackTok = 0; break; - } else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize (", varId)) { + } else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize|clear (", varId)) { pushbackTok = tok3->tokAt(2); } } @@ -640,7 +647,7 @@ void CheckStl::pushback() } // push_back on vector.. - if (vectorid > 0 && Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize (", vectorid)) { + if (vectorid > 0 && Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize|clear (", vectorid)) { if (!invalidIterator.empty() && Token::Match(tok2->tokAt(2), "insert ( %varid% ,", iteratorid)) { invalidIteratorError(tok2, invalidIterator, tok2->strAt(4)); break; @@ -650,16 +657,18 @@ void CheckStl::pushback() tok2 = tok2->linkAt(3); } + else if (tok2->str() == "return" || tok2->str() == "throw") + validatingToken = Token::findsimplematch(tok2->next(), ";"); + // TODO: instead of bail out for 'else' try to check all execution paths. - else if (tok2->str() == "return" || tok2->str() == "break" || tok2->str() == "else") { + else if (tok2->str() == "break" || tok2->str() == "else") invalidIterator.clear(); - } // Using invalid iterator.. if (!invalidIterator.empty()) { if (Token::Match(tok2, "++|--|*|+|-|(|,|=|!= %varid%", iteratorid)) invalidIteratorError(tok2, invalidIterator, tok2->strAt(1)); - if (Token::Match(tok2, "%varid% ++|--|+|-", iteratorid)) + if (Token::Match(tok2, "%varid% ++|--|+|-|.", iteratorid)) invalidIteratorError(tok2, invalidIterator, tok2->str()); } } diff --git a/test/teststl.cpp b/test/teststl.cpp index 0f9561576..00a88172d 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1142,6 +1142,21 @@ private: " ints.insert(iter, 2);\n" "}\n"); ASSERT_EQUALS("[test.cpp:6]: (error) After insert(), the iterator 'iter' may be invalid.\n", errout.str()); + + check("void* f(const std::vector& bars) {\n" + " std::vector::iterator i = bars.begin();\n" + " bars.insert(Bar());\n" + " void* v = &i->foo;\n" + " return v;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str()); + + check("Foo f(const std::vector& bars) {\n" + " std::vector::iterator i = bars.begin();\n" + " bars.insert(Bar());\n" + " return i->foo;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str()); } void insert2() {