From 4fde7f8b18c5925aa916cf08c3f61cb5b934b246 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Tue, 5 Apr 2022 23:35:38 -0500 Subject: [PATCH] Fix 10449: Regression: knownConditionTrueFalse (strlen/wcslen) (#3977) * Refactor library function usage * Evaluate library function in program memory * Fix and add tests * Format --- lib/library.cpp | 47 +++++++++++++++++++++++++++++++ lib/library.h | 7 +++++ lib/programmemory.cpp | 58 ++++++++++++++++++++++++++++++++++++++ lib/programmemory.h | 4 +++ lib/valueflow.cpp | 64 ++++++------------------------------------ test/testcondition.cpp | 12 ++++++++ 6 files changed, 137 insertions(+), 55 deletions(-) diff --git a/lib/library.cpp b/lib/library.cpp index eede643fa..6a0377cf2 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -1651,3 +1651,50 @@ Library::TypeCheck Library::getTypeCheck(const std::string &check, const std::st auto it = mTypeChecks.find(std::pair(check, typeName)); return it == mTypeChecks.end() ? TypeCheck::def : it->second; } + +std::shared_ptr createTokenFromExpression(const std::string& returnValue, + const Settings* settings, + std::unordered_map* lookupVarId) +{ + std::shared_ptr tokenList = std::make_shared(settings); + { + const std::string code = "return " + returnValue + ";"; + std::istringstream istr(code); + if (!tokenList->createTokens(istr)) + return nullptr; + } + + // combine operators, set links, etc.. + std::stack lpar; + for (Token* tok2 = tokenList->front(); tok2; tok2 = tok2->next()) { + if (Token::Match(tok2, "[!<>=] =")) { + tok2->str(tok2->str() + "="); + tok2->deleteNext(); + } else if (tok2->str() == "(") + lpar.push(tok2); + else if (tok2->str() == ")") { + if (lpar.empty()) + return nullptr; + Token::createMutualLinks(lpar.top(), tok2); + lpar.pop(); + } + } + if (!lpar.empty()) + return nullptr; + + // 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); + if (lookupVarId) + (*lookupVarId)[id] = tok2; + } + + // Evaluate expression + tokenList->createAst(); + Token* expr = tokenList->front()->astOperand1(); + ValueFlow::valueFlowConstantFoldAST(expr, settings); + return {tokenList, expr}; +} diff --git a/lib/library.h b/lib/library.h index abbc8dbf1..99e70d764 100644 --- a/lib/library.h +++ b/lib/library.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -35,6 +36,8 @@ #include class Token; +class TokenList; +class Settings; namespace tinyxml2 { class XMLDocument; @@ -650,6 +653,10 @@ private: CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok); +std::shared_ptr createTokenFromExpression(const std::string& returnValue, + const Settings* settings, + std::unordered_map* lookupVarId = nullptr); + /// @} //--------------------------------------------------------------------------- #endif // libraryH diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index ed461deb1..37d5bcdd8 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -730,6 +730,36 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const } if (Token::Match(expr->previous(), ">|%name% {|(")) { + const Token* ftok = expr->previous(); + const Function* f = ftok->function(); + // TODO: Evaluate inline functions as well + if (!f && settings && expr->str() == "(") { + std::unordered_map args; + int argn = 0; + for (const Token* tok : getArguments(expr)) { + ValueFlow::Value result = execute(tok, pm, settings); + if (!result.isUninitValue()) + args[argn] = result; + argn++; + } + // strlen is a special builtin + if (Token::simpleMatch(ftok, "strlen")) { + if (args.count(0) > 0) { + ValueFlow::Value v = args.at(0); + if (v.isTokValue() && v.tokvalue->tokType() == Token::eString) { + v.valueType = ValueFlow::Value::ValueType::INT; + v.intvalue = Token::getStrLength(v.tokvalue); + v.tokvalue = nullptr; + return v; + } + } + } else { + const std::string& returnValue = settings->library.returnValue(ftok); + if (!returnValue.empty()) + return evaluateLibraryFunction(args, returnValue, settings); + } + } + // Check if functon modifies argument visitAstNodes(expr->astOperand2(), [&](const Token* child) { if (child->exprId() > 0 && pm.hasValue(child->exprId())) { ValueFlow::Value& v = pm.at(child->exprId()); @@ -774,6 +804,34 @@ static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Sett return v; } +ValueFlow::Value evaluateLibraryFunction(const std::unordered_map& args, + const std::string& returnValue, + const Settings* settings) +{ + static std::unordered_map& arg)>> + functions = {}; + if (functions.count(returnValue) == 0) { + + std::unordered_map lookupVarId; + std::shared_ptr expr = createTokenFromExpression(returnValue, settings, &lookupVarId); + + functions[returnValue] = + [lookupVarId, expr, settings](const std::unordered_map& xargs) { + if (!expr) + return ValueFlow::Value::unknown(); + ProgramMemory pm{}; + for (const auto& p : xargs) { + auto it = lookupVarId.find(p.first); + if (it != lookupVarId.end()) + pm.setValue(it->second, p.second); + } + return execute(expr.get(), pm, settings); + }; + } + return functions.at(returnValue)(args); +} + void execute(const Token* expr, ProgramMemory* const programMemory, MathLib::bigint* result, diff --git a/lib/programmemory.h b/lib/programmemory.h index f873f3f0f..bd705e718 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -165,6 +165,10 @@ ProgramMemory getProgramMemory(const Token* tok, const Token* expr, const ValueF ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars); +ValueFlow::Value evaluateLibraryFunction(const std::unordered_map& args, + const std::string& returnValue, + const Settings* settings); + #endif diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f4a488090..04e4a71d1 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -6759,69 +6759,23 @@ static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue, } if (returnValue.find("arg") != std::string::npos && argValues.empty()) return; - - TokenList tokenList(settings); - { - const std::string code = "return " + returnValue + ";"; - std::istringstream istr(code); - if (!tokenList.createTokens(istr)) - return; - } - - // combine operators, set links, etc.. - std::stack lpar; - for (Token *tok2 = tokenList.front(); tok2; tok2 = tok2->next()) { - if (Token::Match(tok2, "[!<>=] =")) { - tok2->str(tok2->str() + "="); - tok2->deleteNext(); - } else if (tok2->str() == "(") - lpar.push(tok2); - else if (tok2->str() == ")") { - if (lpar.empty()) - return; - Token::createMutualLinks(lpar.top(), tok2); - lpar.pop(); - } - } - if (!lpar.empty()) - return; - - // set varids - std::unordered_map lookupVarId; - 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); - lookupVarId[id] = tok2; - } - - // Evaluate expression - tokenList.createAst(); - Token* expr = tokenList.front()->astOperand1(); - ValueFlow::valueFlowConstantFoldAST(expr, settings); - productParams(argValues, [&](const std::unordered_map& arg) { - ProgramMemory pm{}; - for (const auto& p : arg) { - const Token* varTok = lookupVarId[p.first]; - pm.setValue(varTok, p.second); - } - MathLib::bigint result = 0; - bool error = false; - execute(expr, &pm, &result, &error); - if (error) + ValueFlow::Value value = evaluateLibraryFunction(arg, returnValue, settings); + if (value.isUninitValue()) return; - ValueFlow::Value value(result); - value.setKnown(); + ValueFlow::Value::ValueKind kind = ValueFlow::Value::ValueKind::Known; for (auto&& p : arg) { if (p.second.isPossible()) - value.setPossible(); + kind = p.second.valueKind; if (p.second.isInconclusive()) { - value.setInconclusive(); + kind = p.second.valueKind; break; } } + if (value.isImpossible() && kind != ValueFlow::Value::ValueKind::Known) + return; + if (!value.isImpossible()) + value.valueKind = kind; setTokenValue(tok, value, settings); }); } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index b426efc81..9f92089f6 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4017,6 +4017,18 @@ private: " }\n" "}\n"); ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'w' is always true\n", errout.str()); + + check("void f() {\n" + " if(strlen(\"abc\") == 3) {;}\n" + " if(strlen(\"abc\") == 1) {;}\n" + " if(wcslen(L\"abc\") == 3) {;}\n" + " if(wcslen(L\"abc\") == 1) {;}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'strlen(\"abc\")==3' is always true\n" + "[test.cpp:3]: (style) Condition 'strlen(\"abc\")==1' is always false\n" + "[test.cpp:4]: (style) Condition 'wcslen(L\"abc\")==3' is always true\n" + "[test.cpp:5]: (style) Condition 'wcslen(L\"abc\")==1' is always false\n", + errout.str()); } void alwaysTrueSymbolic()