Fix some false positives when the same expression at different places does not have the same value
Typically with ``` int F(int *f); void F2(int *a, int *b) { int c = *a; F(a); // modifies *a if (b && c != *a) {} } ``` we would get the following FP: ``` [test.cpp:3] -> [test.cpp:5]: (style) The comparison 'c != *a' is always false because 'c' and '*a' represent the same value.\n ``` I guess it boils down to isSameExpression only checking that the expression is the same (in the above case, "*a" and "*a" are indeed the same), but there's not real check on the values. So the patch here is a bit hackish, and we still have false negatives in cases with dereferenced pointers.
This commit is contained in:
parent
27841d6b81
commit
61ccf888b3
|
@ -2014,7 +2014,9 @@ void CheckOther::checkDuplicateExpression()
|
||||||
if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) {
|
if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) {
|
||||||
if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true))
|
if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true))
|
||||||
continue;
|
continue;
|
||||||
const bool followVar = !isConstVarExpression(tok) || Token::Match(tok, "%comp%|%oror%|&&");
|
const bool pointerDereference = (tok->astOperand1() && tok->astOperand1()->isUnaryOp("*")) ||
|
||||||
|
(tok->astOperand2() && tok->astOperand2()->isUnaryOp("*"));
|
||||||
|
const bool followVar = (!isConstVarExpression(tok) || Token::Match(tok, "%comp%|%oror%|&&")) && !pointerDereference;
|
||||||
if (isSameExpression(mTokenizer->isCPP(),
|
if (isSameExpression(mTokenizer->isCPP(),
|
||||||
true,
|
true,
|
||||||
tok->astOperand1(),
|
tok->astOperand1(),
|
||||||
|
|
|
@ -243,6 +243,8 @@ private:
|
||||||
TEST_CASE(unusedVariableValueTemplate); // #8994
|
TEST_CASE(unusedVariableValueTemplate); // #8994
|
||||||
|
|
||||||
TEST_CASE(moduloOfOne);
|
TEST_CASE(moduloOfOne);
|
||||||
|
|
||||||
|
TEST_CASE(sameExpressionPointers);
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, bool verbose=false, Settings* settings = nullptr) {
|
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, bool verbose=false, Settings* settings = nullptr) {
|
||||||
|
@ -5484,6 +5486,24 @@ private:
|
||||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The comparison 'val < 0' is always false.\n"
|
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The comparison 'val < 0' is always false.\n"
|
||||||
"[test.cpp:2] -> [test.cpp:4]: (style) The comparison 'val > 0' is always false.\n", errout.str());
|
"[test.cpp:2] -> [test.cpp:4]: (style) The comparison 'val > 0' is always false.\n", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" int val = 0;\n"
|
||||||
|
" int *p = &val;n"
|
||||||
|
" val = 1;\n"
|
||||||
|
" if (*p < 0) continue;\n"
|
||||||
|
" if ((*p > 0)) {}\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" int val = 0;\n"
|
||||||
|
" int *p = &val;n"
|
||||||
|
" if (*p < 0) continue;\n"
|
||||||
|
" if ((*p > 0)) {}\n"
|
||||||
|
"}\n");
|
||||||
|
TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The comparison '*p < 0' is always false.\n"
|
||||||
|
"[test.cpp:2] -> [test.cpp:4]: (style) The comparison '*p > 0' is always false.\n", "", errout.str());
|
||||||
|
|
||||||
check("void f() {\n"
|
check("void f() {\n"
|
||||||
" int val = 0;\n"
|
" int val = 0;\n"
|
||||||
" if (val < 0) {\n"
|
" if (val < 0) {\n"
|
||||||
|
@ -8701,6 +8721,15 @@ private:
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void sameExpressionPointers() {
|
||||||
|
check("int f(int *i);\n"
|
||||||
|
"void g(int *a, int *b) {\n"
|
||||||
|
" int c = *a;\n"
|
||||||
|
" f(a);\n"
|
||||||
|
" if (b && c != *a) {}\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestOther)
|
REGISTER_TEST(TestOther)
|
||||||
|
|
Loading…
Reference in New Issue