Improve STL iterators checking (#1380)

* Improve STL interators checking

* Improve error messages for container iterators from different scopes

* Mini refactoring

* Replace hardcoded pattern to ValueType::Type::ITERATOR

* Error messages improvements, more tests and refactoring

* Refactoring after code review

* Put getting operand data into separate function

* Update getErrorMessages and iterator errors ids

* Refactoring

* Fix error

* Refactoring, early return implementation

* Delete redundant code

* Tiny changes in comments

lib/checkstl.cpp

@@ -29,6 +29,7 @@
 
 #include <cstddef>
 #include <list>
+#include <map>
 #include <set>
 #include <sstream>
 #include <utility>
@@ -158,12 +159,46 @@ void CheckStl::invalidIteratorError(const Token *tok, const std::string &iterato
     reportError(tok, Severity::error, "invalidIterator1", "$symbol:"+iteratorName+"\nInvalid iterator: $symbol", CWE664, false);
 }
 
-void CheckStl::iteratorsError(const Token *tok, const std::string &container1, const std::string &container2)
+void CheckStl::iteratorsError(const Token* tok, const std::string& containerName1, const std::string& containerName2)
 {
-    reportError(tok, Severity::error, "iterators",
+    reportError(tok, Severity::error, "iterators1",
-                "$symbol:" + container1 + "\n"
+                "$symbol:" + containerName1 + "\n"
-                "$symbol:" + container2 + "\n"
+                "$symbol:" + containerName2 + "\n"
-                "Same iterator is used with different containers '" + container1 + "' and '" + container2 + "'.", CWE664, false);
+                "Same iterator is used with different containers '" + containerName1 + "' and '" + containerName2 + "'.", CWE664, false);
+}
+
+void CheckStl::iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName1, const std::string& containerName2)
+{
+    std::list<const Token*> callstack = { tok, containerTok };
+    reportError(callstack, Severity::error, "iterators2",
+                "$symbol:" + containerName1 + "\n"
+                "$symbol:" + containerName2 + "\n"
+                "Same iterator is used with different containers '" + containerName1 + "' and '" + containerName2 + "'.", CWE664, false);
+}
+
+void CheckStl::iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName)
+{
+    std::list<const Token*> callstack = { tok, containerTok };
+    reportError(callstack, Severity::error, "iterators3",
+                "$symbol:" + containerName + "\n"
+                "Same iterator is used with containers '" + containerName + "' that are defined in different scopes.", CWE664, false);
+}
+
+void CheckStl::iteratorsCmpError(const Token* cmpOperatorTok, const Token* containerTok1, const Token* containerTok2, const std::string& containerName1, const std::string& containerName2)
+{
+    std::list<const Token*> callstack = { cmpOperatorTok, containerTok1, containerTok2 };
+    reportError(callstack, Severity::error, "iteratorsCmp1",
+                "$symbol:" + containerName1 + "\n"
+                "$symbol:" + containerName2 + "\n"
+                "Comparison of iterators from containers '" + containerName1 + "' and '" + containerName2 + "'.", CWE664, false);
+}
+
+void CheckStl::iteratorsCmpError(const Token* cmpOperatorTok, const Token* containerTok1, const Token* containerTok2, const std::string& containerName)
+{
+    std::list<const Token*> callstack = { cmpOperatorTok, containerTok1, containerTok2 };
+    reportError(callstack, Severity::error, "iteratorsCmp2",
+                "$symbol:" + containerName + "\n"
+                "Comparison of iterators from containers '" + containerName + "' that are defined in different scopes.", CWE664, false);
 }
 
 // Error message used when dereferencing an iterator that has been erased..
@@ -229,10 +264,52 @@ static std::string getContainerName(const Token *containerToken)
     return ret;
 }
 
+enum OperandPosition
+{
+    Left,
+    Right
+};
+
+static const Token* findIteratorContainer(const Token* start, const Token* end, unsigned int id)
+{
+    const Token* containerToken = nullptr;
+    for(const Token* tok = start; tok != end; tok = tok->next())
+    {
+        if (Token::Match(tok, "%varid% = %name% . %name% (", id))
+        {
+            // Iterator is assigned to value
+            if (tok->tokAt(5)->valueType() && tok->tokAt(5)->valueType()->type == ValueType::Type::ITERATOR)
+            {
+                containerToken = tok->tokAt(2);
+            }
+        }
+        else if (Token::Match(tok, "%varid% = %name% (", id))
+        {
+            // Prevent FP: iterator is assigned to something
+            // TODO: Fix it in future
+            containerToken = nullptr;
+        }
+    }
+    return containerToken;
+}
+
 void CheckStl::iterators()
 {
     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
 
+    // Filling map of iterators id and their scope begin
+    std::map<unsigned int, const Token*> iteratorScopeBeginInfo;
+    for (const Variable* var : symbolDatabase->variableList()) {
+        bool inconclusiveType=false;
+        if (!isIterator(var, inconclusiveType))
+            continue;
+        const unsigned int iteratorId = var->declarationId();
+        if (iteratorId != 0)
+            iteratorScopeBeginInfo[iteratorId] = var->nameToken();
+    }
+    // Storage to save found comparison problems to avoid duplicate error messages
+    std::set<const Token*> foundOperatorErrors;
+
     for (const Variable* var : symbolDatabase->variableList()) {
         bool inconclusiveType=false;
         if (!isIterator(var, inconclusiveType))
@@ -269,15 +346,12 @@ void CheckStl::iterators()
                 invalidationScope = nullptr;
             }
 
-            // Is iterator compared against different container?
+            // Is comparison expression?
-            if (tok2->isComparisonOp() && containerToken && tok2->astOperand1() && tok2->astOperand2()) {
+            // Check whether iterator compared against different container or iterator of different container?
-                const Token *other = nullptr;
+            if (tok2->isComparisonOp() && tok2->astOperand1() && tok2->astOperand2() &&
-                if (tok2->astOperand1()->varId() == iteratorId)
+                (foundOperatorErrors.find(tok2) == foundOperatorErrors.end()) &&
-                    other = tok2->astOperand2()->tokAt(-3);
+                compareIteratorAgainstDifferentContainer(tok2, containerToken, iteratorId, iteratorScopeBeginInfo)) {
-                else if (tok2->astOperand2()->varId() == iteratorId)
+                foundOperatorErrors.insert(tok2);
-                    other = tok2->astOperand1()->tokAt(-3);
-                if (Token::Match(other, "%name% . end|rend|cend|crend ( )") && other->varId() != containerToken->varId())
-                    iteratorsError(tok2, getContainerName(containerToken), getContainerName(other));
             }
 
             // Is the iterator used in a insert/erase operation?
@@ -397,6 +471,74 @@ void CheckStl::iterators()
     }
 }
 
+bool CheckStl::compareIteratorAgainstDifferentContainer(const Token* operatorTok, const Token* containerTok, const unsigned int iteratorId, const std::map<unsigned int, const Token*>& iteratorScopeBeginInfo)
+{
+    if (!containerTok)
+        return false;
+
+    const Token *otherOperand = nullptr;
+    OperandPosition operandPosition;
+    if (operatorTok->astOperand1()->varId() == iteratorId)
+    {
+        otherOperand = operatorTok->astOperand2();
+        operandPosition = OperandPosition::Right;
+    }
+    else if (operatorTok->astOperand2()->varId() == iteratorId)
+    {
+        otherOperand = operatorTok->astOperand1();
+        operandPosition = OperandPosition::Left;
+    }
+
+    if (!otherOperand)
+        return false;
+
+    const Token * const otherExprPart = otherOperand->tokAt(-3);
+    if (Token::Match(otherExprPart, "%name% . end|rend|cend|crend ( )") && otherExprPart->varId() != containerTok->varId())
+    {
+        const std::string& firstContainerName = getContainerName(containerTok);
+        const std::string& secondContainerName = getContainerName(otherExprPart);
+        if (firstContainerName != secondContainerName)
+        {
+            if (operandPosition == OperandPosition::Right)
+                iteratorsError(operatorTok, containerTok, firstContainerName, secondContainerName);
+            else
+                iteratorsError(operatorTok, containerTok, secondContainerName, firstContainerName);
+        }
+        else
+        {
+            iteratorsError(operatorTok, containerTok, firstContainerName);
+        }
+        return true;
+    }
+    else
+    {
+        const unsigned int otherId = otherOperand->varId();
+        auto it = iteratorScopeBeginInfo.find(otherId);
+        if (it != iteratorScopeBeginInfo.end())
+        {
+            const Token* otherContainerToken = findIteratorContainer(it->second, operatorTok->astOperand1(), otherId);
+            if (otherContainerToken && otherContainerToken->varId() != containerTok->varId())
+            {
+                const std::string& firstContainerName = getContainerName(containerTok);
+                const std::string& secondContainerName = getContainerName(otherContainerToken);
+                if (firstContainerName != secondContainerName)
+                {
+                    if (operandPosition == OperandPosition::Right)
+                        iteratorsCmpError(operatorTok, containerTok, otherContainerToken, firstContainerName, secondContainerName);
+                    else
+                        iteratorsCmpError(operatorTok, containerTok, otherContainerToken, secondContainerName, firstContainerName);
+                }
+                else
+                {
+                    iteratorsCmpError(operatorTok, containerTok, otherContainerToken, firstContainerName);
+                }
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
 
 // Error message for bad iterator usage..
 void CheckStl::mismatchingContainersError(const Token *tok)

lib/checkstl.h

@@ -195,7 +195,11 @@ private:
     void stlOutOfBoundsError(const Token* tok, const std::string& num, const std::string& var, bool at);
     void negativeIndexError(const Token* tok, const ValueFlow::Value& index);
     void invalidIteratorError(const Token* tok, const std::string& iteratorName);
-    void iteratorsError(const Token* tok, const std::string& container1, const std::string& container2);
+    void iteratorsError(const Token* tok, const std::string& containerName1, const std::string& containerName2);
+    void iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName1, const std::string& containerName2);
+    void iteratorsError(const Token* tok, const Token* containerTok, const std::string& containerName);
+    void iteratorsCmpError(const Token* cmpOperatorTok, const Token* containerTok1, const Token* containerTok2, const std::string& containerName1, const std::string& containerName2);    
+    void iteratorsCmpError(const Token* cmpOperatorTok, const Token* containerTok1, const Token* containerTok2, const std::string& containerName);
     void mismatchingContainersError(const Token* tok);
     void mismatchingContainerExpressionError(const Token *tok1, const Token *tok2);
     void sameIteratorExpressionError(const Token *tok);
@@ -223,11 +227,17 @@ private:
 
     void useStlAlgorithmError(const Token *tok, const std::string &algoName);
 
+    bool compareIteratorAgainstDifferentContainer(const Token* tok, const Token* containerToken, const unsigned int iteratorId, const std::map<unsigned int, const Token*>& iteratorScopeBeginInfo);
+
     void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const override {
         CheckStl c(nullptr, settings, errorLogger);
         c.outOfBoundsError(nullptr, nullptr, nullptr);
         c.invalidIteratorError(nullptr, "iterator");
         c.iteratorsError(nullptr, "container1", "container2");
+        c.iteratorsError(nullptr, nullptr, "container1", "container2");
+        c.iteratorsError(nullptr, nullptr, "container");
+        c.iteratorsCmpError(nullptr, nullptr, nullptr, "container1", "container2");
+        c.iteratorsCmpError(nullptr, nullptr, nullptr, "container");
         c.mismatchingContainersError(nullptr);
         c.mismatchingContainerExpressionError(nullptr, nullptr);
         c.sameIteratorExpressionError(nullptr);

test/teststl.cpp

@@ -57,6 +57,12 @@ private:
         TEST_CASE(iterator13);
         TEST_CASE(iterator14); // #8191
         TEST_CASE(iterator15); // #8341
+        TEST_CASE(iterator16);
+        TEST_CASE(iterator17);
+        TEST_CASE(iterator18);
+        TEST_CASE(iterator19);
+        TEST_CASE(iterator20);
+        TEST_CASE(iterator21);
         TEST_CASE(iteratorExpression);
         TEST_CASE(iteratorSameExpression);
 
@@ -246,7 +252,7 @@ private:
               "    for (list<int>::iterator it = l1.begin(); it != l2.end(); ++it)\n"
               "    { }\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
 
         check("void f()\n"
               "{\n"
@@ -255,7 +261,7 @@ private:
               "    for (list<int>::iterator it = l1.begin(); l2.end() != it; ++it)\n"
               "    { }\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:5]: (error) Same iterator is used with different containers 'l2' and 'l1'.\n", errout.str());
 
         check("struct C { std::list<int> l1; void func(); };\n"
               "void C::func() {\n"
@@ -274,7 +280,7 @@ private:
               "    for (list<int>::const_reverse_iterator it = l1.rbegin(); it != l2.rend(); ++it)\n"
               "    { }\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
     }
 
     void iterator2() {
@@ -288,7 +294,19 @@ private:
               "        ++it;\n"
               "    }\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:6]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:5]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    list<int> l1;\n"
+              "    list<int> l2;\n"
+              "    list<int>::iterator it = l1.begin();\n"
+              "    while (l2.end() != it)\n"
+              "    {\n"
+              "        ++it;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:5]: (error) Same iterator is used with different containers 'l2' and 'l1'.\n", errout.str());
     }
 
     void iterator3() {
@@ -512,7 +530,7 @@ private:
               "        if (it != s2.end()) continue;\n"
               "    }\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:8]: (error) Same iterator is used with different containers 's1' and 's2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:5]: (error) Same iterator is used with different containers 's1' and 's2'.\n", errout.str());
     }
 
     void iterator11() {
@@ -534,7 +552,7 @@ private:
               "    std::map<int, int>::const_iterator it = map1.find(123);\n"
               "    if (it == map2.end()) { }"
               "}");
-        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'map1' and 'map2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (error) Same iterator is used with different containers 'map1' and 'map2'.\n", errout.str());
 
         check("void f() {\n"
               "    std::map<int, int> map1;\n"
@@ -542,7 +560,7 @@ private:
               "    std::map<int, int>::const_iterator it = map1.find(123);\n"
               "    if (map2.end() == it) { }"
               "}");
-        ASSERT_EQUALS("[test.cpp:5]: (error) Same iterator is used with different containers 'map1' and 'map2'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (error) Same iterator is used with different containers 'map2' and 'map1'.\n", errout.str());
 
         check("void f(std::string &s) {\n"
               "    int pos = s.find(x);\n"
@@ -564,7 +582,7 @@ private:
               "    while (it!=a.end())\n"
               "        ++it;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:9]: (error) Same iterator is used with different containers 't' and 'a'.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:8]: (error) Same iterator is used with different containers 't' and 'a'.\n", errout.str());
 
         // #4062
         check("void f() {\n"
@@ -613,6 +631,286 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void iterator16() {
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l2.end();\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:5] -> [test.cpp:6]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.end();\n"
+              "    std::list<int>::iterator it2 = l2.begin();\n"
+              "    while (it2 != it1)\n"
+              "    {\n"
+              "        ++it2;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6] -> [test.cpp:5]: (error) Comparison of iterators from containers 'l2' and 'l1'.\n", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it2 = l2.end();\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6] -> [test.cpp:5]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2(10, 4);\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l2.find(4);\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6] -> [test.cpp:5]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", errout.str());
+
+    }
+
+    void iterator17() {
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l1.end();\n"
+              "    { it2 = l2.end(); }\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:5] -> [test.cpp:7]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l1.end();\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "    it2 = l2.end();\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l1.end();\n"
+              "    it1 = l2.end();\n"
+              "    it1 = l1.end();\n"
+              "    if (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l1.end();\n"
+              "    { it2 = l2.end(); }\n"
+              "    it2 = l1.end();\n"
+              "    { it2 = l2.end(); }\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:10] -> [test.cpp:5] -> [test.cpp:9]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", errout.str());
+    }
+
+    void iterator18() {
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int> l2;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    std::list<int>::iterator it2 = l1.end();\n"
+            "    while (++it1 != --it2)\n"
+            "    {\n"
+            "    }\n"
+            "}");
+      ASSERT_EQUALS("", errout.str());
+
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int> l2;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    std::list<int>::iterator it2 = l1.end();\n"
+            "    while (it1++ != --it2)\n"
+            "    {\n"
+            "    }\n"
+            "}");
+      ASSERT_EQUALS("", errout.str());
+
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int> l2;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    std::list<int>::iterator it2 = l1.end();\n"
+            "    if (--it2 > it1++)\n"
+            "    {\n"
+            "    }\n"
+            "}");
+      TODO_ASSERT_EQUALS("", "[test.cpp:7]: (error) Dangerous comparison using operator< on iterator.\n", errout.str());
+    }
+
+    void iterator19() {
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    {\n"
+            "        std::list<int> l1;\n"
+            "        if (it1 != l1.end())\n"
+            "        {\n"
+            "        }\n"
+            "    }\n"
+            "}");
+      ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n", errout.str());
+
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    {\n"
+            "        std::list<int> l1;\n"
+            "        if (l1.end() > it1)\n"
+            "        {\n"
+            "        }\n"
+            "    }\n"
+            "}");
+      TODO_ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n",
+                         "[test.cpp:7] -> [test.cpp:4]: (error) Same iterator is used with containers 'l1' that are defined in different scopes.\n[test.cpp:7]: (error) Dangerous comparison using operator< on iterator.\n",
+                         errout.str());
+
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    {\n"
+            "        std::list<int> l1;\n"
+            "        std::list<int>::iterator it2 = l1.begin();\n"
+            "        if (it1 != it2)\n"
+            "        {\n"
+            "        }\n"
+            "    }\n"
+            "}");
+      ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:4] -> [test.cpp:7]: (error) Comparison of iterators from containers 'l1' that are defined in different scopes.\n", errout.str());
+
+      check("void foo()\n"
+            "{\n"
+            "    std::list<int> l1;\n"
+            "    std::list<int>::iterator it1 = l1.begin();\n"
+            "    {\n"
+            "        std::list<int> l1;\n"
+            "        std::list<int>::iterator it2 = l1.begin();\n"
+            "        if (it2 != it1)\n"
+            "        {\n"
+            "        }\n"
+            "    }\n"
+            "}");
+      ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:4] -> [test.cpp:7]: (error) Comparison of iterators from containers 'l1' that are defined in different scopes.\n", errout.str());
+
+    }
+
+    void iterator20() {
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l2.begin();\n"
+              "    it1 = it2;\n"
+              "    while (it1 != l1.end())\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        TODO_ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:7]: (error) Same iterator is used with different containers 'l2' and 'l1'.\n", "", errout.str());
+
+        check("std::list<int> l3;\n"
+              "std::list<int>::iterator bar()\n"
+              "{\n"
+              "    return l3.end();\n"
+              "}\n"
+              "void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.begin();\n"
+              "    std::list<int>::iterator it2 = l2.begin();\n"
+              "    it1 = bar();\n"
+              "    while (it1 != it2)\n"
+              "    {\n"
+              "        ++it1;\n"
+              "    }\n"
+              "}");
+        TODO_ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:10] -> [test.cpp:11]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", "", errout.str());
+
+    }
+
+    void iterator21() {
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.end();\n"
+              "    std::list<int>::iterator it2 = l2.begin();\n"
+              "    if (it1 != it2)\n"
+              "    {\n"
+              "    }\n"
+              "    if (it2 != it1)\n"
+              "    {\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6] -> [test.cpp:5]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n"
+                      "[test.cpp:10] -> [test.cpp:6] -> [test.cpp:5]: (error) Comparison of iterators from containers 'l2' and 'l1'.\n", errout.str());
+
+        check("void foo()\n"
+              "{\n"
+              "    std::list<int> l1;\n"
+              "    std::list<int> l2;\n"
+              "    std::list<int>::iterator it1 = l1.end();\n"
+              "    std::list<int>::iterator it2 = l2.begin();\n"
+              "    if (it1 != it2 && it1 != it2)\n"
+              "    {\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6] -> [test.cpp:5]: (error) Comparison of iterators from containers 'l1' and 'l2'.\n", errout.str());
+
+    }
+
     void iteratorExpression() {
         check("std::vector<int>& f();\n"
               "std::vector<int>& g();\n"