From f8e9071de1a873b9ee30c63ebf6c7ef9758ae3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 20 Apr 2017 22:14:54 +0200 Subject: [PATCH] CheckFunction: In the check handle possible/conditional/inconclusive values better --- lib/checkfunctions.cpp | 38 +++++++++++++++++++++++--------------- lib/checkfunctions.h | 4 ++-- lib/token.cpp | 23 +++++++++++++++++++++++ lib/token.h | 2 ++ test/testfunctions.cpp | 2 +- 5 files changed, 51 insertions(+), 18 deletions(-) diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index c18c21ca5..f821c8b7a 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -96,38 +96,46 @@ void CheckFunctions::invalidFunctionUsage() const Token * const argtok = arguments[argnr-1]; // check ... - if (argtok->hasKnownIntValue() && - !_settings->library.isargvalid(functionToken,argnr,argtok->values().front().intvalue)) { - // TODO: Warn about possible values - invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr)); + const ValueFlow::Value *invalidValue = argtok->getInvalidValue(functionToken,argnr,_settings); + if (invalidValue) { + invalidFunctionArgError(argtok, functionToken->next()->astOperand1()->expressionString(), argnr, invalidValue, _settings->library.validarg(functionToken, argnr)); } if (astIsBool(argtok)) { // check if (_settings->library.isboolargbad(functionToken, argnr)) invalidFunctionArgBoolError(argtok, functionToken->str(), argnr); + // Are the values 0 and 1 valid? else if (!_settings->library.isargvalid(functionToken, argnr, 0)) - invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr)); + invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, _settings->library.validarg(functionToken, argnr)); else if (!_settings->library.isargvalid(functionToken, argnr, 1)) - invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr)); + invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, _settings->library.validarg(functionToken, argnr)); } } } } } -void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr) +void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const ValueFlow::Value *invalidValue, const std::string &validstr) { std::ostringstream errmsg; - errmsg << "Invalid " << functionName << "() argument nr " << argnr; - if (!tok) - ; - else if (tok->isNumber()) - errmsg << ". The value is " << tok->str() << " but the valid values are '" << validstr << "'."; - else if (tok->isComparisonOp()) - errmsg << ". The value is 0 or 1 (comparison result) but the valid values are '" << validstr << "'."; - reportError(tok, Severity::error, "invalidFunctionArg", errmsg.str(), CWE628, false); + if (invalidValue && invalidValue->condition) + errmsg << ValueFlow::eitherTheConditionIsRedundant(invalidValue->condition) + << " or " << functionName << "() argument nr " << argnr + << " can have invalid value."; + else + errmsg << "Invalid " << functionName << "() argument nr " << argnr << '.'; + if (invalidValue) + errmsg << " The value is " << invalidValue->intvalue << " but the valid values are '" << validstr << "'."; + else + errmsg << " The value is 0 or 1 (boolean) but the valid values are '" << validstr << "'."; + reportError(tok, + (!invalidValue || !invalidValue->condition) ? Severity::error : Severity::warning, + "invalidFunctionArg", + errmsg.str(), + CWE628, + invalidValue && invalidValue->inconclusive); } void CheckFunctions::invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr) diff --git a/lib/checkfunctions.h b/lib/checkfunctions.h index 088d8f257..ae4397d62 100644 --- a/lib/checkfunctions.h +++ b/lib/checkfunctions.h @@ -88,7 +88,7 @@ public: void checkLibraryMatchFunctions(); private: - void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr); + void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const ValueFlow::Value *invalidValue, const std::string &validstr); void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr); void ignoredReturnValueError(const Token* tok, const std::string& function); void mathfunctionCallWarning(const Token *tok, const unsigned int numParam = 1); @@ -101,7 +101,7 @@ private: c.reportError(nullptr, Severity::style, i->first+"Called", i->second.message); } - c.invalidFunctionArgError(nullptr, "func_name", 1, "1-4"); + c.invalidFunctionArgError(nullptr, "func_name", 1, nullptr,"1:4"); c.invalidFunctionArgBoolError(nullptr, "func_name", 1); c.ignoredReturnValueError(nullptr, "malloc"); c.mathfunctionCallWarning(nullptr); diff --git a/lib/token.cpp b/lib/token.cpp index fb43059bf..21844a757 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1442,6 +1442,29 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett return ret; } +const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, unsigned int argnr, const Settings *settings) const +{ + if (!_values) + return nullptr; + const ValueFlow::Value *ret = nullptr; + std::list::const_iterator it; + for (it = _values->begin(); it != _values->end(); ++it) { + if (it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) { + if (!ret || ret->inconclusive || (ret->condition && !it->inconclusive)) + ret = &(*it); + if (!ret->inconclusive && !ret->condition) + break; + } + } + if (settings && ret) { + if (ret->inconclusive && !settings->inconclusive) + return nullptr; + if (ret->condition && !settings->isEnabled(Settings::WARNING)) + return nullptr; + } + return ret; +} + const Token *Token::getValueTokenMinStrSize() const { if (!_values) diff --git a/lib/token.h b/lib/token.h index a42640168..2ec455b8c 100644 --- a/lib/token.h +++ b/lib/token.h @@ -793,6 +793,8 @@ public: const ValueFlow::Value * getValueLE(const MathLib::bigint val, const Settings *settings) const; const ValueFlow::Value * getValueGE(const MathLib::bigint val, const Settings *settings) const; + const ValueFlow::Value * getInvalidValue(const Token *ftok, unsigned int argnr, const Settings *settings) const; + const Token *getValueTokenMaxStrLength() const; const Token *getValueTokenMinStrSize() const; diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index c0e3ec4c9..97fa0da16 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -434,7 +434,7 @@ private: ASSERT_EQUALS("[test.cpp:2]: (error) Invalid memset() argument nr 3. A non-boolean value is required.\n", errout.str()); check("int f() { strtol(a,b,sizeof(a)!=12); }"); - ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strtol() argument nr 3. The value is 0 or 1 (comparison result) but the valid values are '0,2:36'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strtol() argument nr 3. The value is 0 or 1 (boolean) but the valid values are '0,2:36'.\n", errout.str()); check("int f() { strtol(a,b,1); }"); ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strtol() argument nr 3. The value is 1 but the valid values are '0,2:36'.\n", errout.str());