From 91d2526c41d277d01984deb25c61c71e876a4166 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Thu, 23 Feb 2023 11:04:16 -0600
Subject: [PATCH] Fix reverse analysis when modifying variable with function
 (#4800)

---
 lib/reverseanalyzer.cpp |  4 ++--
 lib/valueflow.cpp       | 28 +++++++++++++++-------------
 test/testvalueflow.cpp  | 13 ++++++++++++-
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp
index 05385fdee..31512c868 100644
--- a/lib/reverseanalyzer.cpp
+++ b/lib/reverseanalyzer.cpp
@@ -56,12 +56,12 @@ struct ReverseTraversal {
 
     bool update(Token* tok) {
         Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Reverse);
-        if (!action.isNone())
-            analyzer->update(tok, action, Analyzer::Direction::Reverse);
         if (action.isInconclusive() && !analyzer->lowerToInconclusive())
             return false;
         if (action.isInvalid())
             return false;
+        if (!action.isNone())
+            analyzer->update(tok, action, Analyzer::Direction::Reverse);
         return true;
     }
 
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 312740f9f..e8c8fdef5 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -7751,11 +7751,10 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
         return tok->exprId() == expr->exprId() || (astIsIterator(tok) && isAliasOf(tok, expr->exprId()));
     }
 
-    Action isWritable(const Token* tok, Direction d) const override {
+    Action isWritable(const Token* tok, Direction /*d*/) const override
+    {
         if (astIsIterator(tok))
             return Action::None;
-        if (d == Direction::Reverse)
-            return Action::None;
         if (!getValue(tok))
             return Action::None;
         if (!tok->valueType())
@@ -7788,8 +7787,6 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
     }
 
     void writeValue(ValueFlow::Value* val, const Token* tok, Direction d) const override {
-        if (d == Direction::Reverse)
-            return;
         if (!val)
             return;
         if (!tok->astParent())
@@ -7800,26 +7797,31 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer {
             return;
         const Token* parent = tok->astParent();
         const Library::Container* container = getLibraryContainer(tok);
+        int n = 0;
 
         if (container->stdStringLike && Token::simpleMatch(parent, "+=") && parent->astOperand2()) {
             const Token* rhs = parent->astOperand2();
             const Library::Container* rhsContainer = getLibraryContainer(rhs);
             if (rhs->tokType() == Token::eString)
-                val->intvalue += Token::getStrLength(rhs);
+                n = Token::getStrLength(rhs);
             else if (rhsContainer && rhsContainer->stdStringLike) {
-                for (const ValueFlow::Value &rhsval : rhs->values()) {
-                    if (rhsval.isKnown() && rhsval.isContainerSizeValue()) {
-                        val->intvalue += rhsval.intvalue;
-                    }
-                }
+                auto it = std::find_if(rhs->values().begin(), rhs->values().end(), [&](const ValueFlow::Value& rhsval) {
+                    return rhsval.isKnown() && rhsval.isContainerSizeValue();
+                });
+                if (it != rhs->values().end())
+                    n = it->intvalue;
             }
         } else if (astIsLHS(tok) && Token::Match(tok->astParent(), ". %name% (")) {
             const Library::Container::Action action = container->getAction(tok->astParent()->strAt(1));
             if (action == Library::Container::Action::PUSH)
-                val->intvalue++;
+                n = 1;
             if (action == Library::Container::Action::POP)
-                val->intvalue--;
+                n = -1;
         }
+        if (d == Direction::Reverse)
+            val->intvalue -= n;
+        else
+            val->intvalue += n;
     }
 
     int getIndirect(const Token* tok) const override
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index 3e095b989..393f1ffcc 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -1529,6 +1529,7 @@ private:
                "   if (x == 4);\n"
                "}";
         ASSERT_EQUALS(true, testValueOfX(code, 2U, 2));
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 2));
 
         code = "void f(int x) {\n"
                "   a = x;\n"
@@ -1536,6 +1537,7 @@ private:
                "   if (x == 4);\n"
                "}";
         ASSERT_EQUALS(true, testValueOfX(code, 2U, 6));
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 6));
 
         code = "void f(int x) {\n"
                "   a = x;\n"
@@ -1543,6 +1545,7 @@ private:
                "   if (x == 42);\n"
                "}";
         ASSERT_EQUALS(true, testValueOfX(code, 2U, 21));
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 21));
 
         code = "void f(int x) {\n"
                "   a = x;\n"
@@ -1550,6 +1553,7 @@ private:
                "   if (x == 42);\n"
                "}";
         ASSERT_EQUALS(true, testValueOfX(code, 2U, 210));
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 210));
 
         // bailout: assignment
         bailout("void f(int x) {\n"
@@ -1612,6 +1616,13 @@ private:
                "  if (x) {}\n"
                "}";
         ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
+
+        code = "void g(int&);"
+               "void f(int x) {\n"
+               "   g(x);\n"
+               "   if (x == 5);\n"
+               "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 2U, 5));
     }
 
     void valueFlowBeforeConditionLoop() { // while, for, do-while
@@ -5666,7 +5677,7 @@ private:
                "  ints.pop_back();\n"
                "  if (ints.empty()) {}\n"
                "}";
-        ASSERT(tokenValues(code, "ints . front").empty());
+        ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 1));
 
         code = "void f(std::vector<int> v) {\n"
                "  v[10] = 0;\n" // <- container size can be 10