Fixed #9363 (knownConditionTrueFalse: False positive about function parameter)

lib/checkcondition.cpp

@@ -1333,11 +1333,28 @@ void CheckCondition::alwaysTrueFalse()
     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
     for (const Scope * scope : symbolDatabase->functionScopes) {
         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
-
             if (tok->link()) // don't write false positives when templates are used
                 continue;
             if (!tok->hasKnownIntValue())
                 continue;
+            {
+                // is this a condition..
+                const Token *parent = tok->astParent();
+                while (Token::Match(parent, "%oror%|&&"))
+                    parent = parent->astParent();
+                if (!parent)
+                    continue;
+                const Token *condition = nullptr;
+                if (parent->str() == "?" && precedes(tok, parent))
+                    condition = parent->astOperand1();
+                else if (Token::Match(parent->previous(), "if|while ("))
+                    condition = parent->astOperand2();
+                else if (parent->str() == ";" && parent->astParent() && parent->astParent()->astParent() && Token::simpleMatch(parent->astParent()->astParent()->previous(), "for ("))
+                    condition = parent->astOperand1();
+                else
+                    continue;
+                (void)condition;
+            }
             // Skip already diagnosed values
             if (diag(tok, false))
                 continue;
@@ -1355,38 +1372,9 @@ void CheckCondition::alwaysTrueFalse()
                 (Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->astParent()->astOperand1(), "if|while"));
             const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
             const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
-            const bool returnStatement = Token::simpleMatch(tok->astTop(), "return") &&
-                                         Token::Match(tok->astParent(), "%oror%|&&|return");
             const bool ternaryExpression = Token::simpleMatch(tok->astParent(), "?");
 
-            if (!(constIfWhileExpression || constValExpr || compExpr || returnStatement || ternaryExpression))
+            if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression))
-                continue;
-
-            if (returnStatement && (!scope->function || !Token::simpleMatch(scope->function->retDef, "bool")))
-                continue;
-
-            if (returnStatement && isConstVarExpression(tok))
-                continue;
-
-            if (returnStatement && Token::simpleMatch(tok->astParent(), "return") && tok->variable() && (
-                    !tok->variable()->isLocal() ||
-                    tok->variable()->isReference() ||
-                    tok->variable()->isConst() ||
-                    !isVariableChanged(tok->variable(), mSettings, mTokenizer->isCPP())))
-                continue;
-
-            // Don't warn in assertions. Condition is often 'always true' by intention.
-            // If platform,defines,etc cause 'always false' then that is not dangerous neither.
-            bool assertFound = false;
-            for (const Token * tok2 = tok->astParent(); tok2 ; tok2 = tok2->astParent()) { // move backwards and try to find "assert"
-                if (tok2->str() == "(" && tok2->astOperand2()) {
-                    const std::string& str = tok2->previous()->str();
-                    if ((str.find("assert")!=std::string::npos || str.find("ASSERT")!=std::string::npos))
-                        assertFound = true;
-                    break;
-                }
-            }
-            if (assertFound)
                 continue;
 
             // Don't warn when there are expanded macros..
@@ -1446,15 +1434,15 @@ void CheckCondition::alwaysTrueFalse()
 
 void CheckCondition::alwaysTrueFalseError(const Token *tok, const ValueFlow::Value *value)
 {
-    const bool condvalue = value && (value->intvalue != 0);
+    const bool alwaysTrue = value && (value->intvalue != 0);
     const std::string expr = tok ? tok->expressionString() : std::string("x");
-    const std::string errmsg = "Condition '" + expr + "' is always " + (condvalue ? "true" : "false");
+    const std::string errmsg = "Condition '" + expr + "' is always " + (alwaysTrue ? "true" : "false");
     const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
     reportError(errorPath,
                 Severity::style,
                 "knownConditionTrueFalse",
                 errmsg,
-                (condvalue ? CWE571 : CWE570), false);
+                (alwaysTrue ? CWE571 : CWE570), false);
 }
 
 void CheckCondition::checkInvalidTestForOverflow()

test/testcondition.cpp

@@ -2681,8 +2681,7 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n"
                       "[test.cpp:2]: (style) Boolean result is used in bitwise operation. Clarify expression with parentheses.\n"
-                      "[test.cpp:2]: (style) Condition 'x&3==2' is always false\n"
+                      "[test.cpp:2]: (style) Condition 'x&3==2' is always false\n", errout.str());
-                      "[test.cpp:2]: (style) Condition '3==2' is always false\n", errout.str());
 
         check("void f() {\n"
               "    if (a & fred1.x == fred2.y) {}\n"
@@ -2834,21 +2833,6 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:4]: (style) Condition '!x' is always true\n", errout.str());
 
-        check("bool f(int x) {\n"
-              "  if(x == 0) { x++; return x == 0; } \n"
-              "  return false;\n"
-              "}");
-        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'x==0' is always false\n", errout.str());
-
-        check("void f() {\n" // #6898 (Token::expressionString)
-              "  int x = 0;\n"
-              "  A(x++ == 1);\n"
-              "  A(x++ == 2);\n"
-              "}");
-        ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'x++==1' is always false\n"
-                      "[test.cpp:4]: (style) Condition 'x++==2' is always false\n",
-                      errout.str());
-
         check("void f1(const std::string &s) { if(s.empty()) if(s.size() == 0) {}} ");
         ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Condition 's.size()==0' is always true\n", errout.str());
 
@@ -2953,6 +2937,15 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        // #9363 - do not warn about value passed to function
+        check("void f(bool b) {\n"
+              "    if (b) {\n"
+              "        if (bar(!b)) {}\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+
         // #7783  FP knownConditionTrueFalse on assert(0 && "message")
         check("void foo(int x) {\n"
               "    if (x<0)\n"