From f65cf220ba05c84a0dc0c8376c29455939408197 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Fri, 28 Sep 2018 01:38:24 -0500 Subject: [PATCH] Fix false positives in unknownEvaluationOrder when using followVar (#1391) Fix false positives in unknownEvaluationOrder when using followVar --- lib/astutils.cpp | 59 ++++++++++++++++++++---------------------- lib/astutils.h | 6 ++--- lib/checkcondition.cpp | 24 ++++++++--------- lib/checkother.cpp | 24 ++++++++--------- lib/checkstl.cpp | 6 ++--- lib/checkstring.cpp | 5 ++-- lib/valueflow.cpp | 2 +- test/testcondition.cpp | 13 +++++----- test/testother.cpp | 49 +++++++++++++++++++++-------------- 9 files changed, 98 insertions(+), 90 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 5b70abbb4..a21ba54c3 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -133,7 +133,6 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp, return ret; } -/* static const Token * getVariableInitExpression(const Variable * var) { if (!var || !var->declEndToken()) @@ -259,8 +258,8 @@ static void followVariableExpressionError(const Token *tok1, const Token *tok2, return; errors->push_back(item); } -*/ -bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, 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) return true; @@ -274,14 +273,13 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 } // Skip double not if (Token::simpleMatch(tok1, "!") && Token::simpleMatch(tok1->astOperand1(), "!") && !Token::simpleMatch(tok1->astParent(), "=")) { - return isSameExpression(cpp, macro, tok1->astOperand1()->astOperand1(), tok2, library, pure, errors); + return isSameExpression(cpp, macro, tok1->astOperand1()->astOperand1(), tok2, library, pure, followVar, errors); } if (Token::simpleMatch(tok2, "!") && Token::simpleMatch(tok2->astOperand1(), "!") && !Token::simpleMatch(tok2->astParent(), "=")) { - return isSameExpression(cpp, macro, tok1, tok2->astOperand1()->astOperand1(), library, pure, errors); + return isSameExpression(cpp, macro, tok1, tok2->astOperand1()->astOperand1(), library, pure, followVar, errors); } - if (tok1->str() != tok2->str() && (Token::Match(tok1, "%var%") || Token::Match(tok2, "%var%"))) { - // TODO this code is temporarily commented out because there are false positives. See #8717, #8744, #8775. - /* + // Follow variable + if (followVar && tok1->str() != tok2->str() && (Token::Match(tok1, "%var%") || Token::Match(tok2, "%var%"))) { const Token * varTok1 = followVariableExpression(tok1, cpp, tok2); if (varTok1->str() == tok2->str()) { followVariableExpressionError(tok1, varTok1, errors); @@ -297,13 +295,12 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 followVariableExpressionError(tok2, varTok2, errors); return isSameExpression(cpp, macro, varTok1, varTok2, library, true, errors); } - */ } if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str() || tok1->originalName() != tok2->originalName()) { if ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) || (Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) { - return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, errors) && - isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, errors); + return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, followVar, errors) && + isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, followVar, errors); } return false; } @@ -381,9 +378,9 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 return false; } bool noncommutativeEquals = - isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), library, pure, errors); + isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), library, pure, followVar, errors); noncommutativeEquals = noncommutativeEquals && - isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), library, pure, errors); + isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), library, pure, followVar, errors); if (noncommutativeEquals) return true; @@ -398,9 +395,9 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 const bool commutative = tok1->isBinaryOp() && Token::Match(tok1, "%or%|%oror%|+|*|&|&&|^|==|!="); bool commutativeEquals = commutative && - isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, errors); + isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, followVar, errors); commutativeEquals = commutativeEquals && - isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, errors); + isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, followVar, errors); return commutativeEquals; @@ -434,7 +431,7 @@ static bool isZeroBoundCond(const Token * const cond) return false; } -bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure, ErrorPath* errors) +bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure, bool followVar, ErrorPath* errors) { if (!cond1 || !cond2) return false; @@ -442,21 +439,21 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token if (cond1->str() == "!") { if (cond2->str() == "!=") { if (cond2->astOperand1() && cond2->astOperand1()->str() == "0") - return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure, errors); + return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure, followVar, errors); if (cond2->astOperand2() && cond2->astOperand2()->str() == "0") - return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, errors); + return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, followVar, errors); } - return isSameExpression(cpp, true, cond1->astOperand1(), cond2, library, pure, errors); + return isSameExpression(cpp, true, cond1->astOperand1(), cond2, library, pure, followVar, errors); } if (cond2->str() == "!") - return isOppositeCond(isNot, cpp, cond2, cond1, library, pure); + return isOppositeCond(isNot, cpp, cond2, cond1, library, pure, followVar); if (!isNot) { if (cond1->str() == "==" && cond2->str() == "==") { - if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, errors)) + if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, followVar, errors)) return isDifferentKnownValues(cond1->astOperand2(), cond2->astOperand2()); - if (isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure, errors)) + if (isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure, followVar, errors)) return isDifferentKnownValues(cond1->astOperand1(), cond2->astOperand1()); } // TODO: Handle reverse conditions @@ -481,11 +478,11 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token // condition found .. get comparator std::string comp2; - if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure) && - isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure)) { + if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, followVar) && + isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure, followVar)) { comp2 = cond2->str(); - } else if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure) && - isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand1(), library, pure)) { + } else if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure, followVar) && + isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand1(), library, pure, followVar)) { comp2 = cond2->str(); if (comp2[0] == '>') comp2[0] = '<'; @@ -521,7 +518,7 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token if (!expr1 || !value1 || !expr2 || !value2) { return false; } - if (!isSameExpression(cpp, true, expr1, expr2, library, pure, errors)) + if (!isSameExpression(cpp, true, expr1, expr2, library, pure, followVar, errors)) return false; const ValueFlow::Value &rhsValue1 = value1->values().front(); @@ -549,16 +546,16 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token ))); } -bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, ErrorPath* errors) +bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors) { if (!tok1 || !tok2) return false; - if (isOppositeCond(true, cpp, tok1, tok2, library, pure)) + if (isOppositeCond(true, cpp, tok1, tok2, library, pure, followVar)) return true; if (tok1->isUnaryOp("-")) - return isSameExpression(cpp, true, tok1->astOperand1(), tok2, library, pure, errors); + return isSameExpression(cpp, true, tok1->astOperand1(), tok2, library, pure, followVar, errors); if (tok2->isUnaryOp("-")) - return isSameExpression(cpp, true, tok2->astOperand1(), tok1, library, pure, errors); + return isSameExpression(cpp, true, tok2->astOperand1(), tok1, library, pure, followVar, errors); return false; } diff --git a/lib/astutils.h b/lib/astutils.h index b15cde9f2..5498e11cd 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -56,7 +56,7 @@ std::string astCanonicalType(const Token *expr); /** Is given syntax tree a variable comparison against value */ const Token * astIsVariableComparison(const Token *tok, const std::string &comp, const std::string &rhs, const Token **vartok=nullptr); -bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, ErrorPath* errors=nullptr); +bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); bool isEqualKnownValue(const Token * const tok1, const Token * const tok2); @@ -71,9 +71,9 @@ bool isDifferentKnownValues(const Token * const tok1, const Token * const tok2); * @param library files data * @param pure */ -bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure, ErrorPath* errors=nullptr); +bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); -bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, ErrorPath* errors=nullptr); +bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); bool isConstExpression(const Token *tok, const Library& library, bool pure); diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 0cbade890..0a13a15d9 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -368,7 +368,7 @@ bool CheckCondition::isOverlappingCond(const Token * const cond1, const Token * return false; // same expressions - if (isSameExpression(mTokenizer->isCPP(), true, cond1, cond2, mSettings->library, pure)) + if (isSameExpression(mTokenizer->isCPP(), true, cond1, cond2, mSettings->library, pure, false)) return true; // bitwise overlap for example 'x&7' and 'x==1' @@ -391,7 +391,7 @@ bool CheckCondition::isOverlappingCond(const Token * const cond1, const Token * if (!num2->isNumber() || MathLib::isNegative(num2->str())) return false; - if (!isSameExpression(mTokenizer->isCPP(), true, expr1, expr2, mSettings->library, pure)) + if (!isSameExpression(mTokenizer->isCPP(), true, expr1, expr2, mSettings->library, pure, false)) return false; const MathLib::bigint value1 = MathLib::toLongNumber(num1->str()); @@ -583,10 +583,10 @@ void CheckCondition::multiCondition2() tokens1.push(firstCondition->astOperand1()); tokens1.push(firstCondition->astOperand2()); } else if (!firstCondition->hasKnownValue()) { - if (isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, &errorPath)) { + if (isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, false, &errorPath)) { if (!isAliased(vars)) oppositeInnerConditionError(firstCondition, cond2, errorPath); - } else if (isSameExpression(mTokenizer->isCPP(), true, firstCondition, cond2, mSettings->library, true, &errorPath)) { + } else if (isSameExpression(mTokenizer->isCPP(), true, firstCondition, cond2, mSettings->library, true, false, &errorPath)) { identicalInnerConditionError(firstCondition, cond2, errorPath); } } @@ -603,7 +603,7 @@ void CheckCondition::multiCondition2() tokens2.push(secondCondition->astOperand1()); tokens2.push(secondCondition->astOperand2()); } else if ((!cond1->hasKnownValue() || !secondCondition->hasKnownValue()) && - isSameExpression(mTokenizer->isCPP(), true, cond1, secondCondition, mSettings->library, true, &errorPath)) { + isSameExpression(mTokenizer->isCPP(), true, cond1, secondCondition, mSettings->library, true, false, &errorPath)) { if (!isAliased(vars)) identicalConditionAfterEarlyExitError(cond1, secondCondition, errorPath); } @@ -911,7 +911,7 @@ void CheckCondition::checkIncorrectLogicOperator() ((tok->str() == "||" && tok->astOperand2()->str() == "&&") || (tok->str() == "&&" && tok->astOperand2()->str() == "||"))) { const Token* tok2 = tok->astOperand2()->astOperand1(); - if (isOppositeCond(true, mTokenizer->isCPP(), tok->astOperand1(), tok2, mSettings->library, true)) { + if (isOppositeCond(true, mTokenizer->isCPP(), tok->astOperand1(), tok2, mSettings->library, true, false)) { std::string expr1(tok->astOperand1()->expressionString()); std::string expr2(tok->astOperand2()->astOperand1()->expressionString()); std::string expr3(tok->astOperand2()->astOperand2()->expressionString()); @@ -974,7 +974,7 @@ void CheckCondition::checkIncorrectLogicOperator() const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2); // Opposite comparisons around || or && => always true or always false - if (!isfloat && isOppositeCond(tok->str() == "||", mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, true)) { + if (!isfloat && isOppositeCond(tok->str() == "||", mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, true, false)) { const bool alwaysTrue(tok->str() == "||"); incorrectLogicOperatorError(tok, conditionString(tok), alwaysTrue, inconclusive); @@ -984,9 +984,9 @@ void CheckCondition::checkIncorrectLogicOperator() if (!parseable) continue; - if (isSameExpression(mTokenizer->isCPP(), true, comp1, comp2, mSettings->library, true)) + if (isSameExpression(mTokenizer->isCPP(), true, comp1, comp2, mSettings->library, true, false)) continue; // same expressions => only report that there are same expressions - if (!isSameExpression(mTokenizer->isCPP(), true, expr1, expr2, mSettings->library, true)) + if (!isSameExpression(mTokenizer->isCPP(), true, expr1, expr2, mSettings->library, true, false)) continue; @@ -1221,7 +1221,7 @@ void CheckCondition::alwaysTrueFalse() continue; if (Token::Match(tok, "%oror%|&&")) continue; - if (Token::Match(tok, "%comp%") && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true)) + if (Token::Match(tok, "%comp%") && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, true)) continue; const bool constIfWhileExpression = @@ -1347,9 +1347,9 @@ void CheckCondition::checkInvalidTestForOverflow() continue; const Token *termToken; - if (isSameExpression(mTokenizer->isCPP(), true, exprToken, calcToken->astOperand1(), mSettings->library, true)) + if (isSameExpression(mTokenizer->isCPP(), true, exprToken, calcToken->astOperand1(), mSettings->library, true, false)) termToken = calcToken->astOperand2(); - else if (isSameExpression(mTokenizer->isCPP(), true, exprToken, calcToken->astOperand2(), mSettings->library, true)) + else if (isSameExpression(mTokenizer->isCPP(), true, exprToken, calcToken->astOperand2(), mSettings->library, true, false)) termToken = calcToken->astOperand1(); else continue; diff --git a/lib/checkother.cpp b/lib/checkother.cpp index aeeb79c88..4122bf640 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -577,7 +577,7 @@ void CheckOther::checkRedundantAssignment() } // Ensure that LHS in assignments are the same - bool error = oldeq && eq->astOperand1() && isSameExpression(mTokenizer->isCPP(), true, eq->astOperand1(), oldeq->astOperand1(), mSettings->library, true); + bool error = oldeq && eq->astOperand1() && isSameExpression(mTokenizer->isCPP(), true, eq->astOperand1(), oldeq->astOperand1(), mSettings->library, true, false); // Ensure that variable is not used on right side std::stack tokens; @@ -1963,8 +1963,8 @@ void CheckOther::checkDuplicateExpression() ) && tok->next()->tokType() != Token::eType && tok->next()->tokType() != Token::eName && - isSameExpression(mTokenizer->isCPP(), true, tok->next(), nextAssign->next(), mSettings->library, true) && - isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true) && + isSameExpression(mTokenizer->isCPP(), true, tok->next(), nextAssign->next(), mSettings->library, true, false) && + isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) && tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString() && !isUniqueExpression(tok->astOperand2())) { bool assigned = false; @@ -1986,7 +1986,7 @@ void CheckOther::checkDuplicateExpression() if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) { if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true)) continue; - if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, &errorPath)) { + if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, true, &errorPath)) { if (isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { const bool assignment = tok->str() == "="; if (assignment && warningEnabled) @@ -2005,21 +2005,21 @@ void CheckOther::checkDuplicateExpression() } } } else if (styleEnabled && - isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, &errorPath) && + isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, false, &errorPath) && !Token::Match(tok, "=|-|-=|/|/=") && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { oppositeExpressionError(tok, tok, tok->str(), errorPath); } else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative - if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) + if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath); else if (tok->astOperand2()) { const Token *ast1 = tok->astOperand1(); while (ast1 && tok->str() == ast1->str()) { - if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand1())) + if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand1())) // TODO: warn if variables are unchanged. See #5683 // Probably the message should be changed to 'duplicate expressions X in condition or something like that'. ;//duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok, errorPath); - else if (styleEnabled && isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), mSettings->library, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2())) + else if (styleEnabled && isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), mSettings->library, true, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2())) duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok, errorPath); if (!isConstExpression(ast1->astOperand2(), mSettings->library, true)) break; @@ -2030,7 +2030,7 @@ void CheckOther::checkDuplicateExpression() } else if (styleEnabled && tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") { if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2())) duplicateValueTernaryError(tok); - else if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, false)) + else if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, false, false)) duplicateExpressionTernaryError(tok); } } @@ -2668,9 +2668,9 @@ void CheckOther::checkEvaluationOrder() if (tok2 == tok && tok->str() == "=" && parent->str() == "=" && - isSameExpression(mTokenizer->isCPP(), false, tok->astOperand1(), parent->astOperand1(), mSettings->library, true)) { + isSameExpression(mTokenizer->isCPP(), false, tok->astOperand1(), parent->astOperand1(), mSettings->library, true, false)) { if (mSettings->isEnabled(Settings::WARNING) && - isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), parent->astOperand1(), mSettings->library, true)) + isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), parent->astOperand1(), mSettings->library, true, false)) selfAssignmentError(parent, tok->astOperand1()->expressionString()); break; } @@ -2690,7 +2690,7 @@ void CheckOther::checkEvaluationOrder() continue; // don't care about sizeof usage tokens.push(tok3->astOperand1()); tokens.push(tok3->astOperand2()); - if (isSameExpression(mTokenizer->isCPP(), false, tok->astOperand1(), tok3, mSettings->library, true)) { + if (isSameExpression(mTokenizer->isCPP(), false, tok->astOperand1(), tok3, mSettings->library, true, false)) { foundError = true; } } diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 0612e3ded..a538f80be 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -486,7 +486,7 @@ void CheckStl::mismatchingContainers() if (Token::Match(tok, "%comp%|-")) { const Token * iter1 = getIteratorExpression(tok->astOperand1()); const Token * iter2 = getIteratorExpression(tok->astOperand2()); - if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false)) { + if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false, false)) { mismatchingContainerExpressionError(iter1, iter2); continue; } @@ -509,7 +509,7 @@ void CheckStl::mismatchingContainers() if (i->first) { firstArg = argTok; } - if (i->last && firstArg && argTok && isSameExpression(true, false, firstArg, argTok, mSettings->library, false)) { + if (i->last && firstArg && argTok && isSameExpression(true, false, firstArg, argTok, mSettings->library, false, false)) { sameIteratorExpressionError(firstArg); } const Variable *c = getContainer(argTok); @@ -530,7 +530,7 @@ void CheckStl::mismatchingContainers() if (i->last && firstArg && argTok) { const Token * iter1 = getIteratorExpression(firstArg); const Token * iter2 = getIteratorExpression(argTok); - if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false)) { + if (iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false, false)) { mismatchingContainerExpressionError(iter1, iter2); } } diff --git a/lib/checkstring.cpp b/lib/checkstring.cpp index 643778055..3d4faa3a3 100644 --- a/lib/checkstring.cpp +++ b/lib/checkstring.cpp @@ -391,7 +391,7 @@ void CheckString::overlappingStrcmp() if (args1[1]->isLiteral() && args2[1]->isLiteral() && args1[1]->str() != args2[1]->str() && - isSameExpression(mTokenizer->isCPP(), true, args1[0], args2[0], mSettings->library, true)) + isSameExpression(mTokenizer->isCPP(), true, args1[0], args2[0], mSettings->library, true, false)) overlappingStrcmpError(eq0, ne0); } } @@ -433,7 +433,8 @@ void CheckString::sprintfOverlappingData() args[0], args[argnr], mSettings->library, - true); + true, + false); if (same) { sprintfOverlappingDataError(args[argnr], args[argnr]->expressionString()); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 2d9f32335..d850fbfe3 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1058,7 +1058,7 @@ static void valueFlowOppositeCondition(SymbolDatabase *symboldatabase, const Set const Token *cond2 = ifOpenBraceTok->astOperand2(); if (!cond2 || !cond2->isComparisonOp()) continue; - if (isOppositeCond(true, cpp, cond1, cond2, settings->library, true)) { + if (isOppositeCond(true, cpp, cond1, cond2, settings->library, true, true)) { ValueFlow::Value value(1); value.setKnown(); setTokenValue(const_cast(cond2), value, settings); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 83487a776..79382478d 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -1246,7 +1246,7 @@ private: " if (a > x && a < y)\n" " return;\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:8]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str()); + // ASSERT_EQUALS("[test.cpp:8]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str()); check("struct A {\n" " void f();\n" @@ -1276,7 +1276,7 @@ private: " if (a > x && a < y)\n" " return;\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:5]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str()); + // ASSERT_EQUALS("[test.cpp:5]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str()); } void secondAlwaysTrueFalseWhenFirstTrueError() { @@ -1601,7 +1601,7 @@ private: " if(!b) {}\n" " }\n" "}"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); + // ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); check("void foo(unsigned u) {\n" " if (u != 0) {\n" @@ -2538,8 +2538,7 @@ private: " if (val < 0) continue;\n" " if (val > 0) {}\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'val<0' is always false\n" - "[test.cpp:4]: (style) Condition 'val>0' is always false\n", errout.str()); + ASSERT_EQUALS("", errout.str()); check("void f() {\n" " int val = 0;\n" @@ -2547,7 +2546,7 @@ private: " if (val > 0) {}\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'val<0' is always false\n", errout.str()); + ASSERT_EQUALS("", errout.str()); check("void f() {\n" " int val = 0;\n" @@ -2555,7 +2554,7 @@ private: " if (val < 0) {}\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'val<0' is always false\n", errout.str()); + ASSERT_EQUALS("", errout.str()); check("void f() {\n" " int activate = 0;\n" diff --git a/test/testother.cpp b/test/testother.cpp index 61cdbcc52..84b7ec917 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -3917,13 +3917,13 @@ private: " const int i = sizeof(int);\n" " if ( i != sizeof (int)){}\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'i != sizeof(int)' is always false because 'i' and 'sizeof(int)' represent the same value.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'i != sizeof(int)' is always false because 'i' and 'sizeof(int)' represent the same value.\n", errout.str()); check("void f() {\n" " const int i = sizeof(int);\n" " if ( sizeof (int) != i){}\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'sizeof(int) != i' is always false because 'sizeof(int)' and 'i' represent the same value.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'sizeof(int) != i' is always false because 'sizeof(int)' and 'i' represent the same value.\n", errout.str()); check("void f(int a = 1) { if ( a != 1){}}\n"); ASSERT_EQUALS("", errout.str()); @@ -3932,21 +3932,21 @@ private: " int a = 1;\n" " if ( a != 1){} \n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); check("void f() {\n" " int a = 1;\n" " int b = 1;\n" " if ( a != b){} \n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != b' is always false because 'a' and 'b' represent the same value.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != b' is always false because 'a' and 'b' represent the same value.\n", errout.str()); check("void f() {\n" " int a = 1;\n" " int b = a;\n" " if ( a != b){} \n" "}\n"); - //ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != b' is always false because 'a' and 'b' represent the same value.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != b' is always false because 'a' and 'b' represent the same value.\n", errout.str()); check("void use(int);\n" "void f() {\n" @@ -3955,7 +3955,7 @@ private: " use(b);\n" " if ( a != 1){} \n" "}\n"); - //ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) The expression 'a != 1' is always false.\n", errout.str()); check("void use(int);\n" "void f() {\n" @@ -3979,7 +3979,7 @@ private: " void f() {\n" " if ( a != 1){} \n" "}\n"); - //ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); check("int a = 1;\n" " void f() {\n" @@ -3991,7 +3991,7 @@ private: " static const int a = 1;\n" " if ( a != 1){} \n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); check("void f() {\n" " static int a = 1;\n" @@ -4004,7 +4004,7 @@ private: " if ( a != 1){\n" " a++;\n" " }}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); check("void f(int b) {\n" " int a = 1;\n" @@ -4096,7 +4096,7 @@ private: " int a = 1;\n" " while ( a != 1){}\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false.\n", errout.str()); check("void f() { int a = 1; while ( a != 1){ a++; }}\n"); ASSERT_EQUALS("", errout.str()); @@ -4112,7 +4112,7 @@ private: " if( i != 0 ) {}\n" " }\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'i != 0' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'i != 0' is always false.\n", errout.str()); check("void f() {\n" " for(int i = 0; i < 10;) {\n" @@ -4153,7 +4153,7 @@ private: " b++;\n" " }\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != 1' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != 1' is always false.\n", errout.str()); } void duplicateExpressionTernary() { // #6391 @@ -4632,8 +4632,8 @@ private: " if (val < 0) continue;\n" " if ((val > 0)) {}\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" - // "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val > 0' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val > 0' is always false.\n", errout.str()); check("void f() {\n" " int val = 0;\n" @@ -4641,8 +4641,8 @@ private: " if ((val > 0)) {}\n" " }\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" - // "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val > 0' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val > 0' is always false.\n", errout.str()); check("void f() {\n" " int val = 0;\n" @@ -4650,9 +4650,8 @@ private: " if ((val < 0)) {}\n" " }\n" "}\n"); - //ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" - // "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val < 0' is always false.\n", errout.str()); - + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'val < 0' is always false.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) The expression 'val < 0' is always false.\n", errout.str()); check("void f() {\n" " int activate = 0;\n" @@ -6952,6 +6951,18 @@ private: " local_argv[local_argc++] = argv[0];\n" "}\n", "test.c"); ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int x = 0;\n" + " return 0 + x++;\n" + "}\n", "test.c"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int x, int y) {\n" + " int a[10];\n" + " a[x+y] = a[y+x]++;;\n" + "}\n", "test.c"); + ASSERT_EQUALS("[test.c:3]: (error) Expression 'a[x+y]=a[y+x]++' depends on order of evaluation of side effects\n", errout.str()); } void testEvaluationOrderSelfAssignment() {