diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index ecf2c4a9b..e15a6c74c 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -507,12 +507,14 @@ void CheckCondition::multiCondition() break; tok2 = tok2->tokAt(4); - if (tok2->astOperand2()) { + if (tok2->astOperand2() && + !cond1->hasKnownIntValue() && + !tok2->astOperand2()->hasKnownIntValue()) { ErrorPath errorPath; if (isOverlappingCond(cond1, tok2->astOperand2(), true)) - overlappingElseIfConditionError(tok2->astOperand2(), cond1->linenr()); + overlappingElseIfConditionError(tok2, cond1->linenr()); else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath)) - oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath); + oppositeElseIfConditionError(cond1, tok2, errorPath); } } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 8e7883ddc..14e515168 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1427,6 +1427,37 @@ static void valueFlowRightShift(TokenList *tokenList, const Settings* settings) } } +static void valueFlowOppositeCondition(SymbolDatabase *symboldatabase, const Settings *settings) +{ + for (const Scope &scope : symboldatabase->scopeList) { + if (scope.type != Scope::eIf) + continue; + Token *tok = const_cast(scope.classDef); + if (!Token::simpleMatch(tok, "if (")) + continue; + const Token *cond1 = tok->next()->astOperand2(); + if (!cond1 || !cond1->isComparisonOp()) + continue; + const bool cpp = symboldatabase->isCPP(); + Token *tok2 = tok->linkAt(1); + while (Token::simpleMatch(tok2, ") {")) { + tok2 = tok2->linkAt(1); + if (!Token::simpleMatch(tok2, "} else { if (")) + break; + Token *ifOpenBraceTok = tok2->tokAt(4); + Token *cond2 = ifOpenBraceTok->astOperand2(); + if (!cond2 || !cond2->isComparisonOp()) + continue; + if (isOppositeCond(true, cpp, cond1, cond2, settings->library, true, true)) { + ValueFlow::Value value(1); + value.setKnown(); + setTokenValue(cond2, value, settings); + } + tok2 = ifOpenBraceTok->link(); + } + } +} + static void valueFlowEnumValue(SymbolDatabase * symboldatabase, const Settings * settings) { @@ -3725,19 +3756,7 @@ static const Token* findIncompleteVar(const Token* start, const Token* end) return nullptr; } -static ValueFlow::Value makeConditionValue(long long val, const Token* condTok, bool assume) -{ - ValueFlow::Value v(val); - v.setKnown(); - v.condition = condTok; - if (assume) - v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true"); - else - v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false"); - return v; -} -// -static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) { for (const Scope * scope : symboldatabase->functionScopes) { if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) { @@ -3754,58 +3773,28 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* // Skip known values if (tok->next()->hasKnownValue()) continue; - Token * parenTok = tok->next(); + const Token * parenTok = tok->next(); if (!Token::simpleMatch(parenTok->link(), ") {")) continue; - Token * blockTok = parenTok->link()->tokAt(1); - const Token* condTok = parenTok->astOperand2(); - - Token* startTok = blockTok; - // Inner condition - { - std::vector conds = {condTok}; - if (Token::simpleMatch(condTok, "&&")) { - std::vector args = astFlatten(condTok, "&&"); - conds.insert(conds.end(), args.begin(), args.end()); - } - for(const Token* condTok2:conds) { - ExpressionAnalyzer a1(condTok2, makeConditionValue(1, condTok2, true), tokenlist); - valueFlowGenericForward(startTok, startTok->link(), a1, settings); - - OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(0, condTok2, true), tokenlist); - valueFlowGenericForward(startTok, startTok->link(), a2, settings); - } - } - - std::vector conds = {condTok}; - if (Token::simpleMatch(condTok, "||")) { - std::vector args = astFlatten(condTok, "||"); - conds.insert(conds.end(), args.begin(), args.end()); - } - - // Check else block - if (Token::simpleMatch(startTok->link(), "} else {")) { - startTok = startTok->link()->tokAt(2); - for(const Token* condTok2:conds) { - ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist); - valueFlowGenericForward(startTok, startTok->link(), a1, settings); - - OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist); - valueFlowGenericForward(startTok, startTok->link(), a2, settings); - } - } - + const Token * blockTok = parenTok->link()->tokAt(1); // Check if the block terminates early - if (isEscapeScope(blockTok, tokenlist)) { - for(const Token* condTok2:conds) { - ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist); - valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a1, settings); + if (!isEscapeScope(blockTok, tokenlist)) + continue; - OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist); - valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a2, settings); - } - } + const Token* condTok = parenTok->astOperand2(); + ValueFlow::Value v1(0); + v1.setKnown(); + v1.condition = condTok; + v1.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true"); + ExpressionAnalyzer a1(condTok, v1, tokenlist); + valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a1, settings); + ValueFlow::Value v2(1); + v2.setKnown(); + v2.condition = condTok; + v2.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false"); + OppositeExpressionAnalyzer a2(true, condTok, v2, tokenlist); + valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a2, settings); } } } @@ -6635,7 +6624,6 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings); valueFlowBitAnd(tokenlist); valueFlowSameExpressions(tokenlist); - valueFlowConditionExpressions(tokenlist, symboldatabase, errorLogger, settings); valueFlowFwdAnalysis(tokenlist, settings); std::size_t values = 0; @@ -6645,6 +6633,8 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowPointerAliasDeref(tokenlist); valueFlowArrayBool(tokenlist); valueFlowRightShift(tokenlist, settings); + valueFlowOppositeCondition(symboldatabase, settings); + valueFlowTerminatingCondition(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings); valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings); valueFlowInferCondition(tokenlist, settings); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index e7cde3c7b..d4beb476c 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -533,27 +533,27 @@ private: " if (a) { b = 1; }\n" " else { if (a) { b = 2; } }\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'a' is always false\n", errout.str()); check("void f(int a, int &b) {\n" " if (a) { b = 1; }\n" " else { if (a) { b = 2; } }\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'a' is always false\n", errout.str()); check("void f(int a, int &b) {\n" " if (a == 1) { b = 1; }\n" " else { if (a == 2) { b = 2; }\n" " else { if (a == 1) { b = 3; } } }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'a==1' is always false\n", errout.str()); check("void f(int a, int &b) {\n" " if (a == 1) { b = 1; }\n" " else { if (a == 2) { b = 2; }\n" " else { if (a == 2) { b = 3; } } }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'a==2' is always false\n", errout.str()); check("void f(int a, int &b) {\n" " if (a++) { b = 1; }\n" @@ -721,9 +721,9 @@ private: " if (x) {}\n" " else if (!x) {}\n" "}"); - ASSERT_EQUALS("test.cpp:3:style:Expression is always true because 'else if' condition is opposite to previous condition at line 2.\n" - "test.cpp:2:note:first condition\n" - "test.cpp:3:note:else if condition is opposite to first condition\n", errout.str()); + ASSERT_EQUALS("test.cpp:3:style:Condition '!x' is always true\n" + "test.cpp:2:note:condition 'x'\n" + "test.cpp:3:note:Condition '!x' is always true\n", errout.str()); check("void f(int x) {\n" " int y = x;\n" diff --git a/test/testsimplifytypedef.cpp b/test/testsimplifytypedef.cpp index 8dff6116d..8a1847329 100644 --- a/test/testsimplifytypedef.cpp +++ b/test/testsimplifytypedef.cpp @@ -1670,7 +1670,7 @@ private: "( ( int ( * * ( * ) ( char * , char * , int , int ) ) ( ) ) global [ 6 ] ) ( \"assoc\" , \"eggdrop\" , 106 , 0 ) ; " "}"; ASSERT_EQUALS(expected, tok(code)); - ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1319:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable global\n", errout.str()); + ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1319:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable global\n", errout.str()); } void simplifyTypedef68() { // ticket #2355 @@ -1877,7 +1877,7 @@ private: " B * b = new B;\n" " b->f = new A::F * [ 10 ];\n" "}"); - ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:12]: (debug) valueflow.cpp:1319:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable idx\n", errout.str()); + ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:12]: (debug) valueflow.cpp:1319:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable idx\n", errout.str()); } void simplifyTypedef83() { // ticket #2620 diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index 90a019233..1b57f3ca6 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -2743,7 +2743,7 @@ private: check("::y(){x}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:1]: (debug) Executable scope 'y' with unknown function.\n" - "[test.cpp:1]: (debug) valueflow.cpp:1321:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable x\n", errout.str()); + "[test.cpp:1]: (debug) valueflow.cpp:1321:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable x\n", errout.str()); } void symboldatabase20() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 8415c4382..d67fcf663 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -122,7 +122,7 @@ private: TEST_CASE(valueFlowUninit); - TEST_CASE(valueFlowConditionExpressions); + TEST_CASE(valueFlowTerminatingCond); TEST_CASE(valueFlowContainerSize); @@ -1266,7 +1266,7 @@ private: " if (x == 123) {}\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable y\n", + "[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n", errout.str()); } @@ -1383,7 +1383,7 @@ private: " y = ((x<0) ? x : ((x==2)?3:4));\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable y\n", + "[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n", errout.str()); bailout("int f(int x) {\n" @@ -1448,7 +1448,7 @@ private: " if (x == 123) {}\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable b\n", + "[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable b\n", errout.str()); code = "void f(int x, bool abc) {\n" @@ -1497,7 +1497,7 @@ private: " };\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n", + "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n", errout.str()); bailout("void f(int x, int y) {\n" @@ -1507,7 +1507,7 @@ private: " };\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n", + "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n", errout.str()); } @@ -1519,7 +1519,7 @@ private: " M;\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n" + "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n" "[test.cpp:4]: (debug) valueflow.cpp:1260:(valueFlow) bailout: variable 'x', condition is defined in macro\n", errout.str()); @@ -1529,7 +1529,7 @@ private: " FREE(x);\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n" + "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n" "[test.cpp:4]: (debug) valueflow.cpp:1260:(valueFlow) bailout: variable 'x', condition is defined in macro\n", errout.str()); } @@ -1543,7 +1543,7 @@ private: " if (x==123){}\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n", + "[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n", errout.str()); // #5721 - FP @@ -4202,7 +4202,7 @@ private: ASSERT_EQUALS(0, values.size()); } - void valueFlowConditionExpressions() { + void valueFlowTerminatingCond() { const char* code; // opposite condition @@ -4272,60 +4272,6 @@ private: "}\n"; ASSERT_EQUALS(false, valueOfTok(code, "!=").intvalue == 1); - code = "void f(bool b, int i, int j) {\n" - " if (b || i == j) return;\n" - " if(i != j) {}\n" - "}\n"; - ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1); - - code = "void f(bool b, int i, int j) {\n" - " if (b && i == j) return;\n" - " if(i != j) {}\n" - "}\n"; - ASSERT_EQUALS(true, tokenValues(code, "!=").empty()); - - code = "void f(int i, int j) {\n" - " if (i == j) {\n" - " if (i != j) {}\n" - " }\n" - "}\n"; - ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 0); - - code = "void f(int i, int j) {\n" - " if (i == j) {} else {\n" - " if (i != j) {}\n" - " }\n" - "}\n"; - ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1); - - code = "void f(bool b, int i, int j) {\n" - " if (b && i == j) {\n" - " if (i != j) {}\n" - " }\n" - "}\n"; - ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 0); - - code = "void f(bool b, int i, int j) {\n" - " if (b || i == j) {\n" - " if (i != j) {}\n" - " }\n" - "}\n"; - ASSERT_EQUALS(true, tokenValues(code, "!=").empty()); - - code = "void f(bool b, int i, int j) {\n" - " if (b || i == j) {} else {\n" - " if (i != j) {}\n" - " }\n" - "}\n"; - ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1); - - code = "void f(bool b, int i, int j) {\n" - " if (b && i == j) {} else {\n" - " if (i != j) {}\n" - " }\n" - "}\n"; - ASSERT_EQUALS(true, tokenValues(code, "!=").empty()); - code = "void foo()\n" // #8924 "{\n" " if ( this->FileIndex >= 0 )\n"