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

knownConditionTrueFalse; avoid several warnings when nonzero expression is compared to see if it is positive or negative

This commit is contained in:
Daniel Marjamäki 2021-06-25 16:25:14 +02:00
parent 452c92494e
commit 9769afe434
4 changed files with 87 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lib/checkcondition.cpp
View File

@ -29,6 +29,8 @@
#include "tokenize.h"
#include "valueflow.h"
#include "checkother.h" // comparisonNonZeroExpressionLessThanZero and testIfNonZeroExpressionIsPositive
#include <algorithm>
#include <limits>
#include <list>
@ -1427,6 +1429,15 @@ void CheckCondition::alwaysTrueFalse()
if (isConstVarExpression(tok, "[|(|&|+|-|*|/|%|^|>>|<<"))
continue;
// there are specific warnings about nonzero expressions (pointer/unsigned)
// do not write alwaysTrueFalse for these comparisons.
{
const ValueFlow::Value *zeroValue = nullptr;
const Token *nonZeroExpr = nullptr;
if (CheckOther::comparisonNonZeroExpressionLessThanZero(tok, &zeroValue, &nonZeroExpr) || CheckOther::testIfNonZeroExpressionIsPositive(tok, &zeroValue, &nonZeroExpr))
continue;
}
const bool constIfWhileExpression =
tok->astParent() && Token::Match(tok->astTop()->astOperand1(), "if|while") && !tok->astTop()->astOperand1()->isConstexpr() &&
(Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->astParent()->astOperand1(), "if|while"));

88
lib/checkother.cpp
View File

@ -2340,41 +2340,69 @@ void CheckOther::checkSignOfUnsignedVariable()
for (const Scope * scope : symbolDatabase->functionScopes) {
// check all the code in the function
for (const Token *tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
continue;
const ValueFlow::Value *v1 = tok->astOperand1()->getValue(0);
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);
if (Token::Match(tok, "<|<=") && v2 && v2->isKnown()) {
const ValueType* vt = tok->astOperand1()->valueType();
if (vt && vt->pointer)
pointerLessThanZeroError(tok, v2);
if (vt && vt->sign == ValueType::UNSIGNED)
unsignedLessThanZeroError(tok, v2, tok->astOperand1()->expressionString());
} else if (Token::Match(tok, ">|>=") && v1 && v1->isKnown()) {
const ValueType* vt = tok->astOperand2()->valueType();
if (vt && vt->pointer)
pointerLessThanZeroError(tok, v1);
if (vt && vt->sign == ValueType::UNSIGNED)
unsignedLessThanZeroError(tok, v1, tok->astOperand2()->expressionString());
} else if (Token::simpleMatch(tok, ">=") && v2 && v2->isKnown()) {
const ValueType* vt = tok->astOperand1()->valueType();
if (vt && vt->pointer)
pointerPositiveError(tok, v2);
if (vt && vt->sign == ValueType::UNSIGNED)
unsignedPositiveError(tok, v2, tok->astOperand1()->expressionString());
} else if (Token::simpleMatch(tok, "<=") && v1 && v1->isKnown()) {
const ValueType* vt = tok->astOperand2()->valueType();
if (vt && vt->pointer)
pointerPositiveError(tok, v1);
if (vt && vt->sign == ValueType::UNSIGNED)
unsignedPositiveError(tok, v1, tok->astOperand2()->expressionString());
const ValueFlow::Value *zeroValue = nullptr;
const Token *nonZeroExpr = nullptr;
if (comparisonNonZeroExpressionLessThanZero(tok, &zeroValue, &nonZeroExpr)) {
const ValueType* vt = nonZeroExpr->valueType();
if (vt->pointer)
pointerLessThanZeroError(tok, zeroValue);
else
unsignedLessThanZeroError(tok, zeroValue, nonZeroExpr->expressionString());
} else if (testIfNonZeroExpressionIsPositive(tok, &zeroValue, &nonZeroExpr)) {
const ValueType* vt = nonZeroExpr->valueType();
if (vt->pointer)
pointerPositiveError(tok, zeroValue);
else
unsignedPositiveError(tok, zeroValue, nonZeroExpr->expressionString());
}
}
}
}
bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr)
{
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
return false;
const ValueFlow::Value *v1 = tok->astOperand1()->getValue(0);
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);
if (Token::Match(tok, "<|<=") && v2 && v2->isKnown()) {
*zeroValue = v2;
*nonZeroExpr = tok->astOperand1();
} else if (Token::Match(tok, ">|>=") && v1 && v1->isKnown()) {
*zeroValue = v1;
*nonZeroExpr = tok->astOperand2();
} else {
return false;
}
const ValueType* vt = (*nonZeroExpr)->valueType();
return vt && (vt->pointer || vt->sign == ValueType::UNSIGNED);
}
bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr)
{
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
return false;
const ValueFlow::Value *v1 = tok->astOperand1()->getValue(0);
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);
if (Token::simpleMatch(tok, ">=") && v2 && v2->isKnown()) {
*zeroValue = v2;
*nonZeroExpr = tok->astOperand1();
} else if (Token::simpleMatch(tok, "<=") && v1 && v1->isKnown()) {
*zeroValue = v1;
*nonZeroExpr = tok->astOperand2();
} else {
return false;
}
const ValueType* vt = (*nonZeroExpr)->valueType();
return vt && (vt->pointer || vt->sign == ValueType::UNSIGNED);
}
void CheckOther::unsignedLessThanZeroError(const Token *tok, const ValueFlow::Value * v, const std::string &varname)
{
reportError(getErrorPath(tok, v, "Unsigned less than zero"), Severity::style, "unsignedLessThanZero",

7
lib/checkother.h
View File

@ -104,6 +104,13 @@ public:
checkOther.checkModuloOfOne();
}
/** Is expression a comparison that checks if a nonzero (unsigned/pointer) expression is less than zero? */
static bool comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr);
/** Is expression a comparison that checks if a nonzero (unsigned/pointer) expression is positive? */
static bool testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr);
/** @brief Clarify calculation for ".. a * b ? .." */
void clarifyCalculation();

15
test/testcondition.cpp
View File

@ -2375,8 +2375,8 @@ private:
check("void f1(const std::string &s) { if(s.size() > 0) if(s.empty()) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void f1(const std::string &s) { if(s.size() < 0) if(s.empty()) {}} ");
ASSERT_EQUALS("[test.cpp:1]: (style) Condition 's.size()<0' is always false\n", errout.str());
check("void f1(const std::string &s) { if(s.size() < 0) if(s.empty()) {}} "); // <- CheckOther reports: checking if unsigned expression is less than zero
ASSERT_EQUALS("", errout.str());
check("void f1(const std::string &s) { if(s.empty()) if(s.size() > 42) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
@ -2411,8 +2411,8 @@ private:
check("void f1(const std::string &s) { if(s.size() < 2) if(s.empty()) {}}");
ASSERT_EQUALS("", errout.str());
check("void f1(const std::string &s) { if(s.size() >= 0) if(s.empty()) {}} ");
ASSERT_EQUALS("[test.cpp:1]: (style) Condition 's.size()>=0' is always true\n", errout.str());
check("void f1(const std::string &s) { if(s.size() >= 0) if(s.empty()) {}} "); // CheckOther says: Unsigned expression 's.size()' can't be negative so it is unnecessary to test it. [unsignedPositive]
ASSERT_EQUALS("", errout.str());
// TODO: These are identical condition since size cannot be negative
check("void f1(const std::string &s) { if(s.size() <= 0) if(s.empty()) {}}");
@ -3632,6 +3632,13 @@ private:
" if (x == -1) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
// do not report both unsignedLessThanZero and knownConditionTrueFalse
check("void foo(unsigned int max) {\n"
" unsigned int num = max - 1;\n"
" if (num < 0) {}\n" // <- do not report knownConditionTrueFalse
"}");
ASSERT_EQUALS("", errout.str());
}
void alwaysTrueInfer() {