Fixed #6830 (ValueFlow: value of switch-variable inside switch)

lib/valueflow.cpp

@@ -596,110 +596,23 @@ static void valueFlowBitAnd(TokenList *tokenlist)
     }
 }
 
-static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
+static void valueFlowReverse(TokenList *tokenlist,
+                             Token *tok,
+                             const Token * const varToken,
+                             ValueFlow::Value val,
+                             ValueFlow::Value val2,
+                             ErrorLogger *errorLogger,
+                             const Settings *settings)
 {
-    const std::size_t functions = symboldatabase->functionScopes.size();
+    const MathLib::bigint    num        = val.intvalue;
-    for (std::size_t i = 0; i < functions; ++i) {
+    const Variable * const   var        = varToken->variable();
-        const Scope * scope = symboldatabase->functionScopes[i];
+    const unsigned int       varid      = varToken->varId();
-        for (Token* tok = const_cast<Token*>(scope->classStart); tok != scope->classEnd; tok = tok->next()) {
+    const Token * const      startToken = var->nameToken();
-            unsigned int varid=0;
-            MathLib::bigint num=0;
-            const Variable *var=0;
-            if (tok->isComparisonOp() && tok->astOperand1() && tok->astOperand2()) {
-                if (tok->astOperand1()->isName() && tok->astOperand2()->isNumber()) {
-                    varid = tok->astOperand1()->varId();
-                    var = tok->astOperand1()->variable();
-                    num = MathLib::toLongNumber(tok->astOperand2()->str());
-                } else if (tok->astOperand1()->isNumber() && tok->astOperand2()->isName()) {
-                    varid = tok->astOperand2()->varId();
-                    var = tok->astOperand2()->variable();
-                    num = MathLib::toLongNumber(tok->astOperand1()->str());
-                } else {
-                    continue;
-                }
-            } else if (Token::Match(tok->previous(), "if|while ( %name% %oror%|&&|)") ||
-                       Token::Match(tok, "%oror%|&& %name% %oror%|&&|)")) {
-                varid = tok->next()->varId();
-                var = tok->next()->variable();
-                num = 0;
-            } else if (tok->str() == "!" && tok->astOperand1() && tok->astOperand1()->isName()) {
-                varid = tok->astOperand1()->varId();
-                var = tok->astOperand1()->variable();
-                num = 0;
-            } else {
-                continue;
-            }
 
-            if (varid == 0U || !var)
-                continue;
-
-            // bailout: global non-const variables
-            if (!(var->isLocal() || var->isArgument()) && !var->isConst()) {
-                if (settings->debugwarnings)
-                    bailout(tokenlist, errorLogger, tok, "global variable " + var->name());
-                continue;
-            }
-
-            // bailout: for/while-condition, variable is changed in while loop
-            for (const Token *tok2 = tok; tok2; tok2 = tok2->astParent()) {
-                if (tok2->astParent() || tok2->str() != "(" || !Token::simpleMatch(tok2->link(), ") {"))
-                    continue;
-
-                // Variable changed in 3rd for-expression
-                if (Token::simpleMatch(tok2->previous(), "for (")) {
-                    if (tok2->astOperand2() && isVariableChanged(tok2->astOperand2()->astOperand2(), tok2->link(), varid)) {
-                        varid = 0U;
-                        if (settings->debugwarnings)
-                            bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
-                    }
-                }
-
-                // Variable changed in loop code
-                if (Token::Match(tok2->previous(), "for|while (")) {
-                    const Token * const start = tok2->link()->next();
-                    const Token * const end   = start->link();
-
-                    if (isVariableChanged(start,end,varid)) {
-                        varid = 0U;
-                        if (settings->debugwarnings)
-                            bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
-                    }
-                }
-
-                // if,macro => bailout
-                else if (Token::simpleMatch(tok2->previous(), "if (") && tok2->previous()->isExpandedMacro()) {
-                    varid = 0U;
-                    if (settings->debugwarnings)
-                        bailout(tokenlist, errorLogger, tok, "variable " + var->name() + ", condition is defined in macro");
-                }
-            }
-            if (varid == 0U)
-                continue;
-
-            // extra logic for unsigned variables 'i>=1' => possible value can also be 0
-            ValueFlow::Value val(tok, num);
-            val.varId = varid;
-            if (Token::Match(tok, "<|>")) {
-                if (num != 0)
-                    continue;
-                if (!var->typeStartToken()->isUnsigned())
-                    continue;
-            }
-            ValueFlow::Value val2;
-            if (num==1U && Token::Match(tok,"<=|>=")) {
-                if (var->typeStartToken()->isUnsigned()) {
-                    val2 = ValueFlow::Value(tok,0);
-                    val2.varId = varid;
-                }
-            }
     for (Token *tok2 = tok->previous(); ; tok2 = tok2->previous()) {
-                if (!tok2 || tok2->next() == scope->classStart) {
+        if (!tok2 ||
-                    if (settings->debugwarnings) {
+            tok2 == startToken ||
-                        std::list<ErrorLogger::ErrorMessage::FileLocation> callstack;
+            (tok2->str() == "{" && tok2->scope()->type == Scope::ScopeType::eFunction)) {
-                        callstack.push_back(ErrorLogger::ErrorMessage::FileLocation(tok,tokenlist));
-                        ErrorLogger::ErrorMessage errmsg(callstack, Severity::debug, "iterated too far", "debugValueFlowBeforeCondition", false);
-                        errorLogger->reportErr(errmsg);
-                    }
             break;
         }
 
@@ -826,6 +739,111 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo
             }
         }
     }
+
+}
+
+static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
+{
+    const std::size_t functions = symboldatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symboldatabase->functionScopes[i];
+        for (Token* tok = const_cast<Token*>(scope->classStart); tok != scope->classEnd; tok = tok->next()) {
+            MathLib::bigint num = 0;
+            const Token *vartok = nullptr;
+            if (tok->isComparisonOp() && tok->astOperand1() && tok->astOperand2()) {
+                if (tok->astOperand1()->isName() && tok->astOperand2()->isNumber()) {
+                    vartok = tok->astOperand1();
+                    num = MathLib::toLongNumber(tok->astOperand2()->str());
+                } else if (tok->astOperand1()->isNumber() && tok->astOperand2()->isName()) {
+                    vartok = tok->astOperand2();
+                    num = MathLib::toLongNumber(tok->astOperand1()->str());
+                } else {
+                    continue;
+                }
+            } else if (Token::Match(tok->previous(), "if|while ( %name% %oror%|&&|)") ||
+                       Token::Match(tok, "%oror%|&& %name% %oror%|&&|)")) {
+                vartok = tok->next();
+                num = 0;
+            } else if (tok->str() == "!" && tok->astOperand1() && tok->astOperand1()->isName()) {
+                vartok = tok->astOperand1();
+                num = 0;
+            } else {
+                continue;
+            }
+
+            unsigned int varid = vartok->varId();
+            const Variable * const var = vartok->variable();
+
+            if (varid == 0U || !var)
+                continue;
+
+            // bailout: global non-const variables
+            if (!(var->isLocal() || var->isArgument()) && !var->isConst()) {
+                if (settings->debugwarnings)
+                    bailout(tokenlist, errorLogger, tok, "global variable " + var->name());
+                continue;
+            }
+
+            // bailout: for/while-condition, variable is changed in while loop
+            for (const Token *tok2 = tok; tok2; tok2 = tok2->astParent()) {
+                if (tok2->astParent() || tok2->str() != "(" || !Token::simpleMatch(tok2->link(), ") {"))
+                    continue;
+
+                // Variable changed in 3rd for-expression
+                if (Token::simpleMatch(tok2->previous(), "for (")) {
+                    if (tok2->astOperand2() && isVariableChanged(tok2->astOperand2()->astOperand2(), tok2->link(), varid)) {
+                        varid = 0U;
+                        if (settings->debugwarnings)
+                            bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
+                    }
+                }
+
+                // Variable changed in loop code
+                if (Token::Match(tok2->previous(), "for|while (")) {
+                    const Token * const start = tok2->link()->next();
+                    const Token * const end   = start->link();
+
+                    if (isVariableChanged(start,end,varid)) {
+                        varid = 0U;
+                        if (settings->debugwarnings)
+                            bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
+                    }
+                }
+
+                // if,macro => bailout
+                else if (Token::simpleMatch(tok2->previous(), "if (") && tok2->previous()->isExpandedMacro()) {
+                    varid = 0U;
+                    if (settings->debugwarnings)
+                        bailout(tokenlist, errorLogger, tok, "variable " + var->name() + ", condition is defined in macro");
+                }
+            }
+            if (varid == 0U)
+                continue;
+
+            // extra logic for unsigned variables 'i>=1' => possible value can also be 0
+            ValueFlow::Value val(tok, num);
+            val.varId = varid;
+            if (Token::Match(tok, "<|>")) {
+                if (num != 0)
+                    continue;
+                if (!var->typeStartToken()->isUnsigned())
+                    continue;
+            }
+            ValueFlow::Value val2;
+            if (num==1U && Token::Match(tok,"<=|>=")) {
+                if (var->typeStartToken()->isUnsigned()) {
+                    val2 = ValueFlow::Value(tok,0);
+                    val2.varId = varid;
+                }
+            }
+            valueFlowReverse(tokenlist,
+                             tok,
+                             vartok,
+                             val,
+                             val2,
+                             errorLogger,
+                             settings);
+
         }
     }
 }
@@ -1898,6 +1916,52 @@ static void valueFlowInjectParameter(TokenList* tokenlist, ErrorLogger* errorLog
     valueFlowForward(const_cast<Token*>(functionScope->classStart->next()), functionScope->classEnd, arg, varid2, argvalues, true, tokenlist, errorLogger, settings);
 }
 
+static void valueFlowSwitchVariable(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
+{
+    for (std::list<Scope>::iterator scope = symboldatabase->scopeList.begin(); scope != symboldatabase->scopeList.end(); ++scope) {
+        if (scope->type != Scope::ScopeType::eSwitch)
+            continue;
+        if (!Token::Match(scope->classDef, "switch ( %var% ) {"))
+            continue;
+        const Token *vartok = scope->classDef->tokAt(2);
+        if (!vartok->variable())
+            continue;
+
+        // bailout: global non-const variables
+        const Variable *var = vartok->variable();
+        if (!(var->isLocal() || var->isArgument()) && !var->isConst()) {
+            if (settings->debugwarnings)
+                bailout(tokenlist, errorLogger, vartok, "switch variable " + var->name() + " is global");
+            continue;
+        }
+
+        for (Token *tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
+            if (tok->str() == "{") {
+                tok = tok->link();
+                continue;
+            }
+            if (Token::Match(tok, "case %num% :")) {
+                std::list<ValueFlow::Value> values;
+                values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str())));
+                while (Token::Match(tok->tokAt(3), "case %num% :")) {
+                    tok = tok->tokAt(3);
+                    values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str())));
+                }
+                for (std::list<ValueFlow::Value>::const_iterator val = values.begin(); val != values.end(); ++val) {
+                    valueFlowReverse(tokenlist,
+                                     const_cast<Token*>(scope->classDef),
+                                     vartok,
+                                     *val,
+                                     ValueFlow::Value(),
+                                     errorLogger,
+                                     settings);
+                }
+                valueFlowForward(tok, vartok->variable()->scope()->classEnd, vartok->variable(), vartok->varId(), values, false, tokenlist, errorLogger, settings);
+            }
+        }
+    }
+}
+
 static void valueFlowSubFunction(TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
 {
     for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
@@ -2063,6 +2127,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
     valueFlowBeforeCondition(tokenlist, symboldatabase, errorLogger, settings);
     valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings);
     valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings);
+    valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings);
     valueFlowSubFunction(tokenlist, errorLogger, settings);
     valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings);
 }

test/testvalueflow.cpp

@@ -60,6 +60,8 @@ private:
 
         TEST_CASE(valueFlowAfterCondition);
 
+        TEST_CASE(valueFlowSwitchVariable);
+
         TEST_CASE(valueFlowForLoop);
         TEST_CASE(valueFlowSubFunction);
         TEST_CASE(valueFlowFunctionReturn);
@@ -1228,6 +1230,20 @@ private:
         ASSERT_EQUALS(false, testValueOfX(code,3U,0x80));
     }
 
+    void valueFlowSwitchVariable() {
+        const char *code;
+        code = "void f(int x) {\n"
+               "    a = x;\n"  // <- x can be 14
+               "    switch (x) {\n"
+               "    case 14: a=x; break;\n"  // <- x is 14
+               "    };\n"
+               "    a = x;\n"  // <- x can be 14
+               "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 2U, 14));
+        ASSERT_EQUALS(true, testValueOfX(code, 4U, 14));
+        ASSERT_EQUALS(true, testValueOfX(code, 6U, 14));
+    }
+
     void valueFlowForLoop() {
         const char *code;