From 5e9e92f87851613ba589377bbd6e7f66f0c950bc Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 5 Apr 2022 20:35:22 -0500 Subject: [PATCH 1/4] Refactor library function usage --- lib/library.cpp | 45 ++++++++++++++++++++++++++++++ lib/library.h | 5 ++++ lib/programmemory.cpp | 23 ++++++++++++++++ lib/programmemory.h | 2 ++ lib/valueflow.cpp | 64 ++++++------------------------------------- 5 files changed, 84 insertions(+), 55 deletions(-) diff --git a/lib/library.cpp b/lib/library.cpp index eede643fac2..170e1f63052 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -1651,3 +1651,48 @@ 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 abbc8dbf1ae..42e87bea240 100644 --- a/lib/library.h +++ b/lib/library.h @@ -31,10 +31,13 @@ #include #include #include +#include #include #include class Token; +class TokenList; +class Settings; namespace tinyxml2 { class XMLDocument; @@ -650,6 +653,8 @@ class CPPCHECKLIB Library { 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 ed461deb193..5edd897a24d 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -774,6 +774,29 @@ 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 f873f3f0f95..7b28dab7381 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -165,6 +165,8 @@ 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 a5c395b5727..fbb5d8b52a7 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); }); } From 2e7f7f87b2ae7979d3a972148d60af0f80e76267 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 5 Apr 2022 21:30:17 -0500 Subject: [PATCH 2/4] Evaluate library function in program memory --- lib/programmemory.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 5edd897a24d..a9f24787552 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -599,6 +599,36 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const else if (!v.isImpossible()) return ValueFlow::Value{v.intvalue == 0}; } + + } else if (Token::Match(expr->previous(), "%name% (")) { + const Token* ftok = expr->previous(); + const Function* f = ftok->function(); + // TODO: Evaluate inline functions + if (!f && settings) { + 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); + return evaluateLibraryFunction(args, returnValue, settings); + } + } } else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() && expr->astOperand1()->exprId() > 0) { ValueFlow::Value rhs = execute(expr->astOperand2(), pm); if (rhs.isUninitValue()) From 90b594c3c988b9eff2bdd07b18bca6780ae18198 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 5 Apr 2022 21:50:42 -0500 Subject: [PATCH 3/4] Fix and add tests --- lib/programmemory.cpp | 60 +++++++++++++++++++++--------------------- test/testcondition.cpp | 11 ++++++++ 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index a9f24787552..8ba9fde66d8 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -599,36 +599,6 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const else if (!v.isImpossible()) return ValueFlow::Value{v.intvalue == 0}; } - - } else if (Token::Match(expr->previous(), "%name% (")) { - const Token* ftok = expr->previous(); - const Function* f = ftok->function(); - // TODO: Evaluate inline functions - if (!f && settings) { - 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); - return evaluateLibraryFunction(args, returnValue, settings); - } - } } else if (expr->isAssignmentOp() && expr->astOperand1() && expr->astOperand2() && expr->astOperand1()->exprId() > 0) { ValueFlow::Value rhs = execute(expr->astOperand2(), pm); if (rhs.isUninitValue()) @@ -760,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()); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index b426efc8147..f5599ce5c67 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4017,6 +4017,17 @@ class TestCondition : public TestFixture { " }\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() From 34cb53df8ecb0015c3f3432717f19d30bd36a995 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 5 Apr 2022 21:52:48 -0500 Subject: [PATCH 4/4] Format --- lib/library.cpp | 8 +++++--- lib/library.h | 6 ++++-- lib/programmemory.cpp | 13 +++++++++---- lib/programmemory.h | 4 +++- test/testcondition.cpp | 3 ++- 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/library.cpp b/lib/library.cpp index 170e1f63052..6a0377cf205 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -1652,7 +1652,9 @@ Library::TypeCheck Library::getTypeCheck(const std::string &check, const std::st 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 createTokenFromExpression(const std::string& returnValue, + const Settings* settings, + std::unordered_map* lookupVarId) { std::shared_ptr tokenList = std::make_shared(settings); { @@ -1663,8 +1665,8 @@ std::shared_ptr createTokenFromExpression(const std::string& returnValue, } // combine operators, set links, etc.. - std::stack lpar; - for (Token *tok2 = tokenList->front(); tok2; tok2 = tok2->next()) { + std::stack lpar; + for (Token* tok2 = tokenList->front(); tok2; tok2 = tok2->next()) { if (Token::Match(tok2, "[!<>=] =")) { tok2->str(tok2->str() + "="); tok2->deleteNext(); diff --git a/lib/library.h b/lib/library.h index 42e87bea240..99e70d764a0 100644 --- a/lib/library.h +++ b/lib/library.h @@ -28,10 +28,10 @@ #include #include +#include #include #include #include -#include #include #include @@ -653,7 +653,9 @@ class CPPCHECKLIB Library { CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok); -std::shared_ptr createTokenFromExpression(const std::string& returnValue, const Settings* settings, std::unordered_map* lookupVarId = nullptr); +std::shared_ptr createTokenFromExpression(const std::string& returnValue, + const Settings* settings, + std::unordered_map* lookupVarId = nullptr); /// @} //--------------------------------------------------------------------------- diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 8ba9fde66d8..37d5bcdd8d5 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -736,7 +736,7 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const if (!f && settings && expr->str() == "(") { std::unordered_map args; int argn = 0; - for(const Token* tok:getArguments(expr)) { + for (const Token* tok : getArguments(expr)) { ValueFlow::Value result = execute(tok, pm, settings); if (!result.isUninitValue()) args[argn] = result; @@ -804,15 +804,20 @@ 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) +ValueFlow::Value evaluateLibraryFunction(const std::unordered_map& args, + const std::string& returnValue, + const Settings* settings) { - static std::unordered_map& arg)>> functions = {}; + 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) { + functions[returnValue] = + [lookupVarId, expr, settings](const std::unordered_map& xargs) { if (!expr) return ValueFlow::Value::unknown(); ProgramMemory pm{}; diff --git a/lib/programmemory.h b/lib/programmemory.h index 7b28dab7381..bd705e718c6 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -165,7 +165,9 @@ 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); +ValueFlow::Value evaluateLibraryFunction(const std::unordered_map& args, + const std::string& returnValue, + const Settings* settings); #endif diff --git a/test/testcondition.cpp b/test/testcondition.cpp index f5599ce5c67..9f92089f67c 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4027,7 +4027,8 @@ class TestCondition : public TestFixture { 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()); + "[test.cpp:5]: (style) Condition 'wcslen(L\"abc\")==1' is always false\n", + errout.str()); } void alwaysTrueSymbolic()