diff --git a/lib/astutils.cpp b/lib/astutils.cpp index ea5a12839..f1695cb68 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -393,7 +393,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) +bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure, ErrorPath* errors) { if (!cond1 || !cond2) return false; @@ -401,11 +401,11 @@ 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); + return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure, errors); if (cond2->astOperand2() && cond2->astOperand2()->str() == "0") - return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure); + return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, errors); } - return isSameExpression(cpp, true, cond1->astOperand1(), cond2, library, pure); + return isSameExpression(cpp, true, cond1->astOperand1(), cond2, library, pure, errors); } if (cond2->str() == "!") @@ -413,9 +413,9 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token if (!isNot) { if (cond1->str() == "==" && cond2->str() == "==") { - if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure)) + if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, errors)) return isDifferentKnownValues(cond1->astOperand2(), cond2->astOperand2()); - if (isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure)) + if (isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure, errors)) return isDifferentKnownValues(cond1->astOperand1(), cond2->astOperand1()); } // TODO: Handle reverse conditions @@ -480,7 +480,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)) + if (!isSameExpression(cpp, true, expr1, expr2, library, pure, errors)) return false; const ValueFlow::Value &rhsValue1 = value1->values().front(); @@ -508,16 +508,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) +bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, ErrorPath* errors) { if (!tok1 || !tok2) return false; if (isOppositeCond(true, cpp, tok1, tok2, library, pure)) return true; if (tok1->isUnaryOp("-")) - return isSameExpression(cpp, true, tok1->astOperand1(), tok2, library, pure); + return isSameExpression(cpp, true, tok1->astOperand1(), tok2, library, pure, errors); if (tok2->isUnaryOp("-")) - return isSameExpression(cpp, true, tok2->astOperand1(), tok1, library, pure); + return isSameExpression(cpp, true, tok2->astOperand1(), tok1, library, pure, errors); return false; } diff --git a/lib/astutils.h b/lib/astutils.h index 83f40d7a2..b15cde9f2 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -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); +bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, 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 isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, ErrorPath* errors=nullptr); bool isConstExpression(const Token *tok, const Library& library, bool pure); diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 72b055c9c..39f3cb8fc 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -565,6 +565,8 @@ void CheckCondition::multiCondition2() // Condition.. const Token *cond2 = tok->next()->astOperand2(); + ErrorPath errorPath; + if (type == MULTICONDITIONTYPE::INNER) { std::stack tokens1; tokens1.push(cond1); @@ -576,11 +578,11 @@ void CheckCondition::multiCondition2() if (firstCondition->str() == "&&") { tokens1.push(firstCondition->astOperand1()); tokens1.push(firstCondition->astOperand2()); - } else if (isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true)) { + } else if (isOppositeCond(false, mTokenizer->isCPP(), firstCondition, cond2, mSettings->library, true, &errorPath)) { if (!isAliased(vars)) - oppositeInnerConditionError(firstCondition, cond2); - } else if (isSameExpression(mTokenizer->isCPP(), true, firstCondition, cond2, mSettings->library, true)) { - identicalInnerConditionError(firstCondition, cond2); + oppositeInnerConditionError(firstCondition, cond2, errorPath); + } else if (isSameExpression(mTokenizer->isCPP(), true, firstCondition, cond2, mSettings->library, true, &errorPath)) { + identicalInnerConditionError(firstCondition, cond2, errorPath); } } } else { @@ -594,9 +596,9 @@ void CheckCondition::multiCondition2() if (secondCondition->str() == "||" || secondCondition->str() == "&&") { tokens2.push(secondCondition->astOperand1()); tokens2.push(secondCondition->astOperand2()); - } else if (isSameExpression(mTokenizer->isCPP(), true, cond1, secondCondition, mSettings->library, true)) { + } else if (isSameExpression(mTokenizer->isCPP(), true, cond1, secondCondition, mSettings->library, true, &errorPath)) { if (!isAliased(vars)) - identicalConditionAfterEarlyExitError(cond1, secondCondition); + identicalConditionAfterEarlyExitError(cond1, secondCondition, errorPath); } } } @@ -674,39 +676,36 @@ void CheckCondition::multiCondition2() } } -void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* tok2) +void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath) { const std::string s1(tok1 ? tok1->expressionString() : "x"); const std::string s2(tok2 ? tok2->expressionString() : "!x"); - ErrorPath errorPath = { - ErrorPathItem(tok1, "outer condition: " + s1), - ErrorPathItem(tok2, "opposite inner condition: " + s2) - }; + errorPath.emplace_back(ErrorPathItem(tok1, "outer condition: " + s1)); + errorPath.emplace_back(ErrorPathItem(tok2, "opposite inner condition: " + s2)); + const std::string msg("Opposite inner 'if' condition leads to a dead code block.\n" "Opposite inner 'if' condition leads to a dead code block (outer condition is '" + s1 + "' and inner condition is '" + s2 + "')."); reportError(errorPath, Severity::warning, "oppositeInnerCondition", msg, CWE398, false); } -void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token* tok2) +void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath) { const std::string s1(tok1 ? tok1->expressionString() : "x"); const std::string s2(tok2 ? tok2->expressionString() : "x"); - ErrorPath errorPath = { - ErrorPathItem(tok1, "outer condition: " + s1), - ErrorPathItem(tok2, "identical inner condition: " + s2) - }; + errorPath.emplace_back(ErrorPathItem(tok1, "outer condition: " + s1)); + errorPath.emplace_back(ErrorPathItem(tok2, "identical inner condition: " + s2)); + const std::string msg("Identical inner 'if' condition is always true.\n" "Identical inner 'if' condition is always true (outer condition is '" + s1 + "' and inner condition is '" + s2 + "')."); reportError(errorPath, Severity::warning, "identicalInnerCondition", msg, CWE398, false); } -void CheckCondition::identicalConditionAfterEarlyExitError(const Token *cond1, const Token* cond2) +void CheckCondition::identicalConditionAfterEarlyExitError(const Token *cond1, const Token* cond2, ErrorPath errorPath) { const std::string cond(cond1 ? cond1->expressionString() : "x"); - ErrorPath errorPath = { - ErrorPathItem(cond1, "first condition"), - ErrorPathItem(cond2, "second condition") - }; + errorPath.emplace_back(ErrorPathItem(cond1, "first condition")); + errorPath.emplace_back(ErrorPathItem(cond2, "second condition")); + reportError(errorPath, Severity::warning, "identicalConditionAfterEarlyExit", "Identical condition '" + cond + "', second condition is always false", CWE398, false); } diff --git a/lib/checkcondition.h b/lib/checkcondition.h index dec3f892b..ac6e08a5f 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -131,11 +131,11 @@ private: bool result); void multiConditionError(const Token *tok, unsigned int line1); - void oppositeInnerConditionError(const Token *tok1, const Token* tok2); + void oppositeInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath); - void identicalInnerConditionError(const Token *tok1, const Token* tok2); + void identicalInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath); - void identicalConditionAfterEarlyExitError(const Token *cond1, const Token *cond2); + void identicalConditionAfterEarlyExitError(const Token *cond1, const Token *cond2, ErrorPath errorPath); void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive); void redundantConditionError(const Token *tok, const std::string &text, bool inconclusive); @@ -152,14 +152,16 @@ private: void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override { CheckCondition c(nullptr, settings, errorLogger); + ErrorPath errorPath; + c.assignIfError(nullptr, nullptr, emptyString, false); c.badBitmaskCheckError(nullptr); c.comparisonError(nullptr, "&", 6, "==", 1, false); c.multiConditionError(nullptr,1); c.mismatchingBitAndError(nullptr, 0xf0, nullptr, 1); - c.oppositeInnerConditionError(nullptr, nullptr); - c.identicalInnerConditionError(nullptr, nullptr); - c.identicalConditionAfterEarlyExitError(nullptr, nullptr); + c.oppositeInnerConditionError(nullptr, nullptr, errorPath); + c.identicalInnerConditionError(nullptr, nullptr, errorPath); + c.identicalConditionAfterEarlyExitError(nullptr, nullptr, errorPath); c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false); c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false); c.moduloAlwaysTrueFalseError(nullptr, "1"); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index b56f1e9c1..f5cf287ab 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2005,10 +2005,10 @@ void CheckOther::checkDuplicateExpression() } } } else if (styleEnabled && - isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false) && + isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, &errorPath) && !Token::Match(tok, "=|-|-=|/|/=") && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { - oppositeExpressionError(tok, tok, tok->str()); + 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())) duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath); @@ -2037,11 +2037,14 @@ void CheckOther::checkDuplicateExpression() } } -void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op) +void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op, ErrorPath errors) { - const std::list toks = { tok2, tok1 }; + if(tok1) + errors.emplace_back(tok1, ""); + if(tok2) + errors.emplace_back(tok2, ""); - reportError(toks, Severity::style, "oppositeExpression", "Opposite expression on both sides of \'" + op + "\'.\n" + reportError(errors, Severity::style, "oppositeExpression", "Opposite expression on both sides of \'" + op + "\'.\n" "Finding the opposite expression on both sides of an operator is suspicious and might " "indicate a cut and paste or logic error. Please examine this code carefully to " "determine if it is correct.", CWE398, false); diff --git a/lib/checkother.h b/lib/checkother.h index b6fe0632b..9d275c52a 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -239,7 +239,7 @@ private: void misusedScopeObjectError(const Token *tok, const std::string &varname); void duplicateBranchError(const Token *tok1, const Token *tok2); void duplicateAssignExpressionError(const Token *tok1, const Token *tok2); - void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op); + void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op, ErrorPath errors); void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors); void duplicateValueTernaryError(const Token *tok); void duplicateExpressionTernaryError(const Token *tok); @@ -302,7 +302,7 @@ private: c.clarifyCalculationError(nullptr, "+"); c.clarifyStatementError(nullptr); c.duplicateBranchError(nullptr, nullptr); - c.oppositeExpressionError(nullptr, nullptr, "&&"); + c.oppositeExpressionError(nullptr, nullptr, "&&", errorPath); c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath); c.duplicateValueTernaryError(nullptr); c.duplicateExpressionTernaryError(nullptr);