diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index cf54c882f34..a56aea7a4b5 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -602,7 +602,7 @@ void CheckCondition::oppositeElseIfConditionError(const Token *ifCond, const Tok //--------------------------------------------------------------------------- // - Opposite inner conditions => always false -// - (TODO) Same/Overlapping inner condition => always true +// - Same/Overlapping inner condition => always true // - same condition after early exit => always false //--------------------------------------------------------------------------- @@ -759,6 +759,8 @@ void CheckCondition::multiCondition2() oppositeInnerConditionError(firstCondition, cond2, errorPath); } else if (!isReturnVar && isSameExpression(true, firstCondition, cond2, *mSettings, true, true, &errorPath)) { identicalInnerConditionError(firstCondition, cond2, errorPath); + } else if (!isReturnVar && isOverlappingCond(cond2, firstCondition, true)) { + overlappingInnerConditionError(firstCondition, cond2, errorPath); } } return ChildrenToVisit::none; @@ -879,6 +881,21 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* reportError(std::move(errorPath), Severity::warning, "oppositeInnerCondition", msg, CWE398, Certainty::normal); } +void CheckCondition::overlappingInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath) +{ + if (diag(tok1, tok2)) + return; + const std::string s1(tok1 ? tok1->expressionString() : "x"); + const std::string s2(tok2 ? tok2->expressionString() : "x"); + const std::string innerSmt = innerSmtString(tok2); + errorPath.emplace_back(tok1, "outer condition: " + s1); + errorPath.emplace_back(tok2, "overlapping inner condition: " + s2); + + const std::string msg("Overlapping inner '" + innerSmt + "' condition is always true.\n" + "Overlapping inner '" + innerSmt + "' condition is always true (outer condition is '" + s1 + "' and inner condition is '" + s2 + "')."); + reportError(std::move(errorPath), Severity::warning, "overlappingInnerCondition", msg, CWE398, Certainty::normal); +} + void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath) { if (diag(tok1, tok2)) @@ -2111,6 +2128,7 @@ void CheckCondition::getErrorMessages(ErrorLogger *errorLogger, const Settings * c.comparisonError(nullptr, "&", 6, "==", 1, false); c.duplicateConditionError(nullptr, nullptr, ErrorPath{}); c.overlappingElseIfConditionError(nullptr, 1); + c.overlappingInnerConditionError(nullptr, nullptr, ErrorPath()); c.mismatchingBitAndError(nullptr, 0xf0, nullptr, 1); c.oppositeInnerConditionError(nullptr, nullptr, ErrorPath{}); c.identicalInnerConditionError(nullptr, nullptr, ErrorPath{}); diff --git a/lib/checkcondition.h b/lib/checkcondition.h index dac3ba8d0de..2c010cd7c5a 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -84,7 +84,7 @@ class CPPCHECKLIB CheckCondition : public Check { /** * multiconditions #2 * - Opposite inner conditions => always false - * - (TODO) Same/Overlapping inner condition => always true + * - Same/Overlapping inner condition => always true * - same condition after early exit => always false **/ void multiCondition2(); @@ -130,6 +130,7 @@ class CPPCHECKLIB CheckCondition : public Check { bool result); void duplicateConditionError(const Token *tok1, const Token *tok2, ErrorPath errorPath); void overlappingElseIfConditionError(const Token *tok, nonneg int line1); + void overlappingInnerConditionError(const Token *tok1, const Token *tok2, ErrorPath errorPath); void oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath); void oppositeInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 421fa53e2bc..20e13b62f82 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -94,6 +94,8 @@ class TestCondition : public TestFixture { TEST_CASE(identicalConditionAfterEarlyExit); TEST_CASE(innerConditionModified); + TEST_CASE(overlappingInnerCondition); + TEST_CASE(clarifyCondition1); // if (a = b() < 0) TEST_CASE(clarifyCondition2); // if (a & b == c) TEST_CASE(clarifyCondition3); // if (! a & b) @@ -3022,6 +3024,22 @@ class TestCondition : public TestFixture { ASSERT_EQUALS("", errout_str()); } + void overlappingInnerCondition() { + check("void f(int x) {\n" + " if (x == 1) {\n" + " if (x & 7) {}\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:3:15]: (warning) Overlapping inner 'if' condition is always true. [overlappingInnerCondition]\n", errout_str()); + + check("void f(int x) {\n" + " if (x & 7) {\n" + " if (x == 1) {}\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout_str()); + } + // clarify conditions with = and comparison void clarifyCondition1() { check("void f() {\n"