Fix redundant FP assignment with unsigned zero (#2521)

* Refactor isNullOperand out of FwdAnalysis

* Improve isNullOperand

* Fix redundantAssignment FP with unsigned zero

* isNullValue check number

* Enhance isNullOperand to handle c++ casts

Also handle cast of NULL.
This commit is contained in:
Rikard Falkeborn 2020-02-09 11:16:08 +01:00 committed by GitHub
parent 60ada656a0
commit b1c6f2946a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 82 additions and 15 deletions

View File

@ -1547,6 +1547,20 @@ static bool isUnchanged(const Token *startToken, const Token *endToken, const st
return true; return true;
} }
bool isNullOperand(const Token *expr)
{
if (!expr)
return false;
if (Token::Match(expr, "static_cast|const_cast|dynamic_cast|reinterpret_cast <"))
expr = expr->astParent();
else if (!expr->isCast())
return Token::Match(expr, "NULL|nullptr");
if (expr->valueType() && expr->valueType()->pointer == 0)
return false;
const Token *castOp = expr->astOperand2() ? expr->astOperand2() : expr->astOperand1();
return Token::Match(castOp, "NULL|nullptr") || (MathLib::isInt(castOp->str()) && MathLib::isNullValue(castOp->str()));
}
struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<int> &exprVarIds, bool local, bool inInnerClass, int depth) struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<int> &exprVarIds, bool local, bool inInnerClass, int depth)
{ {
// Parse the given tokens // Parse the given tokens
@ -2037,12 +2051,3 @@ bool FwdAnalysis::isEscapedAlias(const Token* expr)
} }
return false; return false;
} }
bool FwdAnalysis::isNullOperand(const Token *expr)
{
if (!expr)
return false;
if (Token::Match(expr, "( %name% %name%| * )") && Token::Match(expr->astOperand1(), "0|NULL|nullptr"))
return true;
return Token::Match(expr, "NULL|nullptr");
}

View File

@ -211,6 +211,7 @@ const Variable *getLHSVariable(const Token *tok);
bool isScopeBracket(const Token* tok); bool isScopeBracket(const Token* tok);
bool isNullOperand(const Token *expr);
/** /**
* Forward data flow analysis for checks * Forward data flow analysis for checks
* - unused value * - unused value
@ -252,8 +253,6 @@ public:
bool possiblyAliased(const Token *expr, const Token *startToken) const; bool possiblyAliased(const Token *expr, const Token *startToken) const;
std::set<int> getExprVarIds(const Token* expr, bool* localOut = nullptr, bool* unknownVarIdOut = nullptr) const; std::set<int> getExprVarIds(const Token* expr, bool* localOut = nullptr, bool* unknownVarIdOut = nullptr) const;
static bool isNullOperand(const Token *expr);
private: private:
static bool isEscapedAlias(const Token* expr); static bool isEscapedAlias(const Token* expr);

View File

@ -264,7 +264,7 @@ void CheckFunctions::checkMathFunctions()
// fmod ( x , y) If y is zero, then either a range error will occur or the function will return zero (implementation-defined). // fmod ( x , y) If y is zero, then either a range error will occur or the function will return zero (implementation-defined).
else if (Token::Match(tok, "fmod|fmodf|fmodl (")) { else if (Token::Match(tok, "fmod|fmodf|fmodl (")) {
const Token* nextArg = tok->tokAt(2)->nextArgument(); const Token* nextArg = tok->tokAt(2)->nextArgument();
if (nextArg && nextArg->isNumber() && MathLib::isNullValue(nextArg->str())) if (nextArg && MathLib::isNullValue(nextArg->str()))
mathfunctionCallWarning(tok, 2); mathfunctionCallWarning(tok, 2);
} }
// pow ( x , y) If x is zero, and y is negative --> division by zero // pow ( x , y) If x is zero, and y is negative --> division by zero

View File

@ -448,8 +448,9 @@ void CheckOther::checkRedundantAssignment()
continue; continue;
} }
const Token* rhs = tok->astOperand2();
// Do not warn about assignment with 0 / NULL // Do not warn about assignment with 0 / NULL
if (Token::simpleMatch(tok->astOperand2(), "0") || FwdAnalysis::isNullOperand(tok->astOperand2())) if ((rhs && MathLib::isNullValue(rhs->str())) || isNullOperand(rhs))
continue; continue;
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference()) if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference())

View File

@ -1171,7 +1171,7 @@ void CheckUnusedVar::checkFunctionVariableUsage()
continue; continue;
} }
// Do not warn about assignment with NULL // Do not warn about assignment with NULL
if (FwdAnalysis::isNullOperand(tok->astOperand2())) if (isNullOperand(tok->astOperand2()))
continue; continue;
if (!tok->astOperand1()) if (!tok->astOperand1())

View File

@ -1342,6 +1342,8 @@ bool MathLib::isNullValue(const std::string &str)
if (str.empty() || (!std::isdigit(static_cast<unsigned char>(str[0])) && (str[0] != '.' && str[0] != '-' && str[0] != '+'))) if (str.empty() || (!std::isdigit(static_cast<unsigned char>(str[0])) && (str[0] != '.' && str[0] != '-' && str[0] != '+')))
return false; // Has to be a number return false; // Has to be a number
if (!isInt(str) && !isFloat(str))
return false;
bool isHex = isIntHex(str) || isFloatHex(str); bool isHex = isIntHex(str) || isFloatHex(str);
for (char i : str) { for (char i : str) {
if (std::isdigit(static_cast<unsigned char>(i)) && i != '0') // May not contain digits other than 0 if (std::isdigit(static_cast<unsigned char>(i)) && i != '0') // May not contain digits other than 0

View File

@ -4570,7 +4570,7 @@ const Function* Scope::findFunction(const Token *tok, bool requireConst) const
else if (funcarg->isPointer() && Token::Match(arguments[j], "nullptr|NULL ,|)")) else if (funcarg->isPointer() && Token::Match(arguments[j], "nullptr|NULL ,|)"))
same++; same++;
else if (arguments[j]->isNumber() && funcarg->isPointer() && MathLib::isNullValue(arguments[j]->str())) else if (funcarg->isPointer() && MathLib::isNullValue(arguments[j]->str()))
fallback1++; fallback1++;
// Try to evaluate the apparently more complex expression // Try to evaluate the apparently more complex expression

View File

@ -35,6 +35,7 @@ private:
void run() OVERRIDE { void run() OVERRIDE {
TEST_CASE(findLambdaEndToken); TEST_CASE(findLambdaEndToken);
TEST_CASE(findLambdaStartToken); TEST_CASE(findLambdaStartToken);
TEST_CASE(isNullOperand);
TEST_CASE(isReturnScope); TEST_CASE(isReturnScope);
TEST_CASE(isSameExpression); TEST_CASE(isSameExpression);
TEST_CASE(isVariableChanged); TEST_CASE(isVariableChanged);
@ -106,6 +107,27 @@ private:
ASSERT_EQUALS(true, findLambdaStartToken("[](void) constexpr -> const * const* int { return x; }")); ASSERT_EQUALS(true, findLambdaStartToken("[](void) constexpr -> const * const* int { return x; }"));
} }
bool isNullOperand(const char code[]) {
Settings settings;
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
return ::isNullOperand(tokenizer.tokens());
}
void isNullOperand() {
ASSERT_EQUALS(true, isNullOperand("(void*)0;"));
ASSERT_EQUALS(true, isNullOperand("(void*)0U;"));
ASSERT_EQUALS(true, isNullOperand("(void*)0x0LL;"));
ASSERT_EQUALS(true, isNullOperand("NULL;"));
ASSERT_EQUALS(true, isNullOperand("nullptr;"));
ASSERT_EQUALS(true, isNullOperand("(void*)NULL;"));
ASSERT_EQUALS(true, isNullOperand("static_cast<int*>(0);"));
ASSERT_EQUALS(false, isNullOperand("0;"));
ASSERT_EQUALS(false, isNullOperand("(void*)0.0;"));
ASSERT_EQUALS(false, isNullOperand("(void*)1;"));
}
bool isReturnScope(const char code[], int offset) { bool isReturnScope(const char code[], int offset) {
Settings settings; Settings settings;
Tokenizer tokenizer(&settings, this); Tokenizer tokenizer(&settings, this);

View File

@ -1123,6 +1123,7 @@ private:
ASSERT_EQUALS(false, MathLib::isNullValue("x")); ASSERT_EQUALS(false, MathLib::isNullValue("x"));
ASSERT_EQUALS(false, MathLib::isNullValue("garbage")); ASSERT_EQUALS(false, MathLib::isNullValue("garbage"));
ASSERT_EQUALS(false, MathLib::isNullValue("UL")); ASSERT_EQUALS(false, MathLib::isNullValue("UL"));
ASSERT_EQUALS(false, MathLib::isNullValue("-ENOMEM"));
} }
void incdec() const { void incdec() const {

View File

@ -166,6 +166,7 @@ private:
TEST_CASE(incompleteArrayFill); TEST_CASE(incompleteArrayFill);
TEST_CASE(redundantVarAssignment); TEST_CASE(redundantVarAssignment);
TEST_CASE(redundantVarAssignment_trivial);
TEST_CASE(redundantVarAssignment_struct); TEST_CASE(redundantVarAssignment_struct);
TEST_CASE(redundantVarAssignment_7133); TEST_CASE(redundantVarAssignment_7133);
TEST_CASE(redundantVarAssignment_stackoverflow); TEST_CASE(redundantVarAssignment_stackoverflow);
@ -6604,6 +6605,42 @@ private:
"test.cpp:4:note:*var is overwritten\n", errout.str()); "test.cpp:4:note:*var is overwritten\n", errout.str());
} }
void redundantVarAssignment_trivial() {
check("void f() {\n"
" int a = 0;\n"
" a = 4;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int a;\n"
" a = 0;\n"
" a = 4;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" unsigned a;\n"
" a = 0u;\n"
" a = 2u;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" void* a;\n"
" a = (void*)0;\n"
" a = p;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" void* a;\n"
" a = (void*)0U;\n"
" a = p;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void redundantVarAssignment_struct() { void redundantVarAssignment_struct() {
check("struct foo {\n" check("struct foo {\n"
" int a,b;\n" " int a,b;\n"