Fixed #7184 (Invalid test for overflow 'p + x < p')

lib/checkother.cpp

@@ -2455,3 +2455,59 @@ void CheckOther::unusedLabelError(const Token* tok)
     reportError(tok, Severity::style, "unusedLabel",
                 "Label '" + (tok?tok->str():emptyString) + "' is not used.");
 }
+
+void CheckOther::checkInvalidTestForOverflow()
+{
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+
+        for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
+            if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
+                continue;
+
+            const Token *calcToken, *exprToken;
+            if (Token::Match(tok, "<|<=") && tok->astOperand1()->str() == "+") {
+                calcToken = tok->astOperand1();
+                exprToken = tok->astOperand2();
+            } else if (Token::Match(tok, ">|>=") && tok->astOperand2()->str() == "+") {
+                calcToken = tok->astOperand2();
+                exprToken = tok->astOperand1();
+            } else
+                continue;
+
+            // Only warn for signed integer overflows and pointer overflows.
+            if (!(calcToken->valueType() && (calcToken->valueType()->pointer || calcToken->valueType()->sign == ValueType::Sign::SIGNED)))
+                continue;
+            if (!(exprToken->valueType() && (exprToken->valueType()->pointer || exprToken->valueType()->sign == ValueType::Sign::SIGNED)))
+                continue;
+
+            const Token *termToken;
+            if (isSameExpression(_tokenizer->isCPP(), exprToken, calcToken->astOperand1(), _settings->library.functionpure))
+                termToken = calcToken->astOperand2();
+            else if (isSameExpression(_tokenizer->isCPP(), exprToken, calcToken->astOperand2(), _settings->library.functionpure))
+                termToken = calcToken->astOperand1();
+            else
+                continue;
+
+            if (!termToken)
+                continue;
+
+            // Only warn when termToken is always positive
+            if (termToken->valueType() && termToken->valueType()->sign == ValueType::Sign::UNSIGNED)
+                invalidTestForOverflow(tok);
+            else if (termToken->isNumber() && MathLib::isPositive(termToken->str()))
+                invalidTestForOverflow(tok);
+        }
+    }
+}
+
+void CheckOther::invalidTestForOverflow(const Token* tok)
+{
+    std::string errmsg;
+    errmsg = "Invalid test for overflow '" +
+             (tok ? tok->expressionString() : std::string("x + u < x")) +
+             "'. Condition is always false unless there is overflow, and overflow is UB.";
+    reportError(tok, Severity::warning, "invalidTestForOverflow", errmsg);
+}

lib/checkother.h

@@ -70,6 +70,7 @@ public:
         checkOther.checkZeroDivision();
         checkOther.checkInterlockedDecrement();
         checkOther.checkUnusedLabel();
+        checkOther.checkInvalidTestForOverflow();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -203,6 +204,9 @@ public:
     /** @brief %Check for unused labels */
     void checkUnusedLabel();
 
+    /** @brief %Check for invalid test for overflow 'x+100 < x' */
+    void checkInvalidTestForOverflow();
+
 private:
     // Error messages..
     void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result);
@@ -251,6 +255,7 @@ private:
     void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive);
     void raceAfterInterlockedDecrementError(const Token* tok);
     void unusedLabelError(const Token* tok);
+    void invalidTestForOverflow(const Token* tok);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckOther c(0, settings, errorLogger);
@@ -305,6 +310,7 @@ private:
         c.commaSeparatedReturnError(0);
         c.redundantPointerOpError(0, "varname", false);
         c.unusedLabelError(0);
+        c.invalidTestForOverflow(0);
     }
 
     static std::string myName() {
@@ -327,6 +333,7 @@ private:
                // warning
                "- either division by zero or useless condition\n"
                "- memset() with a value out of range as the 2nd parameter\n"
+               "- invalid test for overflow\n"
 
                // performance
                "- redundant data copying for const variable\n"

test/testother.cpp

@@ -157,6 +157,8 @@ private:
         TEST_CASE(raceAfterInterlockedDecrement);
 
         TEST_CASE(testUnusedLabel);
+
+        TEST_CASE(checkInvalidTestForOverflow);
     }
 
     void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) {
@@ -6017,6 +6019,23 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void checkInvalidTestForOverflow() {
+        check("void f(char *p, unsigned int x) {\n"
+              "    assert((p + x) < p);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)<p'. Condition is always false unless there is overflow, and overflow is UB.\n", errout.str());
+
+        check("void f(signed int x) {\n"
+              "    assert(x + 100 < x);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'x+100<x'. Condition is always false unless there is overflow, and overflow is UB.\n", errout.str());
+
+        check("void f(signed int x) {\n" // unsigned overflow => dont warn
+              "    assert(x + 100U < x);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)