Issue 8830: New check: Function argument evaluates to constant value
Add a check for function arguments that can be constant: ```cpp extern void bar(int); void f(int x) { bar((x & 0x01) >> 7); // function 'bar' is always called with a '0'-argument } ```
This commit is contained in:
parent
2090866cd0
commit
9b973e652c
|
@ -1040,6 +1040,33 @@ bool isLikelyStreamRead(bool cpp, const Token *op)
|
||||||
return (!parent->astOperand1()->valueType() || !parent->astOperand1()->valueType()->isIntegral());
|
return (!parent->astOperand1()->valueType() || !parent->astOperand1()->valueType()->isIntegral());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool isConstVarExpression(const Token *tok)
|
||||||
|
{
|
||||||
|
if (!tok)
|
||||||
|
return false;
|
||||||
|
if (Token::Match(tok->previous(), "sizeof ("))
|
||||||
|
return true;
|
||||||
|
if (Token::Match(tok->previous(), "%name% (")) {
|
||||||
|
std::vector<const Token *> args = getArguments(tok);
|
||||||
|
return std::all_of(args.begin(), args.end(), &isConstVarExpression);
|
||||||
|
}
|
||||||
|
if (Token::Match(tok, "( %type%"))
|
||||||
|
return isConstVarExpression(tok->astOperand1());
|
||||||
|
if (Token::Match(tok, "%cop%")) {
|
||||||
|
if (tok->astOperand1() && !isConstVarExpression(tok->astOperand1()))
|
||||||
|
return false;
|
||||||
|
if (tok->astOperand2() && !isConstVarExpression(tok->astOperand2()))
|
||||||
|
return false;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL"))
|
||||||
|
return true;
|
||||||
|
if (tok->isEnumerator())
|
||||||
|
return true;
|
||||||
|
if (tok->variable())
|
||||||
|
return tok->variable()->isConst();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
static bool nonLocal(const Variable* var)
|
static bool nonLocal(const Variable* var)
|
||||||
{
|
{
|
||||||
|
|
|
@ -160,6 +160,8 @@ const Token *findLambdaEndToken(const Token *first);
|
||||||
*/
|
*/
|
||||||
bool isLikelyStreamRead(bool cpp, const Token *op);
|
bool isLikelyStreamRead(bool cpp, const Token *op);
|
||||||
|
|
||||||
|
bool isConstVarExpression(const Token *tok);
|
||||||
|
|
||||||
class FwdAnalysis {
|
class FwdAnalysis {
|
||||||
public:
|
public:
|
||||||
FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library), mWhat(What::Reassign) {}
|
FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library), mWhat(What::Reassign) {}
|
||||||
|
|
|
@ -1238,26 +1238,6 @@ void CheckCondition::clarifyConditionError(const Token *tok, bool assign, bool b
|
||||||
errmsg, CWE398, false);
|
errmsg, CWE398, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool isConstVarExpression(const Token * tok)
|
|
||||||
{
|
|
||||||
if (!tok)
|
|
||||||
return false;
|
|
||||||
if (Token::Match(tok, "%cop%")) {
|
|
||||||
if (tok->astOperand1() && !isConstVarExpression(tok->astOperand1()))
|
|
||||||
return false;
|
|
||||||
if (tok->astOperand2() && !isConstVarExpression(tok->astOperand2()))
|
|
||||||
return false;
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
if (Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL"))
|
|
||||||
return true;
|
|
||||||
if (tok->isEnumerator())
|
|
||||||
return true;
|
|
||||||
if (tok->variable())
|
|
||||||
return tok->variable()->isConst();
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
void CheckCondition::alwaysTrueFalse()
|
void CheckCondition::alwaysTrueFalse()
|
||||||
{
|
{
|
||||||
if (!mSettings->isEnabled(Settings::STYLE))
|
if (!mSettings->isEnabled(Settings::STYLE))
|
||||||
|
|
|
@ -2798,3 +2798,42 @@ void CheckOther::shadowError(const Token *var, const Token *shadowed, bool shado
|
||||||
std::string message = "$symbol:" + varname + "\nLocal variable $symbol shadows outer " + (shadowVar ? "variable" : "function");
|
std::string message = "$symbol:" + varname + "\nLocal variable $symbol shadows outer " + (shadowVar ? "variable" : "function");
|
||||||
reportError(errorPath, Severity::style, id, message, CWE398, false);
|
reportError(errorPath, Severity::style, id, message, CWE398, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void CheckOther::checkConstArgument()
|
||||||
|
{
|
||||||
|
if (!mSettings->isEnabled(Settings::STYLE))
|
||||||
|
return;
|
||||||
|
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
|
||||||
|
for (const Scope *functionScope : symbolDatabase->functionScopes) {
|
||||||
|
for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
|
||||||
|
if (!Token::simpleMatch(tok->astParent(), "("))
|
||||||
|
continue;
|
||||||
|
if (!Token::Match(tok->astParent()->previous(), "%name%"))
|
||||||
|
continue;
|
||||||
|
if (Token::Match(tok->astParent()->previous(), "if|while|switch|sizeof"))
|
||||||
|
continue;
|
||||||
|
if (tok == tok->astParent()->previous())
|
||||||
|
continue;
|
||||||
|
if (!tok->hasKnownIntValue())
|
||||||
|
continue;
|
||||||
|
if (Token::Match(tok, "%var%"))
|
||||||
|
continue;
|
||||||
|
if (Token::Match(tok, "++|--"))
|
||||||
|
continue;
|
||||||
|
if (isConstVarExpression(tok))
|
||||||
|
continue;
|
||||||
|
constArgumentError(tok, tok->astParent()->previous(), &tok->values().front());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckOther::constArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value)
|
||||||
|
{
|
||||||
|
MathLib::bigint intvalue = value ? value->intvalue : 0;
|
||||||
|
const std::string expr = tok ? tok->expressionString() : std::string("x");
|
||||||
|
const std::string fun = ftok ? ftok->str() : std::string("f");
|
||||||
|
|
||||||
|
const std::string errmsg = "Argument '" + expr + "' to function " + fun + " is always " + std::to_string(intvalue);
|
||||||
|
const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
|
||||||
|
reportError(errorPath, Severity::style, "constArgument", errmsg, CWE570, false);
|
||||||
|
}
|
||||||
|
|
|
@ -82,6 +82,7 @@ public:
|
||||||
checkOther.checkEvaluationOrder();
|
checkOther.checkEvaluationOrder();
|
||||||
checkOther.checkFuncArgNamesDifferent();
|
checkOther.checkFuncArgNamesDifferent();
|
||||||
checkOther.checkShadowVariables();
|
checkOther.checkShadowVariables();
|
||||||
|
checkOther.checkConstArgument();
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @brief Run checks against the simplified token list */
|
/** @brief Run checks against the simplified token list */
|
||||||
|
@ -215,8 +216,9 @@ public:
|
||||||
/** @brief %Check for shadow variables. Less noisy than gcc/clang -Wshadow. */
|
/** @brief %Check for shadow variables. Less noisy than gcc/clang -Wshadow. */
|
||||||
void checkShadowVariables();
|
void checkShadowVariables();
|
||||||
|
|
||||||
private:
|
void checkConstArgument();
|
||||||
|
|
||||||
|
private:
|
||||||
// Error messages..
|
// Error messages..
|
||||||
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, const bool result);
|
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, const bool result);
|
||||||
void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName);
|
void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName);
|
||||||
|
@ -269,6 +271,7 @@ private:
|
||||||
void funcArgNamesDifferent(const std::string & functionName, size_t index, const Token* declaration, const Token* definition);
|
void funcArgNamesDifferent(const std::string & functionName, size_t index, const Token* declaration, const Token* definition);
|
||||||
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
|
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
|
||||||
void shadowError(const Token *var, const Token *shadowed, bool shadowVar);
|
void shadowError(const Token *var, const Token *shadowed, bool shadowVar);
|
||||||
|
void constArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {
|
||||||
CheckOther c(nullptr, settings, errorLogger);
|
CheckOther c(nullptr, settings, errorLogger);
|
||||||
|
@ -333,6 +336,7 @@ private:
|
||||||
c.redundantBitwiseOperationInSwitchError(nullptr, "varname");
|
c.redundantBitwiseOperationInSwitchError(nullptr, "varname");
|
||||||
c.shadowError(nullptr, nullptr, false);
|
c.shadowError(nullptr, nullptr, false);
|
||||||
c.shadowError(nullptr, nullptr, true);
|
c.shadowError(nullptr, nullptr, true);
|
||||||
|
c.constArgumentError(nullptr, nullptr, nullptr);
|
||||||
|
|
||||||
const std::vector<const Token *> nullvec;
|
const std::vector<const Token *> nullvec;
|
||||||
c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);
|
c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);
|
||||||
|
|
|
@ -221,6 +221,7 @@ private:
|
||||||
TEST_CASE(cpp11FunctionArgInit); // #7846 - "void foo(int declaration = {}) {"
|
TEST_CASE(cpp11FunctionArgInit); // #7846 - "void foo(int declaration = {}) {"
|
||||||
|
|
||||||
TEST_CASE(shadowVariables);
|
TEST_CASE(shadowVariables);
|
||||||
|
TEST_CASE(constArgument);
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) {
|
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) {
|
||||||
|
@ -7523,6 +7524,59 @@ private:
|
||||||
"}\n");
|
"}\n");
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void constArgument() {
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" g((x & 0x01) >> 7);\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Argument '(x&1)>>7' to function g is always 0\n", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" g((int)((x & 0x01) >> 7));\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Argument '(int)((x&1)>>7)' to function g is always 0\n", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" g(0);\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void h() { return 1; }\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" g(h());\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" g(std::strlen(\"a\"));\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" g((int)0);\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f(int x) {\n"
|
||||||
|
" x = 0;\n"
|
||||||
|
" g(x);\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void g(int);\n"
|
||||||
|
"void f() {\n"
|
||||||
|
" const int x = 0;\n"
|
||||||
|
" g(x + 1);\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestOther)
|
REGISTER_TEST(TestOther)
|
||||||
|
|
Loading…
Reference in New Issue