Handle possible values

danmar · Jan 29, 2024 · 1834ef2 · 1834ef2
1 parent a5077f0
commit 1834ef2
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 12 deletions.
diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
@@ -3117,18 +3117,32 @@ void CheckStl::knownEmptyContainer()
     }
 }
 
-void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* itertok)
+void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* itertok, const ValueFlow::Value* val)
 {
     const std::string func = ftok ? ftok->str() : std::string("erase");
     const std::string iter = itertok ? itertok->expressionString() : std::string("it");
 
-    const std::string msg = "Calling function '" + func + "()' on the iterator '" + iter + "' which is out of bounds.";
+    const bool isConditional = val && val->isPossible();
+    std::string msg;
+    if (isConditional) {
+       msg = ValueFlow::eitherTheConditionIsRedundant(val->condition) + " or function '" + func + "()' is called on the iterator '" + iter + "' which is out of bounds.";
+    } else {
+        msg = "Calling function '" + func + "()' on the iterator '" + iter + "' which is out of bounds.";
+    }
 
-    reportError(ftok, Severity::error,
+    reportError(ftok, isConditional ? Severity::warning : Severity::error,
                 "eraseIteratorOutOfBounds",
                 msg, CWE398, Certainty::normal);
 }
 
+static const ValueFlow::Value* getIterValue(const Token* tok, ValueFlow::Value::ValueType vt)
+{
+    auto it = std::find_if(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) {
+        return v.valueType == vt && (v.isPossible() || v.isKnown());
+    });
+    return it != tok->values().end() ? &*it : nullptr;
+}
+
 void CheckStl::eraseIteratorOutOfBounds()
 {
     logChecker("CheckStl::eraseIteratorOutOfBounds");
@@ -3149,20 +3163,20 @@ void CheckStl::eraseIteratorOutOfBounds()
             if (args.size() != 1) // empty range is ok
                 continue;
 
-            bool error = false;
-            if (const ValueFlow::Value* endVal = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END)) {
+            const ValueFlow::Value* errVal = nullptr;
+            if (const ValueFlow::Value* endVal = getIterValue(args[0], ValueFlow::Value::ValueType::ITERATOR_END)) {
                 if (endVal->intvalue >= 0)
-                    error = true;
+                    errVal = endVal;
             }
-            else if (const ValueFlow::Value* startVal = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) {
+            else if (const ValueFlow::Value* startVal = getIterValue(args[0], ValueFlow::Value::ValueType::ITERATOR_START)) {
                 if (startVal->intvalue < 0)
-                    error = true;
+                    errVal = startVal;
                 else if (const ValueFlow::Value* sizeVal = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
                     if (startVal->intvalue >= sizeVal->intvalue)
-                        error = true;
+                        errVal = startVal;
             }
-            if (error)
-                eraseIteratorOutOfBoundsError(tok->tokAt(2), args[0]);
+            if (errVal)
+                eraseIteratorOutOfBoundsError(tok->tokAt(2), args[0], errVal);
         }
     }
 }

diff --git a/lib/checkstl.h b/lib/checkstl.h
@@ -237,7 +237,7 @@ class CPPCHECKLIB CheckStl : public Check {
 
     void knownEmptyContainerError(const Token *tok, const std::string& algo);
 
-    void eraseIteratorOutOfBoundsError(const Token* ftok, const Token* itertok);
+    void eraseIteratorOutOfBoundsError(const Token* ftok, const Token* itertok, const ValueFlow::Value* val = nullptr);
 
     void globalLockGuardError(const Token *tok);
     void localMutexError(const Token *tok);

diff --git a/test/teststl.cpp b/test/teststl.cpp
@@ -2185,6 +2185,13 @@ class TestStl : public TestFixture {
         ASSERT_EQUALS("[test.cpp:3]: (error) Calling function 'erase()' on the iterator 'v.begin()-1' which is out of bounds.\n"
                       "[test.cpp:3]: (error) Dereference of an invalid iterator: v.begin()-1\n",
                       errout.str());
+
+        check("void f(std::vector<int>& v, std::vector<int>::iterator it) {\n"
+              "    if (it == v.end()) {}\n"
+              "    v.erase(it);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Either the condition 'it==v.end()' is redundant or function 'erase()' is called on the iterator 'it' which is out of bounds.\n",
+                      errout.str());
     }
 
     // Dereferencing invalid pointer