Infer variables from conditions in valueFlowSubfunction (#3037)

lib/analyzer.h

@@ -120,6 +120,8 @@ struct Analyzer {
     virtual bool lowerToInconclusive() = 0;
     /// If the analysis is unsure whether to update a scope, this will return true if the analysis should bifurcate the scope
     virtual bool updateScope(const Token* endBlock, bool modified) const = 0;
+    /// Called when a scope will be forked
+    virtual void forkScope(const Token* endBlock) {}
     /// If the value is conditional
     virtual bool isConditional() const = 0;
     /// The condition that will be assumed during analysis

lib/astutils.cpp

@@ -707,6 +707,8 @@ static const Token * followVariableExpression(const Token * tok, bool cpp, const
     if (varTok->exprId() == 0) {
         if (!varTok->isLiteral())
             return tok;
+    } else if (!precedes(startToken, endToken)) {
+        return tok;
     } else if (isExpressionChanged(varTok, startToken, endToken, nullptr, cpp)) {
         return tok;
     }
@@ -1804,6 +1806,8 @@ bool isVariablesChanged(const Token* start,
 
 bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp)
 {
+    if (!precedes(start, end))
+        return false;
     for (const Token* tok = start; tok != end; tok = tok->next()) {
         if (!exprDependsOnThis(tok))
             continue;
@@ -1825,6 +1829,8 @@ bool isThisChanged(const Token* start, const Token* end, int indirect, const Set
 
 bool isExpressionChanged(const Token* expr, const Token* start, const Token* end, const Settings* settings, bool cpp, int depth)
 {
+    if (!precedes(start, end))
+        return false;
     const Token* result = findAstNode(expr, [&](const Token* tok) {
         if (exprDependsOnThis(tok) && isThisChanged(start, end, false, settings, cpp)) {
             return true;

lib/forwardanalyzer.cpp

@@ -218,6 +218,7 @@ struct ForwardTraversal {
     void forkScope(Token* endBlock, bool isModified = false) {
         if (analyzer->updateScope(endBlock, isModified)) {
             ForwardTraversal ft = *this;
+            ft.analyzer->forkScope(endBlock);
             ft.updateRange(endBlock->link(), endBlock);
         }
     }

lib/programmemory.cpp

@@ -1,5 +1,6 @@
 
 #include "programmemory.h"
+#include "mathlib.h"
 #include "token.h"
 #include "astutils.h"
 #include "symboldatabase.h"
@@ -124,16 +125,25 @@ bool conditionIsTrue(const Token *condition, const ProgramMemory &programMemory)
     return !error && result == 1;
 }
 
-static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings* settings, bool then)
+void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings* settings, bool then)
 {
     if (Token::Match(tok, "==|>=|<=|<|>|!=")) {
-        if (then && !Token::Match(tok, "==|>=|<="))
+        if (then && !Token::Match(tok, "==|>=|<=|<|>"))
             return;
         if (!then && !Token::Match(tok, "<|>|!="))
             return;
         ValueFlow::Value truevalue;
         ValueFlow::Value falsevalue;
-        const Token* vartok = parseCompareInt(tok, truevalue, falsevalue);
+        const Token* vartok = parseCompareInt(tok, truevalue, falsevalue, [&](const Token* t) -> std::vector<MathLib::bigint> {
+            if (t->hasKnownIntValue())
+                return {t->values().front().intvalue};
+            MathLib::bigint result = 0;
+            bool error = false;
+            execute(t, &pm, &result, &error);
+            if (!error)
+                return {result};
+            return std::vector<MathLib::bigint>{};
+        });
         if (!vartok)
             return;
         if (vartok->varId() == 0)
@@ -170,22 +180,10 @@ static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scop
         return;
     assert(scope != scope->nestedIn);
     fillProgramMemoryFromConditions(pm, scope->nestedIn, endTok, settings);
-    if (scope->type == Scope::eIf || scope->type == Scope::eWhile || scope->type == Scope::eElse) {
+    if (scope->type == Scope::eIf || scope->type == Scope::eWhile || scope->type == Scope::eElse || scope->type == Scope::eFor) {
-        const Token * bodyStart = scope->bodyStart;
+        const Token* condTok = getCondTokFromEnd(scope->bodyEnd);
-        if (scope->type == Scope::eElse) {
+        if (!condTok)
-            if (!Token::simpleMatch(bodyStart->tokAt(-2), "} else {"))
-                return;
-            bodyStart = bodyStart->linkAt(-2);
-        }
-        const Token * condEndTok = bodyStart->previous();
-        if (!Token::simpleMatch(condEndTok, ") {"))
             return;
-        const Token * condStartTok = condEndTok->link();
-        if (!condStartTok)
-            return;
-        if (!Token::Match(condStartTok->previous(), "if|while ("))
-            return;
-        const Token * condTok = condStartTok->astOperand2();
         programMemoryParseCondition(pm, condTok, endTok, settings, scope->type != Scope::eElse);
     }
 }

lib/programmemory.h

@@ -38,6 +38,8 @@ struct ProgramMemory {
     void insert(const ProgramMemory &pm);
 };
 
+void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings* settings, bool then);
+
 struct ProgramMemoryState {
     ProgramMemory state;
     std::map<nonneg int, const Token*> origins;

lib/valueflow.cpp

@@ -229,28 +229,37 @@ static bool isSaturated(MathLib::bigint value)
     return value == std::numeric_limits<MathLib::bigint>::max() || value == std::numeric_limits<MathLib::bigint>::min();
 }
 
-const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value)
+const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value, const std::function<std::vector<MathLib::bigint>(const Token*)>& evaluate)
 {
     if (!tok->astOperand1() || !tok->astOperand2())
         return nullptr;
     if (tok->isComparisonOp()) {
-        if (tok->astOperand1()->hasKnownIntValue()) {
+        std::vector<MathLib::bigint> value1 = evaluate(tok->astOperand1());
-            MathLib::bigint value = tok->astOperand1()->values().front().intvalue;
+        std::vector<MathLib::bigint> value2 = evaluate(tok->astOperand2());
-            if (isSaturated(value))
+        if (!value1.empty()) {
+            if (isSaturated(value1.front()))
                 return nullptr;
-            setConditionalValues(tok, true, value, true_value, false_value);
+            setConditionalValues(tok, true, value1.front(), true_value, false_value);
             return tok->astOperand2();
-        } else if (tok->astOperand2()->hasKnownIntValue()) {
+        } else if (!value2.empty()) {
-            MathLib::bigint value = tok->astOperand2()->values().front().intvalue;
+            if (isSaturated(value2.front()))
-            if (isSaturated(value))
                 return nullptr;
-            setConditionalValues(tok, false, value, true_value, false_value);
+            setConditionalValues(tok, false, value2.front(), true_value, false_value);
             return tok->astOperand1();
         }
     }
     return nullptr;
 }
 
+const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value)
+{
+    return parseCompareInt(tok, true_value, false_value, [](const Token* t) -> std::vector<MathLib::bigint> {
+        if (t->hasKnownIntValue())
+            return {t->values().front().intvalue};
+        return std::vector<MathLib::bigint>{};
+    });
+}
+
 
 static bool isEscapeScope(const Token* tok, TokenList * tokenlist, bool unknown = false)
 {
@@ -4845,11 +4854,12 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas
 struct MultiValueFlowAnalyzer : ValueFlowAnalyzer {
     std::unordered_map<nonneg int, ValueFlow::Value> values;
     std::unordered_map<nonneg int, const Variable*> vars;
+    SymbolDatabase* symboldatabase;
 
-    MultiValueFlowAnalyzer() : ValueFlowAnalyzer(), values(), vars() {}
+    MultiValueFlowAnalyzer() : ValueFlowAnalyzer(), values(), vars(), symboldatabase(nullptr) {}
 
-    MultiValueFlowAnalyzer(const std::unordered_map<const Variable*, ValueFlow::Value>& args, const TokenList* t)
+    MultiValueFlowAnalyzer(const std::unordered_map<const Variable*, ValueFlow::Value>& args, const TokenList* t, SymbolDatabase* s)
-        : ValueFlowAnalyzer(t), values(), vars() {
+        : ValueFlowAnalyzer(t), values(), vars(), symboldatabase(s) {
         for (const auto& p:args) {
             values[p.first->declarationId()] = p.second;
             vars[p.first->declarationId()] = p.first;
@@ -4979,9 +4989,30 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer {
             ps[p.first] = p.second;
         return ps;
     }
+
+    virtual void forkScope(const Token* endBlock) OVERRIDE {
+        ProgramMemory pm = {getProgramState()};
+        const Scope* scope = endBlock->scope();
+        const Token* condTok = getCondTokFromEnd(endBlock);
+        if (scope && condTok)
+            programMemoryParseCondition(pm, condTok, nullptr, getSettings(), scope->type != Scope::eElse);
+        // ProgramMemory pm = pms.get(endBlock->link()->next(), getProgramState());
+        for(const auto& p:pm.values) {
+            int varid = p.first;
+            ValueFlow::Value value = p.second;
+            if (vars.count(varid) != 0)
+                continue;
+            if (value.isImpossible())
+                continue;
+            value.setPossible();
+            values[varid] = value;
+            if (symboldatabase)
+                vars[varid] = symboldatabase->getVariableFromVarId(varid);
+        }
+    }
 };
 
-static void valueFlowInjectParameter(TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings, const Scope* functionScope, const std::unordered_map<const Variable*, std::list<ValueFlow::Value>>& vars)
+static void valueFlowInjectParameter(TokenList* tokenlist, SymbolDatabase* symboldatabase, ErrorLogger* errorLogger, const Settings* settings, const Scope* functionScope, const std::unordered_map<const Variable*, std::list<ValueFlow::Value>>& vars)
 {
     using Args = std::vector<std::unordered_map<const Variable*, ValueFlow::Value>>;
     Args args(1);
@@ -5033,7 +5064,7 @@ static void valueFlowInjectParameter(TokenList* tokenlist, ErrorLogger* errorLog
         }
         if (skip)
             continue;
-        MultiValueFlowAnalyzer a(arg, tokenlist);
+        MultiValueFlowAnalyzer a(arg, tokenlist, symboldatabase);
         valueFlowGenericForward(const_cast<Token*>(functionScope->bodyStart), functionScope->bodyEnd, a, settings);
     }
 }
@@ -5407,7 +5438,7 @@ static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldat
 
                 argvars[argvar] = argvalues;
             }
-            valueFlowInjectParameter(tokenlist, errorLogger, settings, calledFunctionScope, argvars);
+            valueFlowInjectParameter(tokenlist, symboldatabase, errorLogger, settings, calledFunctionScope, argvars);
         }
     }
 }

lib/valueflow.h

@@ -382,6 +382,7 @@ struct LifetimeToken {
     }
 };
 
+const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value, const std::function<std::vector<MathLib::bigint>(const Token*)>& evaluate);
 const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value);
 
 std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,

test/testvalueflow.cpp

@@ -3476,6 +3476,18 @@ private:
         ASSERT_EQUALS(true, testValueOfX(code, 3U, 7));
         ASSERT_EQUALS(true, testValueOfX(code, 3U, 8));
         ASSERT_EQUALS(true, testValueOfX(code, 3U, 9));
+
+        code = "int f(int i, int j) {\n"
+               "    if (i == j) {\n"
+               "        int x = i;\n"
+               "        return x;\n"
+               "    }\n"
+               "    return 0;\n"
+               "}\n"
+               "int g(int x) {\n"
+               "    f(x, -1);\n"
+               "}\n";
+        ASSERT_EQUALS(true, testValueOfX(code, 4U, -1));
     }
     void valueFlowFunctionReturn() {
         const char *code;
@@ -5058,6 +5070,25 @@ private:
                "   if (i2) { }\n"
                "}\n";
         valueOfTok(code, "p");
+
+        code = "struct a;\n"
+               "namespace e {\n"
+               "struct f {\n"
+               "  struct g {\n"
+               "    enum {} h;\n"
+               "    int arg;\n"
+               "  };\n"
+               "  std::vector<g> i;\n"
+               "};\n"
+               "} // namespace e\n"
+               "void fn1() {\n"
+               "  std::vector<a *> arguments;\n"
+               "  e::f b;\n"
+               "  for (e::f::g c : b.i)\n"
+               "    if (c.h)\n"
+               "      a *const d = arguments[c.arg];\n"
+               "}\n";
+        valueOfTok(code, "c");
     }
 
     void valueFlowHang() {