diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 14fa900944d..a097d1cfe23 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1144,6 +1144,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); @@ -1607,6 +1644,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) { @@ -1778,7 +1820,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)) @@ -1790,32 +1831,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); + } } } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f7e057da58f..607268e9616 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1179,6 +1179,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); + } } } } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 241d0400099..c941124ec98 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -6129,6 +6129,18 @@ 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() {