parent
9acda09fa2
commit
8f27cec991
|
@ -352,7 +352,7 @@ class ValueFlow:
|
|||
# http://cppcheck.sourceforge.net/devinfo/doxyoutput/classValueFlow_1_1Value.html
|
||||
|
||||
class Value:
|
||||
## integer value
|
||||
# #integer value
|
||||
intvalue = None
|
||||
## token value
|
||||
tokvalue = None
|
||||
|
|
|
@ -239,51 +239,6 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
|
|||
(comp1 == ">" && comp2 == "<"))));
|
||||
}
|
||||
|
||||
|
||||
bool isIncorrectCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
||||
{
|
||||
if (!cond1 || !cond2)
|
||||
return false;
|
||||
|
||||
if (cond1->str() == "!") {
|
||||
if (cond2->str() == "!=") {
|
||||
if (cond2->astOperand1() && cond2->astOperand1()->str() == "0")
|
||||
return isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand2(), constFunctions);
|
||||
if (cond2->astOperand2() && cond2->astOperand2()->str() == "0")
|
||||
return isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand1(), constFunctions);
|
||||
}
|
||||
return isSameExpression(cpp, cond1->astOperand1(), cond2, constFunctions);
|
||||
}
|
||||
|
||||
if (cond2->str() == "!")
|
||||
return isIncorrectCond(isNot, cpp, cond2, cond1, constFunctions);
|
||||
|
||||
if (!cond1->isComparisonOp() || !cond2->isComparisonOp())
|
||||
return false;
|
||||
|
||||
const std::string &comp1 = cond1->str();
|
||||
|
||||
// condition found .. get comparator
|
||||
std::string comp2;
|
||||
if (isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
|
||||
isSameExpression(cpp, cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
|
||||
comp2 = cond2->str();
|
||||
} else if (isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
|
||||
isSameExpression(cpp, cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
|
||||
comp2 = cond2->str();
|
||||
if (comp2[0] == '>')
|
||||
comp2[0] = '<';
|
||||
else if (comp2[0] == '<')
|
||||
comp2[0] = '>';
|
||||
}
|
||||
|
||||
// is condition incorrect?
|
||||
return ((comp1 == "<" && comp2 == "==") ||
|
||||
(comp1 == ">" && comp2 == "==") ||
|
||||
(comp1 == "==" && comp2 == "<") ||
|
||||
(comp1 == "==" && comp2 == ">"));
|
||||
}
|
||||
|
||||
bool isConstExpression(const Token *tok, const std::set<std::string> &constFunctions)
|
||||
{
|
||||
if (!tok)
|
||||
|
|
|
@ -59,7 +59,6 @@ bool isSameExpression(bool cpp, const Token *tok1, const Token *tok2, const std:
|
|||
* @param constFunctions constFunctions
|
||||
*/
|
||||
bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions);
|
||||
bool isIncorrectCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions);
|
||||
|
||||
bool isConstExpression(const Token *tok, const std::set<std::string> &constFunctions);
|
||||
|
||||
|
|
|
@ -641,187 +641,6 @@ static std::string conditionString(bool not1, const Token *expr1, const std::str
|
|||
(expr1->isName() ? expr1->str() : std::string("EXPR"));
|
||||
}
|
||||
|
||||
void CheckCondition::checkIncorrectCondition()
|
||||
{
|
||||
const bool printStyle = _settings->isEnabled("style");
|
||||
const bool printWarning = _settings->isEnabled("warning");
|
||||
if (!printWarning && !printStyle)
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t ii = 0; ii < functions; ++ii) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[ii];
|
||||
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (!Token::Match(tok, "%oror%|&&") || !tok->astOperand1() || !tok->astOperand2())
|
||||
continue;
|
||||
|
||||
// Opposite comparisons around || or && => always true or always false
|
||||
if ((tok->astOperand1()->isName() || tok->astOperand2()->isName()) &&
|
||||
isIncorrectCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
|
||||
|
||||
const bool alwaysTrue(tok->str() == "||");
|
||||
incorrectConditionError(tok, tok->expressionString(), alwaysTrue);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
// 'A && (!A || B)' is equivalent with 'A && B'
|
||||
// 'A || (!A && B)' is equivalent with 'A || B'
|
||||
if (printStyle &&
|
||||
((tok->str() == "||" && tok->astOperand2()->str() == "&&") ||
|
||||
(tok->str() == "&&" && tok->astOperand2()->str() == "||"))) {
|
||||
const Token* tok2 = tok->astOperand2()->astOperand1();
|
||||
if (isIncorrectCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok2, _settings->library.functionpure)) {
|
||||
std::string expr1(tok->astOperand1()->expressionString());
|
||||
std::string expr2(tok->astOperand2()->astOperand1()->expressionString());
|
||||
std::string expr3(tok->astOperand2()->astOperand2()->expressionString());
|
||||
|
||||
if (expr1.length() + expr2.length() + expr3.length() > 50U) {
|
||||
if (expr1[0] == '!' && expr2[0] != '!') {
|
||||
expr1 = "!A";
|
||||
expr2 = "A";
|
||||
} else {
|
||||
expr1 = "A";
|
||||
expr2 = "!A";
|
||||
}
|
||||
|
||||
expr3 = "B";
|
||||
}
|
||||
|
||||
const std::string cond1 = expr1 + " " + tok->str() + " (" + expr2 + " " + tok->astOperand2()->str() + " " + expr3 + ")";
|
||||
const std::string cond2 = expr1 + " " + tok->str() + " " + expr3;
|
||||
|
||||
redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'");
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// Comparison #1 (LHS)
|
||||
const Token *comp1 = tok->astOperand1();
|
||||
if (comp1 && comp1->str() == tok->str())
|
||||
comp1 = comp1->astOperand2();
|
||||
|
||||
// Comparison #2 (RHS)
|
||||
const Token *comp2 = tok->astOperand2();
|
||||
|
||||
// Parse LHS
|
||||
bool not1;
|
||||
std::string op1, value1;
|
||||
const Token *expr1;
|
||||
if (!parseComparison(comp1, ¬1, &op1, &value1, &expr1))
|
||||
continue;
|
||||
|
||||
// Parse RHS
|
||||
bool not2;
|
||||
std::string op2, value2;
|
||||
const Token *expr2;
|
||||
if (!parseComparison(comp2, ¬2, &op2, &value2, &expr2))
|
||||
continue;
|
||||
|
||||
if (isSameExpression(_tokenizer->isCPP(), comp1, comp2, _settings->library.functionpure))
|
||||
continue; // same expressions => only report that there are same expressions
|
||||
if (!isSameExpression(_tokenizer->isCPP(), expr1, expr2, _settings->library.functionpure)) {
|
||||
if ((tok->astOperand1()->isConstOp() && tok->astOperand2()->isConstOp()) &&
|
||||
isIncorrectCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
|
||||
|
||||
const bool alwaysTrue(tok->str() == "||");
|
||||
incorrectConditionError(tok, tok->expressionString(), alwaysTrue);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2);
|
||||
|
||||
// don't check floating point equality comparisons. that is bad
|
||||
// and deserves different warnings.
|
||||
if (isfloat && (op1 == "==" || op1 == "!=" || op2 == "==" || op2 == "!="))
|
||||
continue;
|
||||
|
||||
const double d1 = (isfloat) ? MathLib::toDoubleNumber(value1) : 0;
|
||||
const double d2 = (isfloat) ? MathLib::toDoubleNumber(value2) : 0;
|
||||
const MathLib::bigint i1 = (isfloat) ? 0 : MathLib::toLongNumber(value1);
|
||||
const MathLib::bigint i2 = (isfloat) ? 0 : MathLib::toLongNumber(value2);
|
||||
const bool useUnsignedInt = (std::numeric_limits<MathLib::bigint>::max()==i1)||(std::numeric_limits<MathLib::bigint>::max()==i2);
|
||||
const MathLib::biguint u1 = (useUnsignedInt) ? MathLib::toLongNumber(value1) : 0;
|
||||
const MathLib::biguint u2 = (useUnsignedInt) ? MathLib::toLongNumber(value2) : 0;
|
||||
// evaluate if expression is always true/false
|
||||
bool alwaysTrue = true, alwaysFalse = true;
|
||||
bool firstTrue = true, secondTrue = true;
|
||||
for (int test = 1; test <= 5; ++test) {
|
||||
// test:
|
||||
// 1 => testvalue is less than both value1 and value2
|
||||
// 2 => testvalue is value1
|
||||
// 3 => testvalue is between value1 and value2
|
||||
// 4 => testvalue value2
|
||||
// 5 => testvalue is larger than both value1 and value2
|
||||
bool result1, result2;
|
||||
if (isfloat) {
|
||||
const double testvalue = getvalue<double>(test, d1, d2);
|
||||
result1 = checkFloatRelation(op1, testvalue, d1);
|
||||
result2 = checkFloatRelation(op2, testvalue, d2);
|
||||
} else if (useUnsignedInt) {
|
||||
const MathLib::biguint testvalue = getvalue<MathLib::biguint>(test, u1, u2);
|
||||
result1 = checkIntRelation(op1, testvalue, u1);
|
||||
result2 = checkIntRelation(op2, testvalue, u2);
|
||||
} else {
|
||||
const MathLib::bigint testvalue = getvalue<MathLib::bigint>(test, i1, i2);
|
||||
result1 = checkIntRelation(op1, testvalue, i1);
|
||||
result2 = checkIntRelation(op2, testvalue, i2);
|
||||
}
|
||||
if (not1)
|
||||
result1 = !result1;
|
||||
if (not2)
|
||||
result2 = !result2;
|
||||
if (tok->str() == "&&") {
|
||||
alwaysTrue &= (result1 && result2);
|
||||
alwaysFalse &= !(result1 && result2);
|
||||
} else {
|
||||
alwaysTrue &= (result1 || result2);
|
||||
alwaysFalse &= !(result1 || result2);
|
||||
}
|
||||
firstTrue &= !(!result1 && result2);
|
||||
secondTrue &= !(result1 && !result2);
|
||||
}
|
||||
|
||||
const std::string cond1str = conditionString(not1, expr1, op1, value1);
|
||||
const std::string cond2str = conditionString(not2, expr2, op2, value2);
|
||||
if (printWarning && (alwaysTrue || alwaysFalse)) {
|
||||
const std::string text = cond1str + " " + tok->str() + " " + cond2str;
|
||||
incorrectConditionError(tok, text, alwaysTrue);
|
||||
} else if (printStyle && secondTrue) {
|
||||
const std::string text = "If '" + cond1str + "', the comparison '" + cond2str +
|
||||
"' is always " + (secondTrue ? "true" : "false") + ".";
|
||||
redundantConditionError(tok, text);
|
||||
} else if (printStyle && firstTrue) {
|
||||
//const std::string text = "The comparison " + cond1str + " is always " +
|
||||
// (firstTrue ? "true" : "false") + " when " +
|
||||
// cond2str + ".";
|
||||
const std::string text = "If '" + cond2str + "', the comparison '" + cond1str +
|
||||
"' is always " + (firstTrue ? "true" : "false") + ".";
|
||||
redundantConditionError(tok, text);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::incorrectConditionError(const Token *tok, const std::string &condition, bool always)
|
||||
{
|
||||
if (always)
|
||||
reportError(tok, Severity::warning, "IncorrectCondition",
|
||||
"Condition always evaluates to true: " + condition + ".\n"
|
||||
"Condition always evaluates to true: " + condition + ". "
|
||||
"Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
else
|
||||
reportError(tok, Severity::warning, "IncorrectCondition",
|
||||
"Condition always evaluates to false: " + condition + ".\n"
|
||||
"Condition always evaluates to false: " + condition + ". "
|
||||
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
}
|
||||
|
||||
|
||||
|
||||
void CheckCondition::checkIncorrectLogicOperator()
|
||||
{
|
||||
const bool printStyle = _settings->isEnabled("style");
|
||||
|
@ -994,7 +813,6 @@ void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::st
|
|||
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
}
|
||||
|
||||
|
||||
void CheckCondition::redundantConditionError(const Token *tok, const std::string &text)
|
||||
{
|
||||
reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text);
|
||||
|
|
|
@ -50,7 +50,6 @@ public:
|
|||
checkCondition.clarifyCondition(); // not simplified because ifAssign
|
||||
checkCondition.oppositeInnerCondition();
|
||||
checkCondition.checkIncorrectLogicOperator();
|
||||
checkCondition.checkIncorrectCondition();
|
||||
checkCondition.checkInvalidTestForOverflow();
|
||||
}
|
||||
|
||||
|
@ -89,7 +88,6 @@ public:
|
|||
|
||||
/** @brief %Check for testing for mutual exclusion over ||*/
|
||||
void checkIncorrectLogicOperator();
|
||||
void checkIncorrectCondition();
|
||||
|
||||
/** @brief %Check for suspicious usage of modulo (e.g. "if(var % 4 == 4)") */
|
||||
void checkModuloAlwaysTrueFalse();
|
||||
|
@ -120,7 +118,6 @@ private:
|
|||
void oppositeInnerConditionError(const Token *tok1, const Token* tok2);
|
||||
|
||||
void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always);
|
||||
void incorrectConditionError(const Token *tok, const std::string &condition, bool always);
|
||||
void redundantConditionError(const Token *tok, const std::string &text);
|
||||
|
||||
void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
|
||||
|
|
Loading…
Reference in New Issue