From 9cbfa0e383e4084cc6feafcd159a761e0ca0f2f5 Mon Sep 17 00:00:00 2001
From: Nekto89 <nekto1989@gmail.com>
Date: Fri, 8 Sep 2017 15:30:42 +0300
Subject: [PATCH] Fix #7803: false negative: condition is always true 'if
 (flags & A)' (#938)

---
 lib/checkcondition.cpp | 16 ++++++++++------
 test/testcondition.cpp | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
index 92c5ee8db..4e9ddbc28 100644
--- a/lib/checkcondition.cpp
+++ b/lib/checkcondition.cpp
@@ -1151,12 +1151,6 @@ void CheckCondition::alwaysTrueFalse()
         const Scope * scope = symbolDatabase->functionScopes[i];
         for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
 
-            const bool constValCond = Token::Match(tok->tokAt(-2), "if|while ( %num%|%char% )") && !Token::Match(tok,"0|1"); // just one number or char inside if|while
-            const bool constValExpr = Token::Match(tok, "%num%|%char%") && tok->astParent() && Token::Match(tok->astParent(),"&&|%oror%|?"); // just one number or char in boolean expression
-            const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
-
-            if (!(constValCond || constValExpr || compExpr))
-                continue;
             if (tok->link()) // don't write false positives when templates are used
                 continue;
             if (!tok->hasKnownIntValue())
@@ -1164,6 +1158,16 @@ void CheckCondition::alwaysTrueFalse()
             if (Token::Match(tok, "[01]"))
                 continue;
 
+            const bool constIfWhileExpression =
+                tok->astParent()
+                && Token::Match(tok->astParent()->astOperand1(), "if|while")
+                && !tok->isBoolean();
+            const bool constValExpr = Token::Match(tok, "%num%|%char%") && tok->astParent() && Token::Match(tok->astParent(),"&&|%oror%|?"); // just one number or char in boolean expression
+            const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
+            
+            if (!(constIfWhileExpression || constValExpr || compExpr))
+                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;
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index 7625ac3a3..ef78e3df7 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -97,6 +97,7 @@ private:
         TEST_CASE(alwaysTrue);
 
         TEST_CASE(checkInvalidTestForOverflow);
+        TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
     }
 
     void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) {
@@ -355,7 +356,7 @@ private:
               "        g(x);\n"
               "    }\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'x' is always true\n", errout.str());
 
         check("void g(int & x);\n"
               "void f() {\n"
@@ -1935,6 +1936,7 @@ private:
               "    if (x & 3 == 2) {}\n"
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n"
+                      "[test.cpp:2]: (style) Condition 'x&3==2' is always false\n"
                       "[test.cpp:2]: (style) Condition '3==2' is always false\n", errout.str());
 
         check("void f() {\n"
@@ -2153,6 +2155,7 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (style) Condition ''a'' is always true\n"
                       "[test.cpp:3]: (style) Condition 'L'b'' is always true\n"
+                      "[test.cpp:4]: (style) Condition '1&&'c'' is always true\n"
                       "[test.cpp:4]: (style) Condition ''c'' is always true\n"
                       "[test.cpp:5]: (style) Condition ''d'' is always true\n", errout.str());
     }
@@ -2188,6 +2191,33 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void checkConditionIsAlwaysTrueOrFalseInsideIfWhile() {
+        check("void f() {\n"
+              "    enum states {A,B,C};\n"
+              "    const unsigned g_flags = B|C;\n"
+              "    if(g_flags & A) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'g_flags&A' is always false\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 5;"
+              "    if(a) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a' is always true\n", errout.str());
+
+        check("void f() {\n"
+             "    int a = 5;"
+             "    while(a + 1) { a--; }\n"
+             "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    int a = 5;"
+              "    while(a + 1) { return; }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestCondition)