walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Merge pull request #2699 from pfultz2/condition-in-expr 

Fix issue 9578: false negative: (style) Condition '...' is always false
This commit is contained in:
Daniel Marjamäki 2020-07-22 09:07:12 +02:00 committed by GitHub
parent ca3fae9afa 0def5d7a9a
commit f39a94660d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 71 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
lib/astutils.cpp
View File

@ -20,6 +20,7 @@
//---------------------------------------------------------------------------
#include "astutils.h"
#include "config.h"
#include "library.h"
#include "mathlib.h"
#include "settings.h"
@ -32,13 +33,13 @@
#include <list>
#include <stack>
void visitAstNodes(const Token *ast, std::function<ChildrenToVisit(const Token *)> visitor)
template<class T, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*>)>
void visitAstNodesGeneric(T *ast, std::function<ChildrenToVisit(T *)> visitor)
{
std::stack<const Token *> tokens;
std::stack<T *> tokens;
tokens.push(ast);
while (!tokens.empty()) {
const Token *tok = tokens.top();
T *tok = tokens.top();
tokens.pop();
if (!tok)
continue;
@ -54,6 +55,16 @@ void visitAstNodes(const Token *ast, std::function<ChildrenToVisit(const Token *
}
}
void visitAstNodes(const Token *ast, std::function<ChildrenToVisit(const Token *)> visitor)
{
visitAstNodesGeneric(ast, std::move(visitor));
}
void visitAstNodes(Token *ast, std::function<ChildrenToVisit(Token *)> visitor)
{
visitAstNodesGeneric(ast, std::move(visitor));
}
static void astFlattenRecursive(const Token *tok, std::vector<const Token *> *result, const char* op, nonneg int depth = 0)
{
++depth;

1
lib/astutils.h
View File

@ -47,6 +47,7 @@ enum class ChildrenToVisit {
* Visit AST nodes recursively. The order is not "well defined"
*/
void visitAstNodes(const Token *ast, std::function<ChildrenToVisit(const Token *)> visitor);
void visitAstNodes(Token *ast, std::function<ChildrenToVisit(Token *)> visitor);
std::vector<const Token*> astFlatten(const Token* tok, const char* op);

5
lib/checkcondition.cpp
View File

@ -1400,6 +1400,8 @@ void CheckCondition::alwaysTrueFalse()
condition = parent->astOperand1();
else if (Token::Match(parent->previous(), "if|while ("))
condition = parent->astOperand2();
else if (Token::simpleMatch(parent, "return"))
condition = parent->astOperand1();
else if (parent->str() == ";" && parent->astParent() && parent->astParent()->astParent() && Token::simpleMatch(parent->astParent()->astParent()->previous(), "for ("))
condition = parent->astOperand1();
else
@ -1426,8 +1428,9 @@ void CheckCondition::alwaysTrueFalse()
const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
const bool ternaryExpression = Token::simpleMatch(tok->astParent(), "?");
const bool returnExpression = Token::simpleMatch(tok->astTop(), "return") && (tok->isComparisonOp() || Token::Match(tok, "&&|%oror%"));
if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression))
if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression || returnExpression))
continue;
// Don't warn when there are expanded macros..

43
lib/valueflow.cpp
View File

@ -4071,39 +4071,36 @@ struct ValueFlowConditionHandler {
if (Token::Match(tok->astParent(), "%oror%|&&")) {
Token *parent = tok->astParent();
const std::string &op(parent->str());
if (parent->astOperand1() == tok && ((op == "&&" && Token::Match(tok, "==|>=|<=|!")) ||
(op == "||" && Token::Match(tok, "%name%|!=")))) {
for (; parent && parent->str() == op; parent = parent->astParent()) {
std::stack<Token *> tokens;
tokens.push(parent->astOperand2());
if (astIsRHS(tok) && parent->astParent() && parent->str() == parent->astParent()->str())
parent = parent->astParent();
else if (!astIsLHS(tok)) {
parent = nullptr;
}
if (parent) {
const std::string &op(parent->str());
std::list<ValueFlow::Value> values = cond.true_values;
if (Token::Match(tok, "==|!="))
changePossibleToKnown(values);
if ((op == "&&" && Token::Match(tok, "==|>=|<=|!")) ||
(op == "||" && Token::Match(tok, "%name%|!="))) {
bool assign = false;
while (!tokens.empty()) {
Token *rhstok = tokens.top();
tokens.pop();
if (!rhstok)
continue;
tokens.push(rhstok->astOperand1());
tokens.push(rhstok->astOperand2());
if (isSameExpression(
tokenlist->isCPP(), false, cond.vartok, rhstok, settings->library, true, false))
setTokenValue(rhstok, cond.true_values.front(), settings);
else if (Token::Match(rhstok, "++|--|=") && isSameExpression(tokenlist->isCPP(),
visitAstNodes(parent->astOperand2(), [&](Token* tok2) {
if (isSameExpression(tokenlist->isCPP(), false, cond.vartok, tok2, settings->library, true, false))
setTokenValue(tok2, values.front(), settings);
else if (Token::Match(tok2, "++|--|=") && isSameExpression(tokenlist->isCPP(),
false,
cond.vartok,
rhstok->astOperand1(),
tok2->astOperand1(),
settings->library,
true,
false)) {
assign = true;
break;
return ChildrenToVisit::done;
}
}
return ChildrenToVisit::op1_and_op2;
});
if (assign)
break;
while (parent->astParent() && parent == parent->astParent()->astOperand2())
parent = parent->astParent();
}
}
}

45
test/testcondition.cpp
View File

@ -840,14 +840,16 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x!=3' is always true\n", errout.str());
check("void f(int x) {\n"
" if (1 != x || 3 != x)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition '3!=x' is always true\n", errout.str());
check("void f(int x) {\n"
" if (x<0 && !x) {}\n"
@ -857,12 +859,14 @@ private:
check("void f(int x) {\n"
" if (x==0 && x) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 0 && x.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 0 && x.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x' is always false\n", errout.str());
check("void f(int x) {\n" // ast..
" if (y == 1 && x == 1 && x == 7) { }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 7.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 7.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==7' is always false\n", errout.str());
check("void f(int x, int y) {\n"
" if (x != 1 || y != 1)\n"
@ -883,7 +887,7 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x!=3' is always true\n", errout.str());
check("void f(int x) {\n"
" if ((x != 1) && (x != 3))\n"
@ -926,7 +930,8 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 5 || x != 6.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 5 || x != 6.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x!=6' is always true\n", errout.str());
check("void f(unsigned int a, unsigned int b, unsigned int c) {\n"
" if((a != b) || (c != b) || (c != a))\n"
@ -950,13 +955,14 @@ private:
" if ((x == 1) && (x == 0x00000001))\n"
" a++;\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==0x00000001' is always true\n", errout.str());
check("void f(int x) {\n"
" if (x == 1 && x == 3)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 3.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 3.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==3' is always false\n", errout.str());
check("void f(int x) {\n"
" if (x == 1.0 && x == 3.0)\n"
@ -1079,7 +1085,8 @@ private:
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If 'x == 3', the comparison 'x != 4' is always true.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If 'x == 3', the comparison 'x != 4' is always true.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x!=4' is always true\n", errout.str());
check("void f(int x) {\n"
" if ((x!=4) && (x==3))\n"
@ -1097,13 +1104,15 @@ private:
" if ((x!=4) || (x==3))\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If 'x == 3', the comparison 'x != 4' is always true.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If 'x == 3', the comparison 'x != 4' is always true.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==3' is always false\n", errout.str());
check("void f(int x) {\n"
" if ((x==3) && (x!=3))\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 3 && x != 3.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 3 && x != 3.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x!=3' is always false\n", errout.str());
check("void f(int x) {\n"
" if ((x==6) || (x!=6))\n"
@ -1195,7 +1204,8 @@ private:
check("void f(char x) {\n"
" if (x == '1' && x == '2') {}\n"
"}", "test.cpp", true);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == '1' && x == '2'.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == '1' && x == '2'.\n"
"[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x=='2'' is always false\n", errout.str());
check("int f(char c) {\n"
" return (c >= 'a' && c <= 'z');\n"
@ -1269,7 +1279,8 @@ private:
" if ((t == A) && (t == B))\n"
" {}\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Logical conjunction always evaluates to false: t == 0 && t == 1.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (warning) Logical conjunction always evaluates to false: t == 0 && t == 1.\n"
"[test.cpp:3] -> [test.cpp:3]: (style) Condition 't==B' is always false\n", errout.str());
}
void incorrectLogicOperator11() {
@ -2881,7 +2892,7 @@ private:
" if(x == 0) { x++; return x == 0; } \n"
" return false;\n"
"}");
TODO_ASSERT_EQUALS("return value is always true?", "", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==0' is always false\n", errout.str());
check("void f() {\n" // #6898 (Token::expressionString)
" int x = 0;\n"
@ -3826,6 +3837,12 @@ private:
check("bool f();\n"
"void foo() { bool x = true; if(x&&f()) {}}\n");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always true\n", errout.str());
// #9578
check("bool f(const std::string &s) {\n"
" return s.size()>2U && s[0]=='4' && s[0]=='2';\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 's[0]=='2'' is always false\n", errout.str());
}
void pointerAdditionResultNotNull() {