Fix #8260 Improve check: Pointer calculation result not null

Signed-off-by: Francois Berder <fberder@outlook.fr>

Author: francois-berder

lib/checkcondition.cpp

@@ -1156,6 +1156,43 @@ static bool isIfConstexpr(const Token* tok) {
     return Token::simpleMatch(top->astOperand1(), "if") && top->astOperand1()->isConstexpr();
 }
 
+static const Token* isPointerArithmeticAdd(const Token* tok)
+{
+    if (!tok || tok->str() != "+" || !astIsPointer(tok))
+        return nullptr;
+
+    const Token* intOp = astIsPointer(tok->astOperand1()) ? tok->astOperand2() : tok->astOperand1();
+    if (intOp && intOp->hasKnownIntValue() && intOp->getKnownIntValue() != 0)
+        return tok;
+
+    return nullptr;
+}
+
+static const Token* getPointerAdditionCalcToken(const Token * const tok)
+{
+    for (const Token *op : {tok, tok->astOperand1(), tok->astOperand2()}) {
+        if (!op)
+            continue;
+
+        // case 1: op is ptr+nonzero
+        if (isPointerArithmeticAdd(op))
+            return op;
+
+        // case 2: op is a pointer variable assigned from ptr+nonzero
+        if (!astIsPointer(op))
+            continue;
+
+        for (const ValueFlow::Value& val : op->values()) {
+            if (!val.isSymbolicValue())
+                continue;
+            if (isPointerArithmeticAdd(val.tokvalue))
+                return op;
+        }
+    }
+
+    return nullptr;
+}
+
 void CheckCondition::checkIncorrectLogicOperator()
 {
     const bool printStyle = mSettings->severity.isEnabled(Severity::style);
@@ -1614,6 +1651,11 @@ void CheckCondition::alwaysTrueFalse()
                     continue;
             }
 
+            // checkPointerAdditionResultNotNull emits a more specific warning for
+            // comparisons where the pointer is the result of an expression ptr+nonzero.
+            if (getPointerAdditionCalcToken(tok))
+                continue;
+
             // Don't warn when there are expanded macros..
             bool isExpandedMacro = false;
             visitAstNodes(tok, [&](const Token * tok2) {
@@ -1785,7 +1827,6 @@ void CheckCondition::invalidTestForOverflow(const Token* tok, const ValueType *v
     reportError(tok, Severity::warning, "invalidTestForOverflow", errmsg, uncheckedErrorConditionCWE, Certainty::normal);
 }
 
-
 void CheckCondition::checkPointerAdditionResultNotNull()
 {
     if (!mSettings->severity.isEnabled(Severity::warning))
@@ -1797,32 +1838,27 @@ void CheckCondition::checkPointerAdditionResultNotNull()
     for (const Scope * scope : symbolDatabase->functionScopes) {
 
         for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
-            if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
-                continue;
-
             // Macros might have pointless safety checks
             if (tok->isExpandedMacro())
                 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
+            const Token *calcToken = getPointerAdditionCalcToken(tok);
+            if (!calcToken)
                 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;
+            if (tok->isComparisonOp() && tok->astOperand1() && tok->astOperand2()) {
+                const Token *exprToken = tok->astOperand1() == calcToken ? tok->astOperand2() : tok->astOperand1();
+
+                if (!exprToken)
+                    continue;
 
-            pointerAdditionResultNotNullError(tok, calcToken);
+                if (!exprToken->hasKnownIntValue() || !exprToken->getValue(0))
+                    continue;
+
+                pointerAdditionResultNotNullError(tok, calcToken);
+            } else if (astIsPointer(tok) && isUsedAsBool(tok, *mSettings) && !tok->astParent()->isComparisonOp()) {
+                pointerAdditionResultNotNullError(tok, calcToken);
+            }
         }
     }
 }

lib/valueflow.cpp

@@ -1174,6 +1174,16 @@ static void valueFlowImpossibleValues(TokenList& tokenList, const Settings& sett
             ValueFlow::Value value{0};
             value.setImpossible();
             setTokenValue(tok, std::move(value), settings);
+        } else if (Token::simpleMatch(tok, "+") && astIsPointer(tok)) {
+            const Token* op1 = tok->astOperand1();
+            const Token* op2 = tok->astOperand2();
+            if ((op1 && op1->hasKnownIntValue() && op1->getKnownIntValue() != 0)
+            || (op2 && op2->hasKnownIntValue() && op2->getKnownIntValue() != 0)) {
+                ValueFlow::Value val(0);
+                val.setImpossible();
+                val.errorPath.emplace_back(tok, "Pointer arithmetic result cannot be null");
+                setTokenValue(tok, std::move(val), settings);
+            }
         }
     }
 }

test/testcondition.cpp

@@ -6234,7 +6234,19 @@ class TestCondition : public TestFixture {
               "  if (ptr + 1 != 0);\n"
               "}");
         ASSERT_EQUALS("[test.cpp:2:15]: (warning) Comparison is wrong. Result of 'ptr+1' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour. [pointerAdditionResultNotNull]\n", errout_str());
-    }
+
+        check("void f(char *ptr) {\n"
+              "  int *q = ptr + 1;\n"
+              "  if (q != 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3:9]: (warning) Comparison is wrong. Result of 'q' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour. [pointerAdditionResultNotNull]\n", errout_str());
+
+        check("void f(char *ptr) {\n"
+              "  int *q = ptr + 1;\n"
+              "  if (q);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3:7]: (warning) Comparison is wrong. Result of 'q' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour. [pointerAdditionResultNotNull]\n", errout_str());
+      }
 
     void duplicateConditionalAssign() {
         setMultiline();