Fixed #8250 (New check: Pointer calculation result cant be NULL unless there is overflow)

danmar · danmar · commit 28cfee2d4f81 · 2017-10-22T14:32:54.000+02:00
diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
@@ -1338,6 +1338,50 @@ void CheckCondition::invalidTestForOverflow(const Token* tok, bool result)
              (tok ? tok->expressionString() : std::string("x + u < x")) +
              "'. Condition is always " +
              std::string(result ? "true" : "false") +
-             " unless there is overflow, and overflow is UB.";
+             " unless there is overflow, and overflow is undefined behaviour.";
     reportError(tok, Severity::warning, "invalidTestForOverflow", errmsg, (result ? CWE571 : CWE570), false);
 }
+
+
+void CheckCondition::checkPointerAdditionResultNotNull()
+{
+    if (!_settings->isEnabled(Settings::WARNING))
+        return;
+
+    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 (tok->astOperand1()->str() == "+") {
+                calcToken = tok->astOperand1();
+                exprToken = tok->astOperand2();
+            } else if (tok->astOperand2()->str() == "+") {
+                calcToken = tok->astOperand2();
+                exprToken = tok->astOperand1();
+            } else
+                continue;
+
+            // pointer comparison against NULL (ptr+12==0)
+            if (calcToken->hasKnownIntValue())
+                continue;
+            if (!calcToken->valueType() || calcToken->valueType()->pointer==0)
+                continue;
+            if (!exprToken->hasKnownIntValue() || !exprToken->getValue(0))
+                continue;
+
+            pointerAdditionResultNotNullError(tok, calcToken);
+        }
+    }
+}
+
+void CheckCondition::pointerAdditionResultNotNullError(const Token *tok, const Token *calc)
+{
+    const std::string s = calc ? calc->expressionString() : "ptr+1";
+    reportError(tok, Severity::warning, "pointerAdditionResultNotNull", "Comparison is wrong. Result of '" + s + "' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour.");
+}
diff --git a/lib/checkcondition.h b/lib/checkcondition.h
@@ -59,6 +59,7 @@ class CPPCHECKLIB CheckCondition : public Check {
         checkCondition.checkIncorrectLogicOperator();
         checkCondition.checkInvalidTestForOverflow();
         checkCondition.alwaysTrueFalse();
+        checkCondition.checkPointerAdditionResultNotNull();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -113,6 +114,9 @@ class CPPCHECKLIB CheckCondition : public Check {
     /** @brief %Check for invalid test for overflow 'x+100 < x' */
     void checkInvalidTestForOverflow();
 
+    /** @brief Check if pointer addition result is NULL '(ptr + 1) == NULL' */
+    void checkPointerAdditionResultNotNull();
+
 private:
     bool isAliased(const std::set<unsigned int> &vars) const;
     bool isOverlappingCond(const Token * const cond1, const Token * const cond2, bool pure) const;
@@ -141,6 +145,7 @@ class CPPCHECKLIB CheckCondition : public Check {
     void alwaysTrueFalseError(const Token *tok, const ValueFlow::Value *value);
 
     void invalidTestForOverflow(const Token* tok, bool result);
+    void pointerAdditionResultNotNullError(const Token *tok, const Token *calc);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckCondition c(nullptr, settings, errorLogger);
@@ -158,6 +163,7 @@ class CPPCHECKLIB CheckCondition : public Check {
         c.clarifyConditionError(nullptr, true, false);
         c.alwaysTrueFalseError(nullptr, nullptr);
         c.invalidTestForOverflow(nullptr, false);
+        c.pointerAdditionResultNotNullError(nullptr, nullptr);
     }
 
     static std::string myName() {
@@ -177,7 +183,7 @@ class CPPCHECKLIB CheckCondition : public Check {
                "- Mutual exclusion over || always evaluating to true\n"
                "- Comparisons of modulo results that are always true/false.\n"
                "- Known variable values => condition is always true/false\n"
-               "- Invalid test for overflow (for example 'ptr+u < ptr'). Condition is always false unless there is overflow, and overflow is UB.\n";
+               "- Invalid test for overflow (for example 'ptr+u < ptr'). Condition is always false unless there is overflow, and overflow is undefined behaviour.\n";
     }
 };
 /// @}
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -98,6 +98,7 @@ class TestCondition : public TestFixture {
 
         TEST_CASE(checkInvalidTestForOverflow);
         TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
+        TEST_CASE(pointerAdditionResultNotNull);
     }
 
     void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) {
@@ -2212,27 +2213,27 @@ class TestCondition : public TestFixture {
         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());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)<p'. Condition is always false unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
 
         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 true unless there is overflow, and overflow is UB.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)>=p'. Condition is always true unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
 
         check("void f(char *p, unsigned int x) {\n"
               "    assert(p > (p + x));\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p>(p+x)'. Condition is always false unless there is overflow, and overflow is UB.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p>(p+x)'. Condition is always false unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
 
         check("void f(char *p, unsigned int x) {\n"
               "    assert(p <= (p + x));\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p<=(p+x)'. Condition is always true unless there is overflow, and overflow is UB.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p<=(p+x)'. Condition is always true unless there is overflow, and overflow is undefined behaviour.\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());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'x+100<x'. Condition is always false unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
 
         check("void f(signed int x) {\n" // unsigned overflow => don't warn
               "    assert(x + 100U < x);\n"
@@ -2266,6 +2267,13 @@ class TestCondition : public TestFixture {
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str());
     }
+
+    void pointerAdditionResultNotNull() {
+        check("void f(char *ptr) {\n"
+              "  if (ptr + 1 != 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison is wrong. Result of 'ptr+1' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestCondition)
