Fix false positives in knownConditionTrueFalse when using expressions with const variables (#2469)

pfultz2 · danmar · commit dd05839a7e63 · 2020-01-03T19:35:28.000+01:00
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -1411,27 +1411,31 @@ bool isCPPCast(const Token* tok)
     return tok && Token::simpleMatch(tok->previous(), "> (") && tok->astOperand2() && tok->astOperand1() && tok->astOperand1()->str().find("_cast") != std::string::npos;
 }
 
-bool isConstVarExpression(const Token *tok)
+bool isConstVarExpression(const Token *tok, const char* skipMatch)
 {
     if (!tok)
         return false;
+    if (skipMatch && Token::Match(tok, skipMatch))
+        return false;
     if (Token::simpleMatch(tok->previous(), "sizeof ("))
         return true;
     if (Token::Match(tok->previous(), "%name% (")) {
+        if (Token::simpleMatch(tok->astOperand1(), ".") && !isConstVarExpression(tok->astOperand1(), skipMatch))
+            return false;
         std::vector<const Token *> args = getArguments(tok);
-        return std::all_of(args.begin(), args.end(), &isConstVarExpression);
+        return std::all_of(args.begin(), args.end(), [&](const Token* t) { return isConstVarExpression(t, skipMatch); });
     }
     if (isCPPCast(tok)) {
-        return isConstVarExpression(tok->astOperand2());
+        return isConstVarExpression(tok->astOperand2(), skipMatch);
     }
     if (Token::Match(tok, "( %type%"))
-        return isConstVarExpression(tok->astOperand1());
+        return isConstVarExpression(tok->astOperand1(), skipMatch);
     if (tok->str() == "::" && tok->hasKnownValue())
-        return isConstVarExpression(tok->astOperand2());
+        return isConstVarExpression(tok->astOperand2(), skipMatch);
     if (Token::Match(tok, "%cop%|[|.")) {
-        if (tok->astOperand1() && !isConstVarExpression(tok->astOperand1()))
+        if (tok->astOperand1() && !isConstVarExpression(tok->astOperand1(), skipMatch))
             return false;
-        if (tok->astOperand2() && !isConstVarExpression(tok->astOperand2()))
+        if (tok->astOperand2() && !isConstVarExpression(tok->astOperand2(), skipMatch))
             return false;
         return true;
     }
@@ -1440,7 +1444,7 @@ bool isConstVarExpression(const Token *tok)
     if (tok->isEnumerator())
         return true;
     if (tok->variable())
-        return tok->variable()->isConst();
+        return tok->variable()->isConst() && tok->variable()->nameToken() && tok->variable()->nameToken()->hasKnownValue();
     return false;
 }
 
diff --git a/lib/astutils.h b/lib/astutils.h
@@ -205,7 +205,7 @@ bool isLikelyStreamRead(bool cpp, const Token *op);
 
 bool isCPPCast(const Token* tok);
 
-bool isConstVarExpression(const Token *tok);
+bool isConstVarExpression(const Token *tok, const char * skipMatch = nullptr);
 
 const Variable *getLHSVariable(const Token *tok);
 
diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
@@ -1378,6 +1378,8 @@ void CheckCondition::alwaysTrueFalse()
                 continue;
             if (Token::Match(tok, "%comp%") && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, true))
                 continue;
+            if (isConstVarExpression(tok, "[|(|&|+|-|*|/|%|^|>>|<<"))
+                continue;
 
             const bool constIfWhileExpression =
                 tok->astParent() && Token::Match(tok->astTop()->astOperand1(), "if|while") && !tok->astTop()->astOperand1()->isConstexpr() &&
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -3000,6 +3000,9 @@ static bool isVariableExpression(const Token* tok)
     if (Token::simpleMatch(tok, "."))
         return isVariableExpression(tok->astOperand1()) &&
                isVariableExpression(tok->astOperand2());
+    if (Token::simpleMatch(tok, "["))
+        return isVariableExpression(tok->astOperand1()) &&
+               tok->astOperand2() && tok->astOperand2()->hasKnownIntValue();
     return false;
 }
 
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -3329,6 +3329,24 @@ class TestCondition : public TestFixture {
               "        return;\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("int f(int a, int b) {\n"
+              "    static const int x = 10;\n"
+              "    return x == 1 ? a : b;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("const bool x = false;\n"
+              "void f() {\n"
+              "    if (x) {}\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("const bool x = false;\n"
+              "void f() {\n"
+              "    if (!x) {}\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void alwaysTrueInfer() {

Original file line number	Diff line number	Diff line change
`@@ -3000,6 +3000,9 @@ static bool isVariableExpression(const Token* tok)`
`3000`	`3000`	`if (Token::simpleMatch(tok, "."))`
`3001`	`3001`	`return isVariableExpression(tok->astOperand1()) &&`
`3002`	`3002`	`isVariableExpression(tok->astOperand2());`
	`3003`	`+ if (Token::simpleMatch(tok, "["))`
	`3004`	`+ return isVariableExpression(tok->astOperand1()) &&`
	`3005`	`+ tok->astOperand2() && tok->astOperand2()->hasKnownIntValue();`
`3003`	`3006`	`return false;`
`3004`	`3007`	`}`
`3005`	`3008`