Fixed #877 (False positive: After insert, iterator may be invalid)

lib/checkstl.cpp

@@ -295,111 +295,123 @@ void CheckStl::pushback()
     // Iterator becomes invalid after push_back or push_front..
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
-        if (Token::simpleMatch(tok, "vector <"))
+        if (!Token::simpleMatch(tok, "vector <"))
+            continue;
+
+        // if iterator declaration inside for() loop
+        bool iteratorDeclaredInsideLoop = false;
+        if ((tok->tokAt(-2) && Token::simpleMatch(tok->tokAt(-2), "for (")) ||
+            (tok->tokAt(-4) && Token::simpleMatch(tok->tokAt(-4), "for ( std ::")))
         {
-            // if iterator declaration inside for() loop
+            iteratorDeclaredInsideLoop = true;
-            bool iteratorDeclaredInsideLoop = false;
+        }
-            if ((tok->tokAt(-2) && Token::simpleMatch(tok->tokAt(-2), "for (")) ||
+
-                (tok->tokAt(-4) && Token::simpleMatch(tok->tokAt(-4), "for ( std ::")))
+        while (tok && tok->str() != ">")
+            tok = tok->next();
+        if (!tok)
+            break;
+        if (!Token::Match(tok, "> :: iterator|const_iterator %var% =|;"))
+            continue;
+
+        const unsigned int iteratorid(tok->tokAt(3)->varId());
+        if (iteratorid == 0)
+            continue;
+
+        if (iteratorDeclaredInsideLoop && tok->tokAt(4)->str() == "=")
+        {
+            // skip "> :: iterator|const_iterator"
+            tok = tok->tokAt(3);
+        }
+
+        std::string vectorname;
+        int indent = 0;
+        std::string invalidIterator;
+        for (const Token *tok2 = tok; indent >= 0 && tok2; tok2 = tok2->next())
+        {
+            if (tok2->str() == "{" || tok2->str() == "(")
+                ++indent;
+            else if (tok2->str() == "}" || tok2->str() == ")")
             {
-                iteratorDeclaredInsideLoop = true;
+                if (indent == 0 && Token::simpleMatch(tok2, ") {"))
+                    tok2 = tok2->next();
+                else
+                    --indent;
             }
 
-            while (tok && tok->str() != ">")
+            // Using push_back or push_front inside a loop..
-                tok = tok->next();
+            if (Token::Match(tok2, "for ("))
-            if (!tok)
-                break;
-            if (Token::Match(tok, "> :: iterator|const_iterator %var% =|;"))
             {
-                const unsigned int iteratorid(tok->tokAt(3)->varId());
+                tok2 = tok2->tokAt(2);
-                if (iteratorid == 0)
+            }
+
+            if (Token::Match(tok2, "%varid% = %var% . begin ( ) ; %varid% != %var% . end ( ) ; ++| %varid% ++| ) {", iteratorid))
+            {
+                const unsigned int vectorid(tok2->tokAt(2)->varId());
+                if (vectorid == 0)
                     continue;
 
-                if (iteratorDeclaredInsideLoop && tok->tokAt(4)->str() == "=")
+                const Token *pushback = 0;
+                unsigned int indent3 = 0;
+                for (const Token *tok3 = tok2->tokAt(20); tok3; tok3 = tok3->next())
                 {
-                    // skip "> :: iterator|const_iterator"
+                    if (tok3->str() == "{")
-                    tok = tok->tokAt(3);
+                        ++indent3;
+                    else if (tok3->str() == "}")
+                    {
+                        if (indent3 <= 1)
+                            break;
+                        --indent3;
+                    }
+                    else if (tok3->str() == "break")
+                    {
+                        pushback = 0;
+                        break;
+                    }
+                    else if (Token::Match(tok3, "%varid% . push_front|push_back|insert (", vectorid))
+                    {
+                        pushback = tok3->tokAt(2);
+                    }
                 }
 
-                std::string vectorname;
+                if (pushback)
-                int indent = 0;
+                    invalidIteratorError(pushback, pushback->str(), tok2->strAt(0));
-                std::string invalidIterator;
+            }
-                for (const Token *tok2 = tok; indent >= 0 && tok2; tok2 = tok2->next())
+
+            // Assigning iterator..
+            if (Token::Match(tok2, "%varid% =", iteratorid))
+            {
+                if (Token::Match(tok2->tokAt(2), "%var% . begin|end|rbegin|rend ( )"))
+                    vectorname = tok2->strAt(2);
+                else
+                    vectorname = "";
+                invalidIterator = "";
+            }
+
+            // push_back on vector..
+            if (vectorname.size() && Token::Match(tok2, (vectorname + " . push_front|push_back|insert").c_str()))
+            {
+                invalidIterator = tok2->strAt(2);
+                if (!iteratorDeclaredInsideLoop)
                 {
-                    if (tok2->str() == "{" || tok2->str() == "(")
+                    while (tok2 && tok2->str() != "(")
-                        ++indent;
+                        tok2 = tok2->next();
-                    else if (tok2->str() == "}" || tok2->str() == ")")
+                    tok2 = tok2 ? tok2->link() : 0;
-                    {
+                    if (!tok2)
-                        if (indent == 0 && Token::simpleMatch(tok2, ") {"))
+                        break;
-                            tok2 = tok2->next();
-                        else
-                            --indent;
-                    }
-
-                    // Using push_back or push_front inside a loop..
-                    if (Token::Match(tok2, "for ("))
-                    {
-                        tok2 = tok2->tokAt(2);
-                    }
-
-                    if (Token::Match(tok2, "%varid% = %var% . begin ( ) ; %varid% != %var% . end ( ) ; ++| %varid% ++| ) {", iteratorid))
-                    {
-                        const unsigned int vectorid(tok2->tokAt(2)->varId());
-                        if (vectorid == 0)
-                            continue;
-
-                        const Token *pushback = 0;
-                        unsigned int indent3 = 0;
-                        for (const Token *tok3 = tok2->tokAt(20); tok3; tok3 = tok3->next())
-                        {
-                            if (tok3->str() == "{")
-                                ++indent3;
-                            else if (tok3->str() == "}")
-                            {
-                                if (indent3 <= 1)
-                                    break;
-                                --indent3;
-                            }
-                            else if (tok3->str() == "break")
-                            {
-                                pushback = 0;
-                                break;
-                            }
-                            else if (Token::Match(tok3, "%varid% . push_front|push_back|insert (", vectorid))
-                            {
-                                pushback = tok3->tokAt(2);
-                            }
-                        }
-
-                        if (pushback)
-                            invalidIteratorError(pushback, pushback->str(), tok2->strAt(0));
-                    }
-
-                    // Assigning iterator..
-                    if (Token::Match(tok2, "%varid% =", iteratorid))
-                    {
-                        if (Token::Match(tok2->tokAt(2), "%var% . begin|end|rbegin|rend ( )"))
-                            vectorname = tok2->strAt(2);
-                        else
-                            vectorname = "";
-                        invalidIterator = "";
-                    }
-
-                    // push_back on vector..
-                    if (vectorname.size() && Token::Match(tok2, (vectorname + " . push_front|push_back|insert").c_str()))
-                        invalidIterator = tok2->strAt(2);
-
-                    // Using invalid iterator..
-                    if (!invalidIterator.empty())
-                    {
-                        if (Token::Match(tok2, "++|--|*|+|-|(|,|=|!= %varid%", iteratorid))
-                            invalidIteratorError(tok2, invalidIterator, tok2->strAt(1));
-                        if (Token::Match(tok2, "%varid% ++|--|+|-", iteratorid))
-                            invalidIteratorError(tok2, invalidIterator, tok2->str());
-                    }
                 }
             }
+
+            // Using invalid iterator..
+            if (!invalidIterator.empty())
+            {
+                if (Token::Match(tok2, "++|--|*|+|-|(|,|=|!= %varid%", iteratorid))
+                    invalidIteratorError(tok2, invalidIterator, tok2->strAt(1));
+                if (Token::Match(tok2, "%varid% ++|--|+|-", iteratorid))
+                    invalidIteratorError(tok2, invalidIterator, tok2->str());
+            }
         }
+
+
     }
 }
 

test/teststl.cpp

@@ -489,6 +489,13 @@ private:
               "    ++iter;\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:5]: (error) After insert, the iterator 'iter' may be invalid\n", errout.str());
+
+        check("void f()\n"
+              "{\n"
+              "    std::vector<int>::iterator iter = ints.begin();\n"
+              "    ints.insert(iter, 1);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }