From f64bcac00450300989f61a727ba2f23445449683 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sun, 5 Dec 2021 08:46:52 -0600
Subject: [PATCH] Fix 10429: Regression: invalidIterator (#3603)

---
 lib/calculate.h         |  7 +++++--
 lib/forwardanalyzer.cpp | 18 +++++++++++++++++-
 lib/valueflow.cpp       |  2 +-
 test/teststl.cpp        |  9 +++++++++
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/lib/calculate.h b/lib/calculate.h
index 599662661..0d27a1c5e 100644
--- a/lib/calculate.h
+++ b/lib/calculate.h
@@ -50,6 +50,9 @@ R calculate(const std::string& s, const T& x, const T& y, bool* error = nullptr)
     auto wrap = [](T z) {
         return R{z};
     };
+    const MathLib::bigint maxBitsShift = sizeof(MathLib::bigint) * 8;
+    // For portability we cannot shift signed integers by 63 bits
+    const MathLib::bigint maxBitsSignedShift = maxBitsShift - 1;
     switch (MathLib::encodeMultiChar(s)) {
     case '+':
         return wrap(x + y);
@@ -82,14 +85,14 @@ R calculate(const std::string& s, const T& x, const T& y, bool* error = nullptr)
     case '<':
         return wrap(x < y);
     case '<<':
-        if (y >= sizeof(MathLib::bigint) * 8 || y < 0 || x < 0) {
+        if (y >= maxBitsSignedShift || y < 0 || x < 0) {
             if (error)
                 *error = true;
             return R{};
         }
         return wrap(MathLib::bigint(x) << MathLib::bigint(y));
     case '>>':
-        if (y >= sizeof(MathLib::bigint) * 8 || y < 0 || x < 0) {
+        if (y >= maxBitsSignedShift || y < 0 || x < 0) {
             if (error)
                 *error = true;
             return R{};
diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
index 09965a022..19cd196df 100644
--- a/lib/forwardanalyzer.cpp
+++ b/lib/forwardanalyzer.cpp
@@ -145,7 +145,8 @@ struct ForwardTraversal {
         // Evaluate:
         //     1. RHS of assignment before LHS
         //     2. Unary op before operand
-        if (tok->isAssignmentOp() || !secondOp)
+        //     3. Function arguments before function call
+        if (tok->isAssignmentOp() || !secondOp || isFunctionCall(tok))
             std::swap(firstOp, secondOp);
         if (firstOp && traverseRecursive(firstOp, f, traverseUnknown, recursion+1) == Progress::Break)
             return Break();
@@ -757,6 +758,21 @@ struct ForwardTraversal {
         return false;
     }
 
+    static bool isFunctionCall(const Token* tok)
+    {
+        if (!Token::simpleMatch(tok, "("))
+            return false;
+        if (tok->isCast())
+            return false;
+        if (!tok->isBinaryOp())
+            return false;
+        if (Token::simpleMatch(tok->link(), ") {"))
+            return false;
+        if (isUnevaluated(tok))
+            return false;
+        return Token::Match(tok->previous(), "%name%|)|]|>");
+    }
+
     static Token* assignExpr(Token* tok) {
         while (tok->astParent() && astIsLHS(tok)) {
             if (tok->astParent()->isAssignmentOp())
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 50e9ded78..a29e4bfd7 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2651,7 +2651,7 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
             return;
         visitAstNodes(start, [&](const Token* tok) {
             const bool top = depth == 0 && tok == start;
-            const bool ispointer = astIsPointer(tok) || astIsSmartPointer(tok);
+            const bool ispointer = astIsPointer(tok) || astIsSmartPointer(tok) || astIsIterator(tok);
             if (!top || !ispointer || value.indirect != 0) {
                 for (const ValueFlow::Value& v : tok->values()) {
                     if (!(v.isLocalLifetimeValue() || (ispointer && v.isSymbolicValue() && v.isKnown())))
diff --git a/test/teststl.cpp b/test/teststl.cpp
index ccbe4af11..79a57331a 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -4173,6 +4173,15 @@ private:
               "    return v;\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("extern bool bar(int);\n"
+              "void f(std::vector<int> & v) {\n"
+              "    std::vector<int>::iterator i= v.begin();\n"
+              "    if(i == v.end() && bar(*(i+1)) ) {}\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:4] -> [test.cpp:4]: (warning) Either the condition 'i==v.end()' is redundant or there is possible dereference of an invalid iterator: i+1.\n",
+            errout.str());
     }
 
     void dereferenceInvalidIterator2() {