Fix issue 9435: False negative: invalidContainer when using range for loop (#2587)

* Fix issue 9435: False negative: invalidContainer when using range for loop

* Use ast

* Make string const

lib/checkstl.cpp

@@ -846,6 +846,59 @@ void CheckStl::invalidContainer()
     }
 }
 
+const Token* getLoopContainer(const Token* tok)
+{
+    if (!Token::simpleMatch(tok, "for ("))
+        return nullptr;
+    const Token * sepTok = tok->next()->astOperand2();
+    if (!Token::simpleMatch(sepTok, ":"))
+        return nullptr;
+    return sepTok->astOperand2();
+}
+
+void CheckStl::invalidContainerLoop()
+{
+    const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
+    const Library& library = mSettings->library;
+    for (const Scope * scope : symbolDatabase->functionScopes) {
+        for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
+            const Token* contTok = getLoopContainer(tok);
+            if (!contTok)
+                continue;
+            const Token * blockStart = tok->next()->link()->next();
+            const Token * blockEnd = blockStart->link();
+            if (!Token::Match(contTok, "%var%"))
+                continue;
+            if (contTok->varId() == 0)
+                continue;
+            if (!astIsContainer(contTok))
+                continue;
+            nonneg int varid = contTok->varId();
+            for (const Token* tok2 = blockStart; tok2 != blockEnd; tok2 = tok2->next()) {
+                if (tok2->varId() != varid)
+                    continue;
+                if (!Token::Match(tok2->next(), ". %name% ("))
+                    continue;
+                if (!isInvalidMethod(tok2))
+                    continue;
+                invalidContainerLoopError(tok2, tok);
+                break;
+            }
+        }
+    }
+}
+
+void CheckStl::invalidContainerLoopError(const Token *tok, const Token * loopTok)
+{
+    ErrorPath errorPath;
+    const std::string method = tok ? tok->strAt(2) : "erase";
+    errorPath.emplace_back(loopTok, "Iterating container here.");
+
+    const std::string msg = "Calling '" + method + "' while iterating the container is invalid.";
+    errorPath.emplace_back(tok, "");
+    reportError(errorPath, Severity::error, "invalidContainerLoop", msg, CWE664, false);
+}
+
 void CheckStl::invalidContainerError(const Token *tok, const Token * contTok, const ValueFlow::Value *val, ErrorPath errorPath)
 {
     const bool inconclusive = val ? val->isInconclusive() : false;

lib/checkstl.h

@@ -77,6 +77,7 @@ public:
         checkStl.negativeIndex();
 
         checkStl.invalidContainer();
+        checkStl.invalidContainerLoop();
         checkStl.mismatchingContainers();
 
         checkStl.stlBoundaries();
@@ -110,6 +111,8 @@ public:
     void iterators();
 
     void invalidContainer();
+    
+    void invalidContainerLoop();
 
     bool checkIteratorPair(const Token* tok1, const Token* tok2);
 
@@ -205,6 +208,7 @@ private:
     void checkFindInsertError(const Token *tok);
     void sizeError(const Token* tok);
     void redundantIfRemoveError(const Token* tok);
+    void invalidContainerLoopError(const Token *tok, const Token * loopTok);
     void invalidContainerError(const Token *tok, const Token * contTok, const ValueFlow::Value *val, ErrorPath errorPath);
     void invalidContainerReferenceError(const Token* tok, const Token* contTok, ErrorPath errorPath);
 
@@ -228,6 +232,7 @@ private:
         c.iteratorsError(nullptr, "container1", "container2");
         c.iteratorsError(nullptr, nullptr, "container0", "container1");
         c.iteratorsError(nullptr, nullptr, "container");
+        c.invalidContainerLoopError(nullptr, nullptr);
         c.invalidContainerError(nullptr, nullptr, nullptr, errorPath);
         c.mismatchingContainersError(nullptr, nullptr);
         c.mismatchingContainerExpressionError(nullptr, nullptr);

test/teststl.cpp

@@ -161,6 +161,7 @@ private:
         TEST_CASE(loopAlgoMinMax);
 
         TEST_CASE(invalidContainer);
+        TEST_CASE(invalidContainerLoop);
         TEST_CASE(findInsert);
     }
 
@@ -4124,6 +4125,19 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void invalidContainerLoop() {
+        // #9435
+        check("void f(std::vector<int> v) {\n"
+              "    for (auto i : v) {\n"
+              "        if (i < 5)\n"
+              "            v.push_back(i * 2);\n"
+              "    }\n"
+              "}\n",
+              true);
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (error) Calling 'push_back' while iterating the container is invalid.\n", errout.str());
+
+    }
+
     void findInsert() {
         check("void f1(std::set<unsigned>& s, unsigned x) {\n"
               "    if (s.find(x) == s.end()) {\n"