From 712ff1c07346c2022406aed2e6a0a972bf950279 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 26 Aug 2021 22:46:33 -0500 Subject: [PATCH] Fix 10436: hang: valueFlowSubFunction 'ispunct(c)..' (#3423) --- lib/programmemory.cpp | 67 ++++++++----- lib/valueflow.cpp | 215 ++++++++++++----------------------------- test/testvalueflow.cpp | 11 +++ 3 files changed, 116 insertions(+), 177 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 5fe3c0cce..b3eb27c74 100644 --- a/lib/programmemory.cpp +++ b/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->str() == "," && expr->astOperand1() && expr->astOperand2()) { + } else if (expr->isUnaryOp("-")) { + 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()) { 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); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index a469c551a..247398c2f 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -103,6 +103,7 @@ #include #include #include +#include #include #include #include @@ -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>& vars) +template +bool productParams(const std::unordered_map>& vars, F f) { - using Args = std::vector>; + using Args = std::vector>; 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) { - std::string fname = ""; - Function* f = functionScope->function; - if (f) - fname = f->name(); - if (settings->debugwarnings) - bailout(tokenlist, errorLogger, functionScope->bodyStart, "Too many argument passed to " + fname); - break; - } + if (args.size() > 256) + return false; + if (p.second.empty()) + continue; 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>& vars) +{ + bool r = productParams(vars, [&](const std::unordered_map& arg) { MultiValueFlowAnalyzer a(arg, tokenlist, symboldatabase); valueFlowGenericForward(const_cast(functionScope->bodyStart), functionScope->bodyEnd, a, settings); + }); + if (!r) { + std::string fname = ""; + 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 &values } } -static bool evaluate(const Token *expr, const std::vector> &values, std::list *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 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 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 getFunctionArgumentValues(const Token *argtok) { std::list argvalues(argtok->values()); @@ -6321,11 +6205,11 @@ static std::list getFunctionArgumentValues(const Token *argtok static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue, const Settings *settings) { - std::vector> argValues; + std::unordered_map> argValues; + int argn = 1; for (const Token *argtok : getArguments(tok->previous())) { - argValues.emplace_back(getFunctionArgumentValues(argtok)); - if (argValues.back().empty()) - return; + argValues[argn] = getFunctionArgumentValues(argtok); + argn++; } 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 results; - if (evaluate(tokenList.front()->astOperand1(), argValues, &results)) - setTokenValues(tok, results, settings); + Token* expr = tokenList.front()->astOperand1(); + ValueFlow::valueFlowConstantFoldAST(expr, settings); + + productParams(argValues, [&](const std::unordered_map& 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) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 9491d45cf..f5923a272 100644 --- a/test/testvalueflow.cpp +++ b/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