Fix 10436: hang: valueFlowSubFunction 'ispunct(c)..' (#3423)

lib/programmemory.cpp

@@ -629,7 +629,26 @@ void execute(const Token* expr,
         }
     }
 
-    else if (expr->isArithmeticalOp() && expr->astOperand1() && expr->astOperand2()) {
+    else if (expr->str() == "&&") {
+        bool error1 = false;
+        execute(expr->astOperand1(), programMemory, result, &error1, f);
+        if (!error1 && *result == 0)
+            *result = 0;
+        else {
+            bool error2 = false;
+            execute(expr->astOperand2(), programMemory, result, &error2, f);
+            if (error1 || error2)
+                *error = true;
+        }
+    }
+
+    else if (expr->str() == "||") {
+        execute(expr->astOperand1(), programMemory, result, error, f);
+        if (*result == 1 && *error == false)
+            *result = 1;
+        else if (*result == 0 && *error == false)
+            execute(expr->astOperand2(), programMemory, result, error, f);
+    } else if (expr->isConstOp() && expr->astOperand1() && expr->astOperand2()) {
         MathLib::bigint result1(0), result2(0);
         execute(expr->astOperand1(), programMemory, &result1, error, f);
         execute(expr->astOperand2(), programMemory, &result2, error, f);
@@ -660,36 +679,24 @@ void execute(const Token* expr,
             } else {
                 *result = result1 >> result2;
             }
+        } else if (expr->str() == "&") {
+            *result = result1 & result2;
+        } else if (expr->str() == "|") {
+            *result = result1 | result2;
+        } else {
+            *error = true;
         }
     }
 
-    else if (expr->str() == "&&") {
-        bool error1 = false;
-        execute(expr->astOperand1(), programMemory, result, &error1, f);
-        if (!error1 && *result == 0)
-            *result = 0;
-        else {
-            bool error2 = false;
-            execute(expr->astOperand2(), programMemory, result, &error2, f);
-            if (error1 || error2)
-                *error = true;
-        }
-    }
-
-    else if (expr->str() == "||") {
-        execute(expr->astOperand1(), programMemory, result, error, f);
-        if (*result == 1 && *error == false)
-            *result = 1;
-        else if (*result == 0 && *error == false)
-            execute(expr->astOperand2(), programMemory, result, error, f);
-    }
-
     else if (expr->str() == "!") {
         execute(expr->astOperand1(), programMemory, result, error, f);
         *result = !(*result);
-    }
+    } else if (expr->isUnaryOp("-")) {
-
+        execute(expr->astOperand1(), programMemory, result, error, f);
-    else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) {
+        *result = -(*result);
+    } else if (expr->isUnaryOp("+")) {
+        execute(expr->astOperand1(), programMemory, result, error, f);
+    } else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) {
         execute(expr->astOperand1(), programMemory, result, error, f);
         execute(expr->astOperand2(), programMemory, result, error, f);
     }
@@ -719,6 +726,16 @@ void execute(const Token* expr,
             *result = 0;
         else
             *error = true;
+    } else if (expr->str() == "?" && expr->astOperand1() && expr->astOperand2()) {
+        execute(expr->astOperand1(), programMemory, result, error, f);
+        if (*error)
+            return;
+        const Token* childTok = expr->astOperand2();
+        if (*result == 0)
+            execute(childTok->astOperand2(), programMemory, result, error, f);
+        else
+            execute(childTok->astOperand1(), programMemory, result, error, f);
+
     } else if (expr->str() == "(" && expr->isCast()) {
         if (Token::simpleMatch(expr->previous(), ">") && expr->previous()->link())
             execute(expr->astOperand2(), programMemory, result, error);

lib/valueflow.cpp

@@ -103,6 +103,7 @@
 #include <cmath>
 #include <cstddef>
 #include <cstdint>
+#include <cstdlib>
 #include <cstring>
 #include <functional>
 #include <iterator>
@@ -6011,9 +6012,10 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer {
     }
 };
 
-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)
+template<class Key, class F>
+bool productParams(const std::unordered_map<Key, std::list<ValueFlow::Value>>& vars, F f)
 {
-    using Args = std::vector<std::unordered_map<const Variable*, ValueFlow::Value>>;
+    using Args = std::vector<std::unordered_map<Key, ValueFlow::Value>>;
     Args args(1);
     // Compute cartesian product of all arguments
     for (const auto& p:vars) {
@@ -6022,15 +6024,10 @@ static void valueFlowInjectParameter(TokenList* tokenlist, SymbolDatabase* symbo
         args.back()[p.first] = p.second.front();
     }
     for (const auto& p:vars) {
-        if (args.size() > 256) {
+        if (args.size() > 256)
-            std::string fname = "<unknown>";
+            return false;
-            Function* f = functionScope->function;
+        if (p.second.empty())
-            if (f)
+            continue;
-                fname = f->name();
-            if (settings->debugwarnings)
-                bailout(tokenlist, errorLogger, functionScope->bodyStart, "Too many argument passed to " + fname);
-            break;
-        }
         std::for_each(std::next(p.second.begin()), p.second.end(), [&](const ValueFlow::Value& value) {
             Args new_args;
             for (auto arg:args) {
@@ -6063,8 +6060,29 @@ static void valueFlowInjectParameter(TokenList* tokenlist, SymbolDatabase* symbo
         }
         if (skip)
             continue;
+        f(arg);
+    }
+    return true;
+}
+
+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)
+{
+    bool r = productParams(vars, [&](const std::unordered_map<const Variable*, ValueFlow::Value>& arg) {
         MultiValueFlowAnalyzer a(arg, tokenlist, symboldatabase);
         valueFlowGenericForward(const_cast<Token*>(functionScope->bodyStart), functionScope->bodyEnd, a, settings);
+    });
+    if (!r) {
+        std::string fname = "<unknown>";
+        Function* f = functionScope->function;
+        if (f)
+            fname = f->name();
+        if (settings->debugwarnings)
+            bailout(tokenlist, errorLogger, functionScope->bodyStart, "Too many argument passed to " + fname);
     }
 }
 
@@ -6174,140 +6192,6 @@ static void setTokenValues(Token *tok, const std::list<ValueFlow::Value> &values
     }
 }
 
-static bool evaluate(const Token *expr, const std::vector<std::list<ValueFlow::Value>> &values, std::list<ValueFlow::Value> *result)
-{
-    if (!expr)
-        return false;
-
-    // strlen(arg)..
-    if (expr->str() == "(" && Token::Match(expr->previous(), "strlen ( %name% )")) {
-        const Token *arg = expr->next();
-        if (arg->str().compare(0,3,"arg") != 0 || arg->str().size() != 4)
-            return false;
-        const char n = arg->str()[3];
-        if (n < '1' || n - '1' >= values.size())
-            return false;
-        for (const ValueFlow::Value &argvalue : values[n - '1']) {
-            if (argvalue.isTokValue() && argvalue.tokvalue->tokType() == Token::eString) {
-                ValueFlow::Value res(argvalue); // copy all "inconclusive", "condition", etc attributes
-                // set return value..
-                res.valueType = ValueFlow::Value::ValueType::INT;
-                res.tokvalue = nullptr;
-                res.intvalue = Token::getStrLength(argvalue.tokvalue);
-                result->emplace_back(std::move(res));
-            }
-        }
-        return !result->empty();
-    }
-
-    // unary operands
-    if (expr->astOperand1() && !expr->astOperand2()) {
-        std::list<ValueFlow::Value> opvalues;
-        if (!evaluate(expr->astOperand1(), values, &opvalues))
-            return false;
-        if (expr->str() == "+") {
-            result->swap(opvalues);
-            return true;
-        }
-        if (expr->str() == "-") {
-            for (ValueFlow::Value v: opvalues) {
-                if (v.isIntValue()) {
-                    v.intvalue = -v.intvalue;
-                    result->emplace_back(std::move(v));
-                }
-            }
-            return true;
-        }
-        return false;
-    }
-    // binary/ternary operands
-    if (expr->astOperand1() && expr->astOperand2()) {
-        std::list<ValueFlow::Value> lhsValues, rhsValues;
-        if (!evaluate(expr->astOperand1(), values, &lhsValues))
-            return false;
-        if (expr->str() != "?" && !evaluate(expr->astOperand2(), values, &rhsValues))
-            return false;
-
-        for (const ValueFlow::Value &val1 : lhsValues) {
-            if (!val1.isIntValue())
-                continue;
-            if (expr->str() == "?") {
-                rhsValues.clear();
-                const Token *expr2 = val1.intvalue ? expr->astOperand2()->astOperand1() : expr->astOperand2()->astOperand2();
-                if (!evaluate(expr2, values, &rhsValues))
-                    continue;
-                result->insert(result->end(), rhsValues.begin(), rhsValues.end());
-                continue;
-            }
-
-            for (const ValueFlow::Value &val2 : rhsValues) {
-                if (!val2.isIntValue())
-                    continue;
-
-                if (val1.varId != 0 && val2.varId != 0) {
-                    if (val1.varId != val2.varId || val1.varvalue != val2.varvalue)
-                        continue;
-                }
-
-                if (expr->str() == "+")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue + val2.intvalue));
-                else if (expr->str() == "-")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue - val2.intvalue));
-                else if (expr->str() == "*")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue * val2.intvalue));
-                else if (expr->str() == "/" && val2.intvalue != 0)
-                    result->emplace_back(ValueFlow::Value(val1.intvalue / val2.intvalue));
-                else if (expr->str() == "%" && val2.intvalue != 0)
-                    result->emplace_back(ValueFlow::Value(val1.intvalue % val2.intvalue));
-                else if (expr->str() == "&")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue & val2.intvalue));
-                else if (expr->str() == "|")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue | val2.intvalue));
-                else if (expr->str() == "^")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue ^ val2.intvalue));
-                else if (expr->str() == "==")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue == val2.intvalue));
-                else if (expr->str() == "!=")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue != val2.intvalue));
-                else if (expr->str() == "<")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue < val2.intvalue));
-                else if (expr->str() == ">")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue > val2.intvalue));
-                else if (expr->str() == ">=")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue >= val2.intvalue));
-                else if (expr->str() == "<=")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue <= val2.intvalue));
-                else if (expr->str() == "&&")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue && val2.intvalue));
-                else if (expr->str() == "||")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue || val2.intvalue));
-                else if (expr->str() == "<<")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue << val2.intvalue));
-                else if (expr->str() == ">>")
-                    result->emplace_back(ValueFlow::Value(val1.intvalue >> val2.intvalue));
-                else
-                    return false;
-                combineValueProperties(val1, val2, &result->back());
-            }
-        }
-        return !result->empty();
-    }
-    if (expr->str().compare(0,3,"arg")==0) {
-        *result = values[expr->str()[3] - '1'];
-        return true;
-    }
-    if (expr->isNumber()) {
-        result->emplace_back(ValueFlow::Value(MathLib::toLongNumber(expr->str())));
-        result->back().setKnown();
-        return true;
-    } else if (expr->tokType() == Token::eChar) {
-        result->emplace_back(ValueFlow::Value(MathLib::toLongNumber(expr->str())));
-        result->back().setKnown();
-        return true;
-    }
-    return false;
-}
-
 static std::list<ValueFlow::Value> getFunctionArgumentValues(const Token *argtok)
 {
     std::list<ValueFlow::Value> argvalues(argtok->values());
@@ -6321,11 +6205,11 @@ static std::list<ValueFlow::Value> getFunctionArgumentValues(const Token *argtok
 
 static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue, const Settings *settings)
 {
-    std::vector<std::list<ValueFlow::Value>> argValues;
+    std::unordered_map<nonneg int, std::list<ValueFlow::Value>> argValues;
+    int argn = 1;
     for (const Token *argtok : getArguments(tok->previous())) {
-        argValues.emplace_back(getFunctionArgumentValues(argtok));
+        argValues[argn] = getFunctionArgumentValues(argtok);
-        if (argValues.back().empty())
+        argn++;
-            return;
     }
     if (returnValue.find("arg") != std::string::npos && argValues.empty())
         return;
@@ -6356,11 +6240,38 @@ static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue,
     if (!lpar.empty())
         return;
 
+    // set varids
+    for (Token* tok2 = tokenList.front(); tok2; tok2 = tok2->next()) {
+        if (tok2->str().compare(0, 3, "arg") != 0)
+            continue;
+        nonneg int id = std::atoi(tok2->str().c_str() + 3);
+        tok2->varId(id);
+    }
+
     // Evaluate expression
     tokenList.createAst();
-    std::list<ValueFlow::Value> results;
+    Token* expr = tokenList.front()->astOperand1();
-    if (evaluate(tokenList.front()->astOperand1(), argValues, &results))
+    ValueFlow::valueFlowConstantFoldAST(expr, settings);
-        setTokenValues(tok, results, settings);
+
+    productParams(argValues, [&](const std::unordered_map<nonneg int, ValueFlow::Value>& arg) {
+        ProgramMemory pm{arg};
+        MathLib::bigint result = 0;
+        bool error = false;
+        execute(expr, &pm, &result, &error);
+        if (error)
+            return;
+        ValueFlow::Value value(result);
+        value.setKnown();
+        for (auto&& p : arg) {
+            if (p.second.isPossible())
+                value.setPossible();
+            if (p.second.isInconclusive()) {
+                value.setInconclusive();
+                break;
+            }
+        }
+        setTokenValue(tok, value, settings);
+    });
 }
 
 static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldatabase,  ErrorLogger* errorLogger, const Settings* settings)

test/testvalueflow.cpp

@@ -5978,6 +5978,17 @@ private:
                "    for (auto *el = root->FirstChildElement(\"Result\"); el && !ParseAddItem(GetItem(el)); el = el->NextSiblingElement(\"Result\")) ;\n"
                "}\n";
         valueOfTok(code, "root");
+
+        code = "bool isCharPotentialOperator(char ch)  {\n"
+               "    return (ispunct((unsigned char) ch)\n"
+               "            && ch != '{' && ch != '}'\n"
+               "            && ch != '(' && ch != ')'\n"
+               "            && ch != '[' && ch != ']'\n"
+               "            && ch != ';' && ch != ','\n"
+               "            && ch != '#' && ch != '\\'\n"
+               "            && ch != '\'' && ch != '\"');\n"
+               "}\n";
+        valueOfTok(code, "return");
     }
 
     void valueFlowCrashConstructorInitialization() { // #9577