library: Add new warning: ignoredReturnErrorCode (#2877)

* library: Add optional "type" attribute to "use-retval"

Added an optional "type" attribute to "use-retval" nodes in the
configuration. When the return type of a function configured with
`<use-retval type="error-code"\>` node does not used, the new style
error "ignoredReturnErrorCode" will be generated.

* Fix and improve patch after the initial review

* Fixed severity level and [[nodiscard]] attribute

* Fix incorrect condition

* Remove redundant condition
This commit is contained in:
Georgy Komarov 2020-11-05 15:35:52 +03:00 committed by GitHub
parent 6f7f508967
commit 80dee36e68
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 51 additions and 13 deletions

View File

@ -192,7 +192,7 @@ void CheckFunctions::invalidFunctionArgStrError(const Token *tok, const std::str
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckFunctions::checkIgnoredReturnValue() void CheckFunctions::checkIgnoredReturnValue()
{ {
if (!mSettings->isEnabled(Settings::WARNING)) if (!mSettings->isEnabled(Settings::WARNING) && !mSettings->isEnabled(Settings::STYLE))
return; return;
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
@ -216,9 +216,15 @@ void CheckFunctions::checkIgnoredReturnValue()
} }
if ((!tok->function() || !Token::Match(tok->function()->retDef, "void %name%")) && if ((!tok->function() || !Token::Match(tok->function()->retDef, "void %name%")) &&
(mSettings->library.isUseRetVal(tok) || (tok->function() && tok->function()->isAttributeNodiscard())) &&
!WRONG_DATA(!tok->next()->astOperand1(), tok)) { !WRONG_DATA(!tok->next()->astOperand1(), tok)) {
const Library::UseRetValType retvalTy = mSettings->library.getUseRetValType(tok);
if (mSettings->isEnabled(Settings::WARNING) &&
((retvalTy == Library::UseRetValType::DEFAULT) ||
(tok->function() && tok->function()->isAttributeNodiscard())))
ignoredReturnValueError(tok, tok->next()->astOperand1()->expressionString()); ignoredReturnValueError(tok, tok->next()->astOperand1()->expressionString());
else if (mSettings->isEnabled(Settings::STYLE) &&
retvalTy == Library::UseRetValType::ERROR_CODE)
ignoredReturnErrorCode(tok, tok->next()->astOperand1()->expressionString());
} }
} }
} }
@ -230,6 +236,11 @@ void CheckFunctions::ignoredReturnValueError(const Token* tok, const std::string
"$symbol:" + function + "\nReturn value of function $symbol() is not used.", CWE252, false); "$symbol:" + function + "\nReturn value of function $symbol() is not used.", CWE252, false);
} }
void CheckFunctions::ignoredReturnErrorCode(const Token* tok, const std::string& function)
{
reportError(tok, Severity::style, "ignoredReturnErrorCode",
"$symbol:" + function + "\nError code from the return value of function $symbol() is not used.", CWE252, false);
}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// Detect passing wrong values to <cmath> functions like atan(0, x); // Detect passing wrong values to <cmath> functions like atan(0, x);

View File

@ -109,6 +109,7 @@ private:
void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr); void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr);
void invalidFunctionArgStrError(const Token *tok, const std::string &functionName, nonneg int argnr); void invalidFunctionArgStrError(const Token *tok, const std::string &functionName, nonneg int argnr);
void ignoredReturnValueError(const Token* tok, const std::string& function); void ignoredReturnValueError(const Token* tok, const std::string& function);
void ignoredReturnErrorCode(const Token* tok, const std::string& function);
void mathfunctionCallWarning(const Token *tok, const nonneg int numParam = 1); void mathfunctionCallWarning(const Token *tok, const nonneg int numParam = 1);
void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp); void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp);
void memsetZeroBytesError(const Token *tok); void memsetZeroBytesError(const Token *tok);

View File

@ -655,9 +655,12 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
func.isconst = true; // a constant function is pure func.isconst = true; // a constant function is pure
} else if (functionnodename == "leak-ignore") } else if (functionnodename == "leak-ignore")
func.leakignore = true; func.leakignore = true;
else if (functionnodename == "use-retval") else if (functionnodename == "use-retval") {
func.useretval = true; func.useretval = Library::UseRetValType::DEFAULT;
else if (functionnodename == "returnValue") { if (const char *type = functionnode->Attribute("type"))
if (std::strcmp(type, "error-code") == 0)
func.useretval = Library::UseRetValType::ERROR_CODE;
} else if (functionnodename == "returnValue") {
if (const char *expr = functionnode->GetText()) if (const char *expr = functionnode->GetText())
mReturnValue[name] = expr; mReturnValue[name] = expr;
if (const char *type = functionnode->Attribute("type")) if (const char *type = functionnode->Attribute("type"))
@ -1278,14 +1281,14 @@ bool Library::formatstr_secure(const Token* ftok) const
return functions.at(getFunctionName(ftok)).formatstr_secure; return functions.at(getFunctionName(ftok)).formatstr_secure;
} }
bool Library::isUseRetVal(const Token* ftok) const Library::UseRetValType Library::getUseRetValType(const Token *ftok) const
{ {
if (isNotLibraryFunction(ftok)) if (isNotLibraryFunction(ftok))
return false; return Library::UseRetValType::NONE;
const std::map<std::string, Function>::const_iterator it = functions.find(getFunctionName(ftok)); const std::map<std::string, Function>::const_iterator it = functions.find(getFunctionName(ftok));
if (it != functions.cend()) if (it != functions.cend())
return it->second.useretval; return it->second.useretval;
return false; return Library::UseRetValType::NONE;
} }
const std::string& Library::returnValue(const Token *ftok) const const std::string& Library::returnValue(const Token *ftok) const

View File

@ -179,7 +179,8 @@ public:
bool isNotLibraryFunction(const Token *ftok) const; bool isNotLibraryFunction(const Token *ftok) const;
bool matchArguments(const Token *ftok, const std::string &functionName) const; bool matchArguments(const Token *ftok, const std::string &functionName) const;
bool isUseRetVal(const Token* ftok) const; enum class UseRetValType { NONE, DEFAULT, ERROR_CODE };
UseRetValType getUseRetValType(const Token* ftok) const;
const std::string& returnValue(const Token *ftok) const; const std::string& returnValue(const Token *ftok) const;
const std::string& returnValueType(const Token *ftok) const; const std::string& returnValueType(const Token *ftok) const;
@ -306,12 +307,12 @@ public:
bool leakignore; bool leakignore;
bool isconst; bool isconst;
bool ispure; bool ispure;
bool useretval; UseRetValType useretval;
bool ignore; // ignore functions/macros from a library (gtk, qt etc) bool ignore; // ignore functions/macros from a library (gtk, qt etc)
bool formatstr; bool formatstr;
bool formatstr_scan; bool formatstr_scan;
bool formatstr_secure; bool formatstr_secure;
Function() : use(false), leakignore(false), isconst(false), ispure(false), useretval(false), ignore(false), formatstr(false), formatstr_scan(false), formatstr_secure(false) {} Function() : use(false), leakignore(false), isconst(false), ispure(false), useretval(UseRetValType::NONE), ignore(false), formatstr(false), formatstr_scan(false), formatstr_secure(false) {}
}; };
const Function *getFunction(const Token *ftok) const; const Function *getFunction(const Token *ftok) const;

View File

@ -76,6 +76,7 @@ private:
// Ignored return value // Ignored return value
TEST_CASE(checkIgnoredReturnValue); TEST_CASE(checkIgnoredReturnValue);
TEST_CASE(checkIgnoredErrorCode);
// memset.. // memset..
TEST_CASE(memsetZeroBytes); TEST_CASE(memsetZeroBytes);
@ -1232,6 +1233,27 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void checkIgnoredErrorCode() {
Settings settings2;
settings2.addEnabled("style");
const char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def version=\"2\">\n"
" <function name=\"mystrcmp\">\n"
" <use-retval type=\"error-code\"/>\n"
" <arg nr=\"1\"/>\n"
" <arg nr=\"2\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings2.library.load(doc);
check("void foo() {\n"
" mystrcmp(a, b);\n"
"}", "test.cpp", &settings2);
ASSERT_EQUALS("[test.cpp:2]: (style) Error code from the return value of function mystrcmp() is not used.\n", errout.str());
}
void memsetZeroBytes() { void memsetZeroBytes() {
check("void f() {\n" check("void f() {\n"
" memset(p, 10, 0x0);\n" " memset(p, 10, 0x0);\n"