Fix issue 9721: ValueFlow: Comparison is always false, but ValueFlow says it is always true (cppcheck-opensource#2658)

pfultz2 · web-flow · commit 0832830a959b · 2020-05-23T07:15:13.000+02:00
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
@@ -4321,9 +4321,19 @@ static void valueFlowInferCondition(TokenList* tokenlist,
         } else if (tok->isComparisonOp()) {
             MathLib::bigint val = 0;
             const Token* varTok = nullptr;
+            std::string op = tok->str();
             if (tok->astOperand1()->hasKnownIntValue()) {
                 val = tok->astOperand1()->values().front().intvalue;
                 varTok = tok->astOperand2();
+                // Flip the operator
+                if (op == ">")
+                    op = "<";
+                else if (op == "<")
+                    op = ">";
+                else if (op == ">=")
+                    op = "<=";
+                else if (op == "<=")
+                    op = ">=";
             } else if (tok->astOperand2()->hasKnownIntValue()) {
                 val = tok->astOperand2()->values().front().intvalue;
                 varTok = tok->astOperand1();
@@ -4336,22 +4346,22 @@ static void valueFlowInferCondition(TokenList* tokenlist,
                 continue;
             const ValueFlow::Value* result = nullptr;
             bool known = false;
-            if (Token::Match(tok, "==|!=")) {
+            if (op == "==" || op == "!=") {
                 result = proveNotEqual(varTok->values(), val);
-                known = tok->str() == "!=";
-            } else if (Token::Match(tok, "<|>=")) {
+                known = op == "!=";
+            } else if (op == "<" || op == ">=") {
                 result = proveLessThan(varTok->values(), val);
-                known = tok->str() == "<";
+                known = op == "<";
                 if (!result && !isSaturated(val)) {
                     result = proveGreaterThan(varTok->values(), val - 1);
-                    known = tok->str() == ">=";
+                    known = op == ">=";
                 }
-            } else if (Token::Match(tok, ">|<=")) {
+            } else if (op == ">" || op == "<=") {
                 result = proveGreaterThan(varTok->values(), val);
-                known = tok->str() == ">";
+                known = op == ">";
                 if (!result && !isSaturated(val)) {
                     result = proveLessThan(varTok->values(), val + 1);
-                    known = tok->str() == "<=";
+                    known = op == "<=";
                 }
             }
             if (!result)
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -3507,6 +3507,15 @@ class TestCondition : public TestFixture {
               "    return pos;\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (style) Condition 'pos>0' is always true\n", errout.str());
+
+        // #9721
+        check("void f(int x) {\n"
+              "    if (x > 127) {\n"
+              "        if ( (x>255) || (-128>x) )\n"
+              "            return;\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition '-128>x' is always false\n", errout.str());
     }
 
     void alwaysTrueContainer() {
