Fix #11093, #12377 FP mismatchingContainerIterator (#5911)

…r to get reference to container
danmar · Feb 9, 2024 · 503c109 · 503c109
1 parent 272028c
commit 503c109
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 6 deletions.
diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
@@ -686,11 +686,19 @@ void CheckStl::sameIteratorExpressionError(const Token *tok)
     reportError(tok, Severity::style, "sameIteratorExpression", "Same iterators expression are used for algorithm.", CWE664, Certainty::normal);
 }
 
-static const Token* getAddressContainer(const Token* tok)
+static std::vector<const Token*> getAddressContainer(const Token* tok)
 {
     if (Token::simpleMatch(tok, "[") && tok->astOperand1())
-        return tok->astOperand1();
-    return tok;
+        return { tok->astOperand1() };
+    std::vector<ValueFlow::Value> values = ValueFlow::getLifetimeObjValues(tok, /*inconclusive*/ false);
+    std::vector<const Token*> res;
+    for (const auto& v : values) {
+        if (v.tokvalue)
+            res.emplace_back(v.tokvalue);
+    }
+    if (res.empty())
+        res.emplace_back(tok);
+    return res;
 }
 
 static bool isSameIteratorContainerExpression(const Token* tok1,
@@ -701,10 +709,18 @@ static bool isSameIteratorContainerExpression(const Token* tok1,
     if (isSameExpression(true, false, tok1, tok2, library, false, false)) {
         return !astIsContainerOwned(tok1) || !isTemporary(true, tok1, &library);
     }
-    if (kind == ValueFlow::Value::LifetimeKind::Address) {
-        return isSameExpression(true, false, getAddressContainer(tok1), getAddressContainer(tok2), library, false, false);
+    if (astContainerYield(tok2) == Library::Container::Yield::ITEM)
+        return true;
+    if (kind == ValueFlow::Value::LifetimeKind::Address || kind == ValueFlow::Value::LifetimeKind::Iterator) {
+        const auto address1 = getAddressContainer(tok1);
+        const auto address2 = getAddressContainer(tok2);
+        return std::any_of(address1.begin(), address1.end(), [&](const Token* tok1) {
+            return std::any_of(address2.begin(), address2.end(), [&](const Token* tok2) {
+                return isSameExpression(true, false, tok1, tok2, library, false, false);
+            });
+        });
     }
-    return astContainerYield(tok2) == Library::Container::Yield::ITEM;
+    return false;
 }
 
 static ValueFlow::Value getLifetimeIteratorValue(const Token* tok, MathLib::bigint path = 0)
@@ -866,6 +882,8 @@ void CheckStl::mismatchingContainerIterator()
             ValueFlow::Value val = getLifetimeIteratorValue(iterTok);
             if (!val.tokvalue)
                 continue;
+            if (!val.isKnown() && Token::simpleMatch(val.tokvalue->astParent(), ":"))
+                continue;
             if (val.lifetimeKind != ValueFlow::Value::LifetimeKind::Iterator)
                 continue;
             if (iterTok->str() == "*" && iterTok->astOperand1()->valueType() && iterTok->astOperand1()->valueType()->type == ValueType::ITERATOR)

diff --git a/test/teststl.cpp b/test/teststl.cpp
@@ -2134,6 +2134,51 @@ class TestStl : public TestFixture {
               "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        // #11093
+        check("struct S {\n"
+              "    std::vector<int> v1, v2;\n"
+              "    void f(bool b) {\n"
+              "        std::vector<int>& v = b ? v1 : v2;\n"
+              "        v.erase(v.begin());\n"
+              "    }\n"
+              "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // #12377
+        check("void f(bool b) {\n"
+              "    std::vector<int> *pv;\n"
+              "    if (b) {\n"
+              "        std::vector<int>& r = get1();\n"
+              "        pv = &r;\n"
+              "    }\n"
+              "    else {\n"
+              "        std::vector<int>& r = get2();\n"
+              "        pv = &r;\n"
+              "    }\n"
+              "    std::vector<int>::iterator it = pv->begin();\n"
+              "    it = pv->erase(it, it + 2);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("struct S {\n"
+              "    std::vector<int> v;\n"
+              "    void f() {\n"
+              "        std::vector<int>* p = &v;\n"
+              "        p->insert(std::find(p->begin(), p->end(), 0), 1);\n"
+              "    }\n"
+              "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("struct S {\n"
+              "    std::vector<int> v;\n"
+              "    void f(int i) {\n"
+              "        std::vector<int>* p = &v;\n"
+              "        if (p->size() > i)\n"
+              "            p->erase(p->begin() + i, p->end());\n"
+              "    }\n"
+              "};\n");
+        ASSERT_EQUALS("", errout.str());
+
         // #11067
         check("struct S {\n"
               "    std::vector<int> v;\n"