From 2691408acfb0e3b5f0bf859fc3614047f99f6ee0 Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Wed, 24 Jan 2024 16:45:29 +0100
Subject: [PATCH 01/15] New check: erasing end()

---
 lib/checkstl.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++
 lib/checkstl.h   |  5 +++++
 test/teststl.cpp | 40 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index f1e7c01d23c..60aedd4b0d6 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3117,6 +3117,49 @@ void CheckStl::knownEmptyContainer()
     }
 }
 
+void CheckStl::eraseEndIteratorError(const Token *ftok, const Token* itertok)
+{
+    const std::string func = ftok ? ftok->str() : std::string("erase");
+    const std::string iter = itertok ? itertok->expressionString() : std::string("it");
+
+    const std::string msg = "Function '" + func + "()' must not be called on the end iterator '" + iter + "'.";
+
+    reportError(ftok, Severity::error,
+                "eraseEndIterator",
+                msg, CWE398, Certainty::normal);
+}
+
+void CheckStl::eraseEndIterator()
+{
+    logChecker("CheckStl::eraseEndIterator");
+    for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
+        for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
+
+            if (!tok->valueType())
+                continue;
+            const Library::Container* container = tok->valueType()->container;
+            if (!container)
+                continue;
+            if (!Token::Match(tok->next(), ". %name% ("))
+                continue;
+            const Library::Container::Action action = container->getAction(tok->strAt(2));
+            if (action != Library::Container::Action::ERASE)
+                continue;
+            const std::vector<const Token*> args = getArguments(tok->tokAt(2));
+            if (args.size() != 1) // empty range is ok
+                continue;
+
+            if (args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END))
+                eraseEndIteratorError(tok->tokAt(2), args[0]);
+            else if (args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) {
+                if (const ValueFlow::Value* size = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
+                    if (size->intvalue == 0)
+                        eraseEndIteratorError(tok->tokAt(2), args[0]);
+            }
+        }
+    }
+}
+
 static bool isMutex(const Variable* var)
 {
     const Token* tok = Token::typeDecl(var->nameToken()).first;
diff --git a/lib/checkstl.h b/lib/checkstl.h
index 6694c3cdd16..d191d887056 100644
--- a/lib/checkstl.h
+++ b/lib/checkstl.h
@@ -78,6 +78,7 @@ class CPPCHECKLIB CheckStl : public Check {
         checkStl.mismatchingContainers();
         checkStl.mismatchingContainerIterator();
         checkStl.knownEmptyContainer();
+        checkStl.eraseEndIterator();
 
         checkStl.stlBoundaries();
         checkStl.checkDereferenceInvalidIterator();
@@ -183,6 +184,8 @@ class CPPCHECKLIB CheckStl : public Check {
 
     void knownEmptyContainer();
 
+    void eraseEndIterator();
+
     void checkMutexes();
 
     bool isContainerSize(const Token *containerToken, const Token *expr) const;
@@ -234,6 +237,8 @@ class CPPCHECKLIB CheckStl : public Check {
 
     void knownEmptyContainerError(const Token *tok, const std::string& algo);
 
+    void eraseEndIteratorError(const Token* ftok, const Token* itertok);
+
     void globalLockGuardError(const Token *tok);
     void localMutexError(const Token *tok);
 
diff --git a/test/teststl.cpp b/test/teststl.cpp
index f02afead149..e7d75b2b850 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -74,6 +74,7 @@ class TestStl : public TestFixture {
         TEST_CASE(iteratorExpression);
         TEST_CASE(iteratorSameExpression);
         TEST_CASE(mismatchingContainerIterator);
+        TEST_CASE(eraseEndIterator);
 
         TEST_CASE(dereference);
         TEST_CASE(dereference_break);  // #3644 - handle "break"
@@ -2137,6 +2138,37 @@ class TestStl : public TestFixture {
         ASSERT_EQUALS("", errout.str());
     }
 
+    void eraseEndIterator() {
+        check("void f() {\n"
+              "    std::vector<int> v;\n"
+              "    v.erase(v.begin());\n"
+              "}\n"
+              "void g() {\n"
+              "    std::vector<int> v;\n"
+              "    v.erase(v.end());\n"
+              "}\n"
+              "void h() {\n"
+              "    std::vector<int> v;\n"
+              "    auto it = v.begin();\n"
+              "    v.erase(it);\n"
+              "}\n"
+              "void j() {\n"
+              "    std::vector<int> v{ 1, 2, 3 };\n"
+              "    auto it = v.end();\n"
+              "    v.erase(it);\n"
+              "}\n"
+              "void k() {\n"
+              "    std::vector<int> v{ 1, 2, 3 };\n"
+              "    auto it = v.begin();\n"
+              "    v.erase(it);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the end iterator 'v.begin()'.\n"
+                      "[test.cpp:7]: (error) Function 'erase()' must not be called on the end iterator 'v.end()'.\n"
+                      "[test.cpp:12]: (error) Function 'erase()' must not be called on the end iterator 'it'.\n"
+                      "[test.cpp:17]: (error) Function 'erase()' must not be called on the end iterator 'it'.\n",
+                      errout.str());
+    }
+
     // Dereferencing invalid pointer
     void dereference() {
         check("void f()\n"
@@ -2196,7 +2228,9 @@ class TestStl : public TestFixture {
               "    ints.erase(iter);\n"
               "    std::cout << iter->first << std::endl;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n"
+                      "[test.cpp:6]: (error) Function 'erase()' must not be called on the end iterator 'iter'.\n",
+                      errout.str());
 
         // Reverse iterator
         check("void f()\n"
@@ -2207,7 +2241,9 @@ class TestStl : public TestFixture {
               "    ints.erase(iter);\n"
               "    std::cout << iter->first << std::endl;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n"
+                      "[test.cpp:6]: (error) Function 'erase()' must not be called on the end iterator 'iter'.\n",
+                      errout.str());
     }
 
     void dereference_auto() {

From 99e540d030f3afaa94615c785055596a8b87053c Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Wed, 24 Jan 2024 17:56:24 +0100
Subject: [PATCH 02/15] Fix FP

---
 lib/checkstl.cpp | 10 ++++++----
 test/teststl.cpp |  4 ++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 60aedd4b0d6..0a26029e64d 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3149,11 +3149,13 @@ void CheckStl::eraseEndIterator()
             if (args.size() != 1) // empty range is ok
                 continue;
 
-            if (args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END))
-                eraseEndIteratorError(tok->tokAt(2), args[0]);
+            if (const ValueFlow::Value* itValue = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END)) {
+                if (itValue->intvalue == 0)
+                    eraseEndIteratorError(tok->tokAt(2), args[0]);
+            }
             else if (args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) {
-                if (const ValueFlow::Value* size = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
-                    if (size->intvalue == 0)
+                if (const ValueFlow::Value* sizeValue = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
+                    if (sizeValue->intvalue == 0)
                         eraseEndIteratorError(tok->tokAt(2), args[0]);
             }
         }
diff --git a/test/teststl.cpp b/test/teststl.cpp
index e7d75b2b850..49d43422db1 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -2161,6 +2161,10 @@ class TestStl : public TestFixture {
               "    std::vector<int> v{ 1, 2, 3 };\n"
               "    auto it = v.begin();\n"
               "    v.erase(it);\n"
+              "}\n"
+              "void m() {\n"
+              "    std::vector<int> v{ 1, 2, 3 };\n"
+              "    v.erase(v.end() - 1);\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the end iterator 'v.begin()'.\n"
                       "[test.cpp:7]: (error) Function 'erase()' must not be called on the end iterator 'v.end()'.\n"

From 9b2fdc70a5cec740f8e69ac89f7ca25a376451dd Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Mon, 29 Jan 2024 14:19:06 +0100
Subject: [PATCH 03/15] Check offset

---
 lib/checkstl.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 0a26029e64d..65887f6829f 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3149,14 +3149,15 @@ void CheckStl::eraseEndIterator()
             if (args.size() != 1) // empty range is ok
                 continue;
 
-            if (const ValueFlow::Value* itValue = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END)) {
-                if (itValue->intvalue == 0)
+            if (const ValueFlow::Value* endVal = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_END)) {
+                if (endVal->intvalue == 0)
                     eraseEndIteratorError(tok->tokAt(2), args[0]);
             }
-            else if (args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) {
-                if (const ValueFlow::Value* sizeValue = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
-                    if (sizeValue->intvalue == 0)
-                        eraseEndIteratorError(tok->tokAt(2), args[0]);
+            else if (const ValueFlow::Value* startVal = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) {
+                if (startVal->intvalue == 0)
+                    if (const ValueFlow::Value* sizeVal = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
+                        if (sizeVal->intvalue == 0)
+                            eraseEndIteratorError(tok->tokAt(2), args[0]);
             }
         }
     }

From a6d1bc1f13119d212c8721978692f3ee924bbb46 Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Mon, 29 Jan 2024 17:52:04 +0100
Subject: [PATCH 04/15] Address reviewer comments, generalize

---
 lib/checkstl.cpp | 26 +++++++++++++++-----------
 lib/checkstl.h   |  6 +++---
 test/teststl.cpp | 46 ++++++++++++++++++++++++++++++----------------
 3 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 65887f6829f..49ec2f2db88 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3117,21 +3117,21 @@ void CheckStl::knownEmptyContainer()
     }
 }
 
-void CheckStl::eraseEndIteratorError(const Token *ftok, const Token* itertok)
+void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* itertok)
 {
     const std::string func = ftok ? ftok->str() : std::string("erase");
     const std::string iter = itertok ? itertok->expressionString() : std::string("it");
 
-    const std::string msg = "Function '" + func + "()' must not be called on the end iterator '" + iter + "'.";
+    const std::string msg = "Function '" + func + "()' must not be called on the iterator '" + iter + "' since it is out of bounds.";
 
     reportError(ftok, Severity::error,
-                "eraseEndIterator",
+                "eraseIteratorOutOfBounds",
                 msg, CWE398, Certainty::normal);
 }
 
-void CheckStl::eraseEndIterator()
+void CheckStl::eraseIteratorOutOfBounds()
 {
-    logChecker("CheckStl::eraseEndIterator");
+    logChecker("CheckStl::eraseIteratorOutOfBounds");
     for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
         for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
 
@@ -3149,16 +3149,20 @@ void CheckStl::eraseEndIterator()
             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)) {
-                if (endVal->intvalue == 0)
-                    eraseEndIteratorError(tok->tokAt(2), args[0]);
+                if (endVal->intvalue >= 0)
+                    error = true;
             }
             else if (const ValueFlow::Value* startVal = args[0]->getKnownValue(ValueFlow::Value::ValueType::ITERATOR_START)) {
-                if (startVal->intvalue == 0)
-                    if (const ValueFlow::Value* sizeVal = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
-                        if (sizeVal->intvalue == 0)
-                            eraseEndIteratorError(tok->tokAt(2), args[0]);
+                if (startVal->intvalue < 0)
+                    error = true;
+                else if (const ValueFlow::Value* sizeVal = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
+                    if (startVal->intvalue >= sizeVal->intvalue)
+                        error = true;
             }
+            if (error)
+                eraseIteratorOutOfBoundsError(tok->tokAt(2), args[0]);
         }
     }
 }
diff --git a/lib/checkstl.h b/lib/checkstl.h
index d191d887056..d0dae64b18b 100644
--- a/lib/checkstl.h
+++ b/lib/checkstl.h
@@ -78,7 +78,7 @@ class CPPCHECKLIB CheckStl : public Check {
         checkStl.mismatchingContainers();
         checkStl.mismatchingContainerIterator();
         checkStl.knownEmptyContainer();
-        checkStl.eraseEndIterator();
+        checkStl.eraseIteratorOutOfBounds();
 
         checkStl.stlBoundaries();
         checkStl.checkDereferenceInvalidIterator();
@@ -184,7 +184,7 @@ class CPPCHECKLIB CheckStl : public Check {
 
     void knownEmptyContainer();
 
-    void eraseEndIterator();
+    void eraseIteratorOutOfBounds();
 
     void checkMutexes();
 
@@ -237,7 +237,7 @@ class CPPCHECKLIB CheckStl : public Check {
 
     void knownEmptyContainerError(const Token *tok, const std::string& algo);
 
-    void eraseEndIteratorError(const Token* ftok, const Token* itertok);
+    void eraseIteratorOutOfBoundsError(const Token* ftok, const Token* itertok);
 
     void globalLockGuardError(const Token *tok);
     void localMutexError(const Token *tok);
diff --git a/test/teststl.cpp b/test/teststl.cpp
index 49d43422db1..2dc806fa6ff 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -74,7 +74,7 @@ class TestStl : public TestFixture {
         TEST_CASE(iteratorExpression);
         TEST_CASE(iteratorSameExpression);
         TEST_CASE(mismatchingContainerIterator);
-        TEST_CASE(eraseEndIterator);
+        TEST_CASE(eraseIteratorOutOfBounds);
 
         TEST_CASE(dereference);
         TEST_CASE(dereference_break);  // #3644 - handle "break"
@@ -2138,38 +2138,52 @@ class TestStl : public TestFixture {
         ASSERT_EQUALS("", errout.str());
     }
 
-    void eraseEndIterator() {
+    void eraseIteratorOutOfBounds() {
         check("void f() {\n"
               "    std::vector<int> v;\n"
               "    v.erase(v.begin());\n"
-              "}\n"
-              "void g() {\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the iterator 'v.begin()' since it is out of bounds.\n", errout.str());
+
+        check("void f() {\n"
               "    std::vector<int> v;\n"
               "    v.erase(v.end());\n"
-              "}\n"
-              "void h() {\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the iterator 'v.end()' since it is out of bounds.\n", errout.str());
+
+        check("void f() {\n"
               "    std::vector<int> v;\n"
               "    auto it = v.begin();\n"
               "    v.erase(it);\n"
-              "}\n"
-              "void j() {\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Function 'erase()' must not be called on the iterator 'it' since it is out of bounds.\n", errout.str());
+
+        check("void f() {\n"
               "    std::vector<int> v{ 1, 2, 3 };\n"
               "    auto it = v.end();\n"
               "    v.erase(it);\n"
-              "}\n"
-              "void k() {\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Function 'erase()' must not be called on the iterator 'it' since it is out of bounds.\n", errout.str());
+
+        check("void f() {\n"
               "    std::vector<int> v{ 1, 2, 3 };\n"
               "    auto it = v.begin();\n"
               "    v.erase(it);\n"
-              "}\n"
-              "void m() {\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
               "    std::vector<int> v{ 1, 2, 3 };\n"
               "    v.erase(v.end() - 1);\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the end iterator 'v.begin()'.\n"
-                      "[test.cpp:7]: (error) Function 'erase()' must not be called on the end iterator 'v.end()'.\n"
-                      "[test.cpp:12]: (error) Function 'erase()' must not be called on the end iterator 'it'.\n"
-                      "[test.cpp:17]: (error) Function 'erase()' must not be called on the end iterator 'it'.\n",
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    std::vector<int> v{ 1, 2, 3 };\n"
+              "    v.erase(v.begin() - 1);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the iterator 'v.begin()-1' since it is out of bounds.\n"
+                      "[test.cpp:3]: (error) Dereference of an invalid iterator: v.begin()-1\n",
                       errout.str());
     }
 

From a5077f01a979d8cd11c8ef5178b248b9b5418a8d Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Mon, 29 Jan 2024 18:01:20 +0100
Subject: [PATCH 05/15] Fix, tweak message

---
 lib/checkstl.cpp |  2 +-
 test/teststl.cpp | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 49ec2f2db88..1b1d2bbede5 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3122,7 +3122,7 @@ void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* ite
     const std::string func = ftok ? ftok->str() : std::string("erase");
     const std::string iter = itertok ? itertok->expressionString() : std::string("it");
 
-    const std::string msg = "Function '" + func + "()' must not be called on the iterator '" + iter + "' since it is out of bounds.";
+    const std::string msg = "Calling function '" + func + "()' on the iterator '" + iter + "' which is out of bounds.";
 
     reportError(ftok, Severity::error,
                 "eraseIteratorOutOfBounds",
diff --git a/test/teststl.cpp b/test/teststl.cpp
index 2dc806fa6ff..6522ae50884 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -2143,27 +2143,27 @@ class TestStl : public TestFixture {
               "    std::vector<int> v;\n"
               "    v.erase(v.begin());\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the iterator 'v.begin()' since it is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (error) Calling function 'erase()' on the iterator 'v.begin()' which is out of bounds.\n", errout.str());
 
         check("void f() {\n"
               "    std::vector<int> v;\n"
               "    v.erase(v.end());\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the iterator 'v.end()' since it is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (error) Calling function 'erase()' on the iterator 'v.end()' which is out of bounds.\n", errout.str());
 
         check("void f() {\n"
               "    std::vector<int> v;\n"
               "    auto it = v.begin();\n"
               "    v.erase(it);\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (error) Function 'erase()' must not be called on the iterator 'it' since it is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (error) Calling function 'erase()' on the iterator 'it' which is out of bounds.\n", errout.str());
 
         check("void f() {\n"
               "    std::vector<int> v{ 1, 2, 3 };\n"
               "    auto it = v.end();\n"
               "    v.erase(it);\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (error) Function 'erase()' must not be called on the iterator 'it' since it is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (error) Calling function 'erase()' on the iterator 'it' which is out of bounds.\n", errout.str());
 
         check("void f() {\n"
               "    std::vector<int> v{ 1, 2, 3 };\n"
@@ -2182,7 +2182,7 @@ class TestStl : public TestFixture {
               "    std::vector<int> v{ 1, 2, 3 };\n"
               "    v.erase(v.begin() - 1);\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:3]: (error) Function 'erase()' must not be called on the iterator 'v.begin()-1' since it is out of bounds.\n"
+        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());
     }
@@ -2247,7 +2247,7 @@ class TestStl : public TestFixture {
               "    std::cout << iter->first << std::endl;\n"
               "}");
         ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n"
-                      "[test.cpp:6]: (error) Function 'erase()' must not be called on the end iterator 'iter'.\n",
+                      "[test.cpp:6]: (error) Calling function 'erase()' on the iterator 'iter' which is out of bounds.\n",
                       errout.str());
 
         // Reverse iterator
@@ -2260,7 +2260,7 @@ class TestStl : public TestFixture {
               "    std::cout << iter->first << std::endl;\n"
               "}");
         ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n"
-                      "[test.cpp:6]: (error) Function 'erase()' must not be called on the end iterator 'iter'.\n",
+                      "[test.cpp:6]: (error) Calling function 'erase()' on the iterator 'iter' which is out of bounds.\n",
                       errout.str());
     }
 

From 1834ef2ca8f112cea8a24675fc598b7fb5df4d7b Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Mon, 29 Jan 2024 18:26:19 +0100
Subject: [PATCH 06/15] Handle possible values

---
 lib/checkstl.cpp | 36 +++++++++++++++++++++++++-----------
 lib/checkstl.h   |  2 +-
 test/teststl.cpp |  7 +++++++
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 1b1d2bbede5..46405f605d1 100644
--- 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
index d0dae64b18b..a864e11b3e4 100644
--- 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
index 6522ae50884..37827e86c68 100644
--- 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

From 5f8aa932baace7177e208d65b11fd4de9f1f95a6 Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Mon, 29 Jan 2024 18:52:47 +0100
Subject: [PATCH 07/15] Format

---
 lib/checkstl.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 46405f605d1..bb72ad3ff9a 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3125,7 +3125,7 @@ void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* ite
     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.";
+        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.";
     }

From 1e134e2b7a1e0249ee74e54bbbd5ed4476f7fd77 Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Mon, 29 Jan 2024 19:04:33 +0100
Subject: [PATCH 08/15] cwe

---
 lib/checkstl.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index bb72ad3ff9a..433c2a82e99 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3132,7 +3132,7 @@ void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* ite
 
     reportError(ftok, isConditional ? Severity::warning : Severity::error,
                 "eraseIteratorOutOfBounds",
-                msg, CWE398, Certainty::normal);
+                msg, CWE628, Certainty::normal);
 }
 
 static const ValueFlow::Value* getIterValue(const Token* tok, ValueFlow::Value::ValueType vt)

From 69516692aa8aaab2a4030dd4e19d1c6d63d11876 Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Wed, 31 Jan 2024 19:31:45 +0100
Subject: [PATCH 09/15] Handle extra parentheses

---
 lib/astutils.cpp |  4 +++-
 lib/checkstl.cpp | 11 +++++------
 test/teststl.cpp |  7 +++++++
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index 2773e7e59a8..fdd2c9482fe 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -3019,8 +3019,10 @@ const Token* findExpressionChangedSkipDeadCode(const Token* expr,
 const Token* getArgumentStart(const Token* ftok)
 {
     const Token* tok = ftok;
-    if (Token::Match(tok, "%name% (|{"))
+    if (Token::Match(tok, "%name% (|{|)"))
         tok = ftok->next();
+    while (Token::simpleMatch(tok, ")"))
+        tok = tok->next();
     if (!Token::Match(tok, "(|{|["))
         return nullptr;
     const Token* startTok = tok->astOperand2();
diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 433c2a82e99..c5357c0cbfd 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3152,14 +3152,13 @@ void CheckStl::eraseIteratorOutOfBounds()
             if (!tok->valueType())
                 continue;
             const Library::Container* container = tok->valueType()->container;
-            if (!container)
-                continue;
-            if (!Token::Match(tok->next(), ". %name% ("))
+            if (!container || !Token::simpleMatch(tok->astParent(), "."))
                 continue;
-            const Library::Container::Action action = container->getAction(tok->strAt(2));
+            const Token* const ftok = tok->astParent()->astOperand2();
+            const Library::Container::Action action = container->getAction(ftok->str());
             if (action != Library::Container::Action::ERASE)
                 continue;
-            const std::vector<const Token*> args = getArguments(tok->tokAt(2));
+            const std::vector<const Token*> args = getArguments(ftok);
             if (args.size() != 1) // empty range is ok
                 continue;
 
@@ -3176,7 +3175,7 @@ void CheckStl::eraseIteratorOutOfBounds()
                         errVal = startVal;
             }
             if (errVal)
-                eraseIteratorOutOfBoundsError(tok->tokAt(2), args[0], errVal);
+                eraseIteratorOutOfBoundsError(ftok, args[0], errVal);
         }
     }
 }
diff --git a/test/teststl.cpp b/test/teststl.cpp
index 37827e86c68..257fd757706 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -2192,6 +2192,13 @@ class TestStl : public TestFixture {
               "}\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());
+
+        check("void f() {\n"
+              "    std::vector<int> v;\n"
+              "    ((v).erase)(v.begin());\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Calling function 'erase()' on the iterator 'v.begin()' which is out of bounds.\n",
+                      errout.str());
     }
 
     // Dereferencing invalid pointer

From 7bbfa0ac581d377219ee8786b169bf88b37a3f70 Mon Sep 17 00:00:00 2001
From: chrchr-github <christian.riesch@live.com>
Date: Wed, 31 Jan 2024 21:38:37 +0100
Subject: [PATCH 10/15] Check lhs

---
 lib/checkstl.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index c5357c0cbfd..5c113662d1c 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3152,7 +3152,7 @@ void CheckStl::eraseIteratorOutOfBounds()
             if (!tok->valueType())
                 continue;
             const Library::Container* container = tok->valueType()->container;
-            if (!container || !Token::simpleMatch(tok->astParent(), "."))
+            if (!container || (!astIsLHS(tok) && !Token::simpleMatch(tok->astParent(), ".")))
                 continue;
             const Token* const ftok = tok->astParent()->astOperand2();
             const Library::Container::Action action = container->getAction(ftok->str());

From c724d88472813a99392aba4d2706d472a9c11bde Mon Sep 17 00:00:00 2001
From: chrchr-github <christian.riesch@live.com>
Date: Wed, 31 Jan 2024 22:24:51 +0100
Subject: [PATCH 11/15] Fix

---
 lib/checkstl.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 5c113662d1c..e1c5cb333d3 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3152,7 +3152,7 @@ void CheckStl::eraseIteratorOutOfBounds()
             if (!tok->valueType())
                 continue;
             const Library::Container* container = tok->valueType()->container;
-            if (!container || (!astIsLHS(tok) && !Token::simpleMatch(tok->astParent(), ".")))
+            if (!container || !astIsLHS(tok) || !Token::simpleMatch(tok->astParent(), "."))
                 continue;
             const Token* const ftok = tok->astParent()->astOperand2();
             const Library::Container::Action action = container->getAction(ftok->str());

From 4ca14a92fea30847e9918fed6901a4264563a3ea Mon Sep 17 00:00:00 2001
From: chrchr-github <christian.riesch@live.com>
Date: Thu, 1 Feb 2024 22:10:25 +0100
Subject: [PATCH 12/15] conditional id, classInfo(), getErrorMessages()

---
 lib/checkstl.cpp | 6 ++++--
 lib/checkstl.h   | 2 ++
 releasenotes.txt | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index e1c5cb333d3..ff06b922dca 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3130,8 +3130,10 @@ void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* ite
         msg = "Calling function '" + func + "()' on the iterator '" + iter + "' which is out of bounds.";
     }
 
-    reportError(ftok, isConditional ? Severity::warning : Severity::error,
-                "eraseIteratorOutOfBounds",
+    const Severity severity = isConditional ? Severity::warning : Severity::error;
+    const std::string id = isConditional ? "eraseIteratorOutOfBoundsCond" : "eraseIteratorOutOfBounds";
+    reportError(ftok, severity,
+                id,
                 msg, CWE628, Certainty::normal);
 }
 
diff --git a/lib/checkstl.h b/lib/checkstl.h
index a864e11b3e4..ec476bb5156 100644
--- a/lib/checkstl.h
+++ b/lib/checkstl.h
@@ -276,6 +276,7 @@ class CPPCHECKLIB CheckStl : public Check {
         c.uselessCallsEmptyError(nullptr);
         c.uselessCallsRemoveError(nullptr, "remove");
         c.dereferenceInvalidIteratorError(nullptr, "i");
+        c.eraseIteratorOutOfBoundsError(nullptr, nullptr);
         c.useStlAlgorithmError(nullptr, emptyString);
         c.knownEmptyContainerError(nullptr, emptyString);
         c.globalLockGuardError(nullptr);
@@ -301,6 +302,7 @@ class CPPCHECKLIB CheckStl : public Check {
                "- common mistakes when using string::c_str()\n"
                "- useless calls of string and STL functions\n"
                "- dereferencing an invalid iterator\n"
+               "- erasing an iterator that is out of bounds\n"
                "- reading from empty STL container\n"
                "- iterating over an empty STL container\n"
                "- consider using an STL algorithm instead of raw loop\n"
diff --git a/releasenotes.txt b/releasenotes.txt
index 6b313cce25d..3cf24efdb0f 100644
--- a/releasenotes.txt
+++ b/releasenotes.txt
@@ -1,7 +1,7 @@
 Release Notes for Cppcheck 2.14
 
 New checks:
--
+eraseIteratorOutOfBounds: warns when erase() is called on an iterator that is out of bounds
 
 Improved checking:
 -

From b80a19375956d9de9485d0c0f977ef4569e6ab7c Mon Sep 17 00:00:00 2001
From: chrchr-github <christian.riesch@live.com>
Date: Thu, 1 Feb 2024 23:44:12 +0100
Subject: [PATCH 13/15] loop

---
 lib/checkstl.cpp | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index ff06b922dca..32091b55eab 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3137,10 +3137,20 @@ void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* ite
                 msg, CWE628, Certainty::normal);
 }
 
-static const ValueFlow::Value* getIterValue(const Token* tok, ValueFlow::Value::ValueType vt)
+static const ValueFlow::Value* getOOBIterValue(const Token* tok, const ValueFlow::Value* sizeVal)
 {
     auto it = std::find_if(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) {
-        return v.valueType == vt && (v.isPossible() || v.isKnown());
+        if (v.isPossible() || v.isKnown()) {
+            switch (v.valueType) {
+            case ValueFlow::Value::ValueType::ITERATOR_END:
+                return v.intvalue >= 0;
+            case ValueFlow::Value::ValueType::ITERATOR_START:
+                return (v.intvalue < 0) || (sizeVal && v.intvalue >= sizeVal->intvalue);
+            default:
+                break;
+            }
+        }
+        return false;
     });
     return it != tok->values().end() ? &*it : nullptr;
 }
@@ -3161,22 +3171,11 @@ void CheckStl::eraseIteratorOutOfBounds()
             if (action != Library::Container::Action::ERASE)
                 continue;
             const std::vector<const Token*> args = getArguments(ftok);
-            if (args.size() != 1) // empty range is ok
+            if (args.size() != 1) // TODO: check range overload
                 continue;
 
-            const ValueFlow::Value* errVal = nullptr;
-            if (const ValueFlow::Value* endVal = getIterValue(args[0], ValueFlow::Value::ValueType::ITERATOR_END)) {
-                if (endVal->intvalue >= 0)
-                    errVal = endVal;
-            }
-            else if (const ValueFlow::Value* startVal = getIterValue(args[0], ValueFlow::Value::ValueType::ITERATOR_START)) {
-                if (startVal->intvalue < 0)
-                    errVal = startVal;
-                else if (const ValueFlow::Value* sizeVal = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
-                    if (startVal->intvalue >= sizeVal->intvalue)
-                        errVal = startVal;
-            }
-            if (errVal)
+            const ValueFlow::Value* sizeVal = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE);
+            if (const ValueFlow::Value* errVal = getOOBIterValue(args[0], sizeVal))
                 eraseIteratorOutOfBoundsError(ftok, args[0], errVal);
         }
     }

From d6c2045dd99ec8d66dfca342c7eff37a3fcd1f61 Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Fri, 2 Feb 2024 17:38:03 +0100
Subject: [PATCH 14/15] Mock error message

---
 lib/checkstl.cpp | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index 32091b55eab..a9bf016aee4 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3119,10 +3119,16 @@ void CheckStl::knownEmptyContainer()
 
 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");
+    if (!ftok || !itertok || !val) {
+        reportError(ftok, Severity::error, "eraseIteratorOutOfBounds",
+                    "Calling function 'erase()' on the iterator 'iter' which is out of bounds.", CWE628, Certainty::normal);
+        reportError(ftok, Severity::warning, "eraseIteratorOutOfBoundsCond",
+                    "Either the condition 'x' is redundant or function 'erase()' is called on the iterator 'iter' which is out of bounds.", CWE628, Certainty::normal);
+    }
+    const std::string& func = ftok->str();
+    const std::string iter = itertok->expressionString();
 
-    const bool isConditional = val && val->isPossible();
+    const bool isConditional = 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.";

From 2b7970241481c9620acf2313991c6ecd42bced9f Mon Sep 17 00:00:00 2001
From: chrchr <christian.riesch@live.com>
Date: Fri, 2 Feb 2024 17:52:16 +0100
Subject: [PATCH 15/15] return

---
 lib/checkstl.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index a9bf016aee4..5609b0fafb0 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -3124,6 +3124,7 @@ void CheckStl::eraseIteratorOutOfBoundsError(const Token *ftok, const Token* ite
                     "Calling function 'erase()' on the iterator 'iter' which is out of bounds.", CWE628, Certainty::normal);
         reportError(ftok, Severity::warning, "eraseIteratorOutOfBoundsCond",
                     "Either the condition 'x' is redundant or function 'erase()' is called on the iterator 'iter' which is out of bounds.", CWE628, Certainty::normal);
+        return;
     }
     const std::string& func = ftok->str();
     const std::string iter = itertok->expressionString();