From 0f897acecd2da8849fb335882eaadfaaffa9d95a Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Mon, 9 Aug 2021 13:41:10 -0500
Subject: [PATCH] Fix FP in solveExpr when using symbolic values (#3391)

---
 lib/valueflow.cpp      | 54 +++++++++++++++++++-----------------------
 test/testvalueflow.cpp | 13 ++++++++++
 2 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index a0febe7b3..808120562 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2322,7 +2322,7 @@ struct ValueFlowAnalyzer : Analyzer {
     }
 };
 
-ValuePtr<Analyzer> makeAnalyzer(const Token* exprTok, const ValueFlow::Value& value, const TokenList* tokenlist);
+ValuePtr<Analyzer> makeAnalyzer(const Token* exprTok, ValueFlow::Value value, const TokenList* tokenlist);
 
 struct SingleValueFlowAnalyzer : ValueFlowAnalyzer {
     std::unordered_map<nonneg int, const Variable*> varids;
@@ -2580,52 +2580,43 @@ static const Token* parseBinaryIntOp(const Token* expr, MathLib::bigint& known)
     return varTok;
 }
 
-template<class F>
-void transformIntValues(std::list<ValueFlow::Value>& values, F f)
-{
-    std::transform(values.begin(), values.end(), values.begin(), [&](ValueFlow::Value x) {
-        if (x.isIntValue() || x.isIteratorValue())
-            x.intvalue = f(x.intvalue);
-        return x;
-    });
-}
-
-static const Token* solveExprValues(const Token* expr, std::list<ValueFlow::Value>& values)
+static const Token* solveExprValue(const Token* expr, ValueFlow::Value& value)
 {
+    if (!value.isIntValue() && !value.isIteratorValue() && !value.isSymbolicValue())
+        return expr;
+    if (value.isSymbolicValue() && !Token::Match(expr, "+|-"))
+        return expr;
     MathLib::bigint intval;
     const Token* binaryTok = parseBinaryIntOp(expr, intval);
     if (binaryTok && expr->str().size() == 1) {
         switch (expr->str()[0]) {
         case '+': {
-            transformIntValues(values, [&](MathLib::bigint x) {
-                    return x - intval;
-                });
-            return solveExprValues(binaryTok, values);
+            value.intvalue -= intval;
+            return solveExprValue(binaryTok, value);
+        }
+        case '-': {
+            value.intvalue += intval;
+            return solveExprValue(binaryTok, value);
         }
         case '*': {
             if (intval == 0)
                 break;
-            transformIntValues(values, [&](MathLib::bigint x) {
-                    return x / intval;
-                });
-            return solveExprValues(binaryTok, values);
+            value.intvalue /= intval;
+            return solveExprValue(binaryTok, value);
         }
         case '^': {
-            transformIntValues(values, [&](MathLib::bigint x) {
-                    return x ^ intval;
-                });
-            return solveExprValues(binaryTok, values);
+            value.intvalue ^= intval;
+            return solveExprValue(binaryTok, value);
         }
         }
     }
     return expr;
 }
 
-ValuePtr<Analyzer> makeAnalyzer(const Token* exprTok, const ValueFlow::Value& value, const TokenList* tokenlist)
+ValuePtr<Analyzer> makeAnalyzer(const Token* exprTok, ValueFlow::Value value, const TokenList* tokenlist)
 {
-    std::list<ValueFlow::Value> values = {value};
-    const Token* expr = solveExprValues(exprTok, values);
-    return ExpressionAnalyzer(expr, values.front(), tokenlist);
+    const Token* expr = solveExprValue(exprTok, value);
+    return ExpressionAnalyzer(expr, value, tokenlist);
 }
 
 static Analyzer::Result valueFlowForward(Token* startToken,
@@ -2635,8 +2626,11 @@ static Analyzer::Result valueFlowForward(Token* startToken,
                                          TokenList* const tokenlist,
                                          const Settings* settings)
 {
-    const Token* expr = solveExprValues(exprTok, values);
-    return valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings);
+    Analyzer::Result result{};
+    for (const ValueFlow::Value& v : values) {
+        result.update(valueFlowGenericForward(startToken, endToken, makeAnalyzer(exprTok, v, tokenlist), settings));
+    }
+    return result;
 }
 
 static Analyzer::Result valueFlowForward(Token* top,
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index 1ce2bf932..b18f76e46 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -6028,6 +6028,19 @@ private:
         ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, "i", 0));
         ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, "i", 1));
         ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, "j", 0));
+
+        code = "void f(int x) {\n"
+               "  int y = x + 1;\n"
+               "  return x;\n"
+               "}\n";
+        ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, "y", 0));
+        ASSERT_EQUALS(true, testValueOfXKnown(code, 3U, "y", -1));
+
+        code = "void f(int x) {\n"
+               "  int y = x * 2;\n"
+               "  return x;\n"
+               "}\n";
+        ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, "y", 0));
     }
 };