Fix #11579 FN knownConditionTrueFalse with non-bool as bool parameter / #9450 string literal to bool conversion in function call (#5338)
This commit is contained in:
parent
5dbcea3f1d
commit
827e87afe7
|
@ -1464,6 +1464,19 @@ static bool astIsBoolLike(const Token* tok)
|
||||||
return astIsBool(tok) || isUsedAsBool(tok);
|
return astIsBool(tok) || isUsedAsBool(tok);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool isBooleanFuncArg(const Token* tok) {
|
||||||
|
if (tok->variable() && tok->variable()->valueType() && tok->variable()->valueType()->type == ValueType::BOOL) // skip trivial case: bool passed as bool
|
||||||
|
return false;
|
||||||
|
int argn{};
|
||||||
|
const Token* ftok = getTokenArgumentFunction(tok, argn);
|
||||||
|
if (!ftok)
|
||||||
|
return false;
|
||||||
|
std::vector<const Variable*> argvars = getArgumentVars(ftok, argn);
|
||||||
|
if (argvars.size() != 1)
|
||||||
|
return false;
|
||||||
|
return !argvars[0]->isReference() && argvars[0]->valueType() && argvars[0]->valueType()->type == ValueType::BOOL;
|
||||||
|
}
|
||||||
|
|
||||||
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors)
|
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors)
|
||||||
{
|
{
|
||||||
if (tok1 == nullptr && tok2 == nullptr)
|
if (tok1 == nullptr && tok2 == nullptr)
|
||||||
|
|
|
@ -252,10 +252,15 @@ bool isStructuredBindingVariable(const Variable* var);
|
||||||
const Token* isInLoopCondition(const Token* tok);
|
const Token* isInLoopCondition(const Token* tok);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Is token used a boolean, that is to say cast to a bool, or used as a condition in a if/while/for
|
* Is token used as boolean, that is to say cast to a bool, or used as a condition in a if/while/for
|
||||||
*/
|
*/
|
||||||
CPPCHECKLIB bool isUsedAsBool(const Token * const tok);
|
CPPCHECKLIB bool isUsedAsBool(const Token * const tok);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Is token passed to a function taking a bool argument
|
||||||
|
*/
|
||||||
|
CPPCHECKLIB bool isBooleanFuncArg(const Token* tok);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Are two conditions opposite
|
* Are two conditions opposite
|
||||||
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false
|
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false
|
||||||
|
|
|
@ -1466,7 +1466,7 @@ void CheckCondition::alwaysTrueFalse()
|
||||||
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
|
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
|
||||||
if (Token::simpleMatch(tok, "<") && tok->link()) // don't write false positives when templates are used
|
if (Token::simpleMatch(tok, "<") && tok->link()) // don't write false positives when templates are used
|
||||||
continue;
|
continue;
|
||||||
if (!tok->hasKnownIntValue())
|
if (!tok->hasKnownBoolValue())
|
||||||
continue;
|
continue;
|
||||||
if (Token::Match(tok->previous(), "%name% (") && tok->previous()->function()) {
|
if (Token::Match(tok->previous(), "%name% (") && tok->previous()->function()) {
|
||||||
const Function* f = tok->previous()->function();
|
const Function* f = tok->previous()->function();
|
||||||
|
@ -1490,6 +1490,8 @@ void CheckCondition::alwaysTrueFalse()
|
||||||
else if (parent->str() == ";" && parent->astParent() && parent->astParent()->astParent() &&
|
else if (parent->str() == ";" && parent->astParent() && parent->astParent()->astParent() &&
|
||||||
Token::simpleMatch(parent->astParent()->astParent()->previous(), "for ("))
|
Token::simpleMatch(parent->astParent()->astParent()->previous(), "for ("))
|
||||||
condition = parent->astParent()->astParent()->previous();
|
condition = parent->astParent()->astParent()->previous();
|
||||||
|
else if (isBooleanFuncArg(tok))
|
||||||
|
condition = tok;
|
||||||
else
|
else
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
@ -1580,14 +1582,17 @@ void CheckCondition::alwaysTrueFalse()
|
||||||
if (hasSizeof)
|
if (hasSizeof)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
alwaysTrueFalseError(tok, condition, &tok->values().front());
|
auto it = std::find_if(tok->values().begin(), tok->values().end(), [](const ValueFlow::Value& v) {
|
||||||
|
return v.isIntValue();
|
||||||
|
});
|
||||||
|
alwaysTrueFalseError(tok, condition, &*it);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckCondition::alwaysTrueFalseError(const Token* tok, const Token* condition, const ValueFlow::Value* value)
|
void CheckCondition::alwaysTrueFalseError(const Token* tok, const Token* condition, const ValueFlow::Value* value)
|
||||||
{
|
{
|
||||||
const bool alwaysTrue = value && (value->intvalue != 0);
|
const bool alwaysTrue = value && (value->intvalue != 0 || value->isImpossible());
|
||||||
const std::string expr = tok ? tok->expressionString() : std::string("x");
|
const std::string expr = tok ? tok->expressionString() : std::string("x");
|
||||||
const std::string conditionStr = (Token::simpleMatch(condition, "return") ? "Return value" : "Condition");
|
const std::string conditionStr = (Token::simpleMatch(condition, "return") ? "Return value" : "Condition");
|
||||||
const std::string errmsg = conditionStr + " '" + expr + "' is always " + (alwaysTrue ? "true" : "false");
|
const std::string errmsg = conditionStr + " '" + expr + "' is always " + (alwaysTrue ? "true" : "false");
|
||||||
|
|
|
@ -292,8 +292,10 @@ void CheckString::checkIncorrectStringCompare()
|
||||||
incorrectStringBooleanError(tok->next(), tok->strAt(1));
|
incorrectStringBooleanError(tok->next(), tok->strAt(1));
|
||||||
} else if (Token::Match(tok, "if|while ( %str%|%char% )") && !tok->tokAt(2)->getValue(0)) {
|
} else if (Token::Match(tok, "if|while ( %str%|%char% )") && !tok->tokAt(2)->getValue(0)) {
|
||||||
incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2));
|
incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2));
|
||||||
} else if (tok->str() == "?" && Token::Match(tok->astOperand1(), "%str%|%char%"))
|
} else if (tok->str() == "?" && Token::Match(tok->astOperand1(), "%str%|%char%")) {
|
||||||
incorrectStringBooleanError(tok->astOperand1(), tok->astOperand1()->str());
|
incorrectStringBooleanError(tok->astOperand1(), tok->astOperand1()->str());
|
||||||
|
} else if (Token::Match(tok, "%str%") && isBooleanFuncArg(tok))
|
||||||
|
incorrectStringBooleanError(tok, tok->str());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -488,7 +488,7 @@ struct ForwardTraversal {
|
||||||
if (allAnalysis.isIncremental())
|
if (allAnalysis.isIncremental())
|
||||||
return Break(Analyzer::Terminate::Bail);
|
return Break(Analyzer::Terminate::Bail);
|
||||||
} else if (allAnalysis.isModified()) {
|
} else if (allAnalysis.isModified()) {
|
||||||
std::vector<ForwardTraversal> ftv = tryForkScope(endBlock, allAnalysis.isModified());
|
std::vector<ForwardTraversal> ftv = tryForkScope(endBlock, /*isModified*/ true);
|
||||||
bool forkContinue = true;
|
bool forkContinue = true;
|
||||||
for (ForwardTraversal& ft : ftv) {
|
for (ForwardTraversal& ft : ftv) {
|
||||||
if (condTok)
|
if (condTok)
|
||||||
|
|
|
@ -2371,6 +2371,15 @@ bool Token::hasKnownIntValue() const
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool Token::hasKnownBoolValue() const
|
||||||
|
{
|
||||||
|
if (!mImpl->mValues)
|
||||||
|
return false;
|
||||||
|
return std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) {
|
||||||
|
return value.isIntValue() && (value.isKnown() || (value.intvalue == 0 && value.isImpossible()));
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
bool Token::hasKnownValue() const
|
bool Token::hasKnownValue() const
|
||||||
{
|
{
|
||||||
return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown));
|
return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown));
|
||||||
|
|
|
@ -1212,6 +1212,7 @@ public:
|
||||||
}
|
}
|
||||||
|
|
||||||
bool hasKnownIntValue() const;
|
bool hasKnownIntValue() const;
|
||||||
|
bool hasKnownBoolValue() const;
|
||||||
bool hasKnownValue() const;
|
bool hasKnownValue() const;
|
||||||
bool hasKnownValue(ValueFlow::Value::ValueType t) const;
|
bool hasKnownValue(ValueFlow::Value::ValueType t) const;
|
||||||
bool hasKnownSymbolicValue(const Token* tok) const;
|
bool hasKnownSymbolicValue(const Token* tok) const;
|
||||||
|
|
|
@ -912,7 +912,7 @@ static void compilePrecedence2(Token *&tok, AST_state& state)
|
||||||
}
|
}
|
||||||
compileBinOp(tok, state, compileScope);
|
compileBinOp(tok, state, compileScope);
|
||||||
} else if (tok->str() == "[") {
|
} else if (tok->str() == "[") {
|
||||||
if (state.cpp && isPrefixUnary(tok, state.cpp) && Token::Match(tok->link(), "] (|{|<")) { // Lambda
|
if (state.cpp && isPrefixUnary(tok, /*cpp*/ true) && Token::Match(tok->link(), "] (|{|<")) { // Lambda
|
||||||
// What we do here:
|
// What we do here:
|
||||||
// - Nest the round bracket under the square bracket.
|
// - Nest the round bracket under the square bracket.
|
||||||
// - Nest what follows the lambda (if anything) with the lambda opening [
|
// - Nest what follows the lambda (if anything) with the lambda opening [
|
||||||
|
|
|
@ -4514,6 +4514,21 @@ private:
|
||||||
" return a;\n"
|
" return a;\n"
|
||||||
"}\n");
|
"}\n");
|
||||||
ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'a==\"x\"' is always true\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'a==\"x\"' is always true\n", errout.str());
|
||||||
|
|
||||||
|
check("void g(bool);\n"
|
||||||
|
"void f() {\n"
|
||||||
|
" int i = 5;\n"
|
||||||
|
" int* p = &i;\n"
|
||||||
|
" g(i == 7);\n"
|
||||||
|
" g(p == nullptr);\n"
|
||||||
|
" g(p);\n"
|
||||||
|
" g(&i);\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'i==7' is always false\n"
|
||||||
|
"[test.cpp:6]: (style) Condition 'p==nullptr' is always false\n"
|
||||||
|
"[test.cpp:7]: (style) Condition 'p' is always true\n"
|
||||||
|
"[test.cpp:8]: (style) Condition '&i' is always true\n",
|
||||||
|
errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void alwaysTrueSymbolic()
|
void alwaysTrueSymbolic()
|
||||||
|
|
|
@ -743,6 +743,13 @@ private:
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of char literal '\\0' to bool always evaluates to false.\n"
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of char literal '\\0' to bool always evaluates to false.\n"
|
||||||
"[test.cpp:3]: (warning) Conversion of char literal L'\\0' to bool always evaluates to false.\n", errout.str());
|
"[test.cpp:3]: (warning) Conversion of char literal L'\\0' to bool always evaluates to false.\n", errout.str());
|
||||||
|
|
||||||
|
check("void f(bool b);\n" // #9450
|
||||||
|
"void f(std::string s);\n"
|
||||||
|
"void g() {\n"
|
||||||
|
" f(\"abc\");\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:4]: (warning) Conversion of string literal \"abc\" to bool always evaluates to true.\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void deadStrcmp() {
|
void deadStrcmp() {
|
||||||
|
|
Loading…
Reference in New Issue