Fix 10449: Regression: knownConditionTrueFalse (strlen/wcslen) (#3977)

* Refactor library function usage

* Evaluate library function in program memory

* Fix and add tests

* Format
This commit is contained in:
Paul Fultz II 2022-04-05 23:35:38 -05:00 committed by GitHub
parent 1d92665ad2
commit 4fde7f8b18
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 137 additions and 55 deletions

View File

@ -1651,3 +1651,50 @@ Library::TypeCheck Library::getTypeCheck(const std::string &check, const std::st
auto it = mTypeChecks.find(std::pair<std::string, std::string>(check, typeName)); auto it = mTypeChecks.find(std::pair<std::string, std::string>(check, typeName));
return it == mTypeChecks.end() ? TypeCheck::def : it->second; return it == mTypeChecks.end() ? TypeCheck::def : it->second;
} }
std::shared_ptr<Token> createTokenFromExpression(const std::string& returnValue,
const Settings* settings,
std::unordered_map<nonneg int, const Token*>* lookupVarId)
{
std::shared_ptr<TokenList> tokenList = std::make_shared<TokenList>(settings);
{
const std::string code = "return " + returnValue + ";";
std::istringstream istr(code);
if (!tokenList->createTokens(istr))
return nullptr;
}
// combine operators, set links, etc..
std::stack<Token*> 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};
}

View File

@ -28,6 +28,7 @@
#include <cstddef> #include <cstddef>
#include <map> #include <map>
#include <memory>
#include <set> #include <set>
#include <string> #include <string>
#include <unordered_map> #include <unordered_map>
@ -35,6 +36,8 @@
#include <vector> #include <vector>
class Token; class Token;
class TokenList;
class Settings;
namespace tinyxml2 { namespace tinyxml2 {
class XMLDocument; class XMLDocument;
@ -650,6 +653,10 @@ private:
CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok); CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok);
std::shared_ptr<Token> createTokenFromExpression(const std::string& returnValue,
const Settings* settings,
std::unordered_map<nonneg int, const Token*>* lookupVarId = nullptr);
/// @} /// @}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
#endif // libraryH #endif // libraryH

View File

@ -730,6 +730,36 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const
} }
if (Token::Match(expr->previous(), ">|%name% {|(")) { 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<nonneg int, ValueFlow::Value> 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) { visitAstNodes(expr->astOperand2(), [&](const Token* child) {
if (child->exprId() > 0 && pm.hasValue(child->exprId())) { if (child->exprId() > 0 && pm.hasValue(child->exprId())) {
ValueFlow::Value& v = pm.at(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; return v;
} }
ValueFlow::Value evaluateLibraryFunction(const std::unordered_map<nonneg int, ValueFlow::Value>& args,
const std::string& returnValue,
const Settings* settings)
{
static std::unordered_map<std::string,
std::function<ValueFlow::Value(const std::unordered_map<nonneg int, ValueFlow::Value>& arg)>>
functions = {};
if (functions.count(returnValue) == 0) {
std::unordered_map<nonneg int, const Token*> lookupVarId;
std::shared_ptr<Token> expr = createTokenFromExpression(returnValue, settings, &lookupVarId);
functions[returnValue] =
[lookupVarId, expr, settings](const std::unordered_map<nonneg int, ValueFlow::Value>& 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, void execute(const Token* expr,
ProgramMemory* const programMemory, ProgramMemory* const programMemory,
MathLib::bigint* result, MathLib::bigint* result,

View File

@ -165,6 +165,10 @@ ProgramMemory getProgramMemory(const Token* tok, const Token* expr, const ValueF
ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars); ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars);
ValueFlow::Value evaluateLibraryFunction(const std::unordered_map<nonneg int, ValueFlow::Value>& args,
const std::string& returnValue,
const Settings* settings);
#endif #endif

View File

@ -6759,69 +6759,23 @@ static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue,
} }
if (returnValue.find("arg") != std::string::npos && argValues.empty()) if (returnValue.find("arg") != std::string::npos && argValues.empty())
return; 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<Token *> 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<nonneg int, const Token*> 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<nonneg int, ValueFlow::Value>& arg) { productParams(argValues, [&](const std::unordered_map<nonneg int, ValueFlow::Value>& arg) {
ProgramMemory pm{}; ValueFlow::Value value = evaluateLibraryFunction(arg, returnValue, settings);
for (const auto& p : arg) { if (value.isUninitValue())
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)
return; return;
ValueFlow::Value value(result); ValueFlow::Value::ValueKind kind = ValueFlow::Value::ValueKind::Known;
value.setKnown();
for (auto&& p : arg) { for (auto&& p : arg) {
if (p.second.isPossible()) if (p.second.isPossible())
value.setPossible(); kind = p.second.valueKind;
if (p.second.isInconclusive()) { if (p.second.isInconclusive()) {
value.setInconclusive(); kind = p.second.valueKind;
break; break;
} }
} }
if (value.isImpossible() && kind != ValueFlow::Value::ValueKind::Known)
return;
if (!value.isImpossible())
value.valueKind = kind;
setTokenValue(tok, value, settings); setTokenValue(tok, value, settings);
}); });
} }

View File

@ -4017,6 +4017,18 @@ private:
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'w' is always true\n", errout.str()); 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() void alwaysTrueSymbolic()