diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 52d039e60..09898acfe 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -523,13 +523,27 @@ void CheckNullPointer::arithmetic() for (std::size_t i = 0; i < functions; ++i) { const Scope * scope = symbolDatabase->functionScopes[i]; for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - if (!tok->astOperand2() || tok->str() != "-") + if (!Token::Match(tok, "-|+|+=|-=|++|--")) continue; - // pointer subtraction - if (!tok->valueType() || !tok->valueType()->pointer) + const Token *pointerOperand; + if (tok->astOperand1() && tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->pointer != 0) + pointerOperand = tok->astOperand1(); + else if (tok->astOperand2() && tok->astOperand2()->valueType() && tok->astOperand2()->valueType()->pointer != 0) + pointerOperand = tok->astOperand2(); + else continue; - // Can LHS be NULL? - const ValueFlow::Value *value = tok->astOperand1()->getValue(0); + MathLib::bigint checkValue = 0; + // When using an assign op, the value read from + // valueflow has already been updated, so instead of + // checking for zero we check that the value is equal + // to RHS + if (tok->astOperand2() && tok->astOperand2()->hasKnownIntValue()) { + if (tok->str() == "-=") + checkValue -= tok->astOperand2()->values().front().intvalue; + else if (tok->str() == "+=") + checkValue = tok->astOperand2()->values().front().intvalue; + } + const ValueFlow::Value *value = pointerOperand->getValue(checkValue); if (!value) continue; if (!_settings->inconclusive && value->isInconclusive()) @@ -544,10 +558,17 @@ void CheckNullPointer::arithmetic() void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value *value) { std::string errmsg; - if (value && value->condition) - errmsg = ValueFlow::eitherTheConditionIsRedundant(value->condition) + " or there is overflow in pointer subtraction."; - else - errmsg = "Overflow in pointer arithmetic, NULL pointer is subtracted."; + if (tok && tok->str().front() == '-') { + if (value && value->condition) + errmsg = ValueFlow::eitherTheConditionIsRedundant(value->condition) + " or there is overflow in pointer subtraction."; + else + errmsg = "Overflow in pointer arithmetic, NULL pointer is subtracted."; + } else { + if (value && value->condition) + errmsg = ValueFlow::eitherTheConditionIsRedundant(value->condition) + " or there is pointer arithmetic with NULL pointer."; + else + errmsg = "Pointer arithmetic with NULL pointer."; + } std::list callstack; callstack.push_back(tok); diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 4eb269f83..7424ffd8e 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -105,6 +105,7 @@ private: TEST_CASE(nullpointer_internal_error); // #5080 TEST_CASE(ticket6505); TEST_CASE(subtract); + TEST_CASE(addNull); } void check(const char code[], bool inconclusive = false, const char filename[] = "test.cpp") { @@ -2571,6 +2572,68 @@ private: " p = s - 20;\n" "}\n"); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is overflow in pointer subtraction.\n", errout.str()); + + check("void foo(char *s) {\n" + " s -= 20;\n" + "}\n" + "void bar() { foo(0); }\n"); + ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", errout.str()); + + check("void foo(char *s) {\n" + " if (!s) {}\n" + " s -= 20;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is overflow in pointer subtraction.\n", errout.str()); + + check("int* f8() { int *x = NULL; return --x; }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", errout.str()); + + check("int* f9() { int *x = NULL; return x--; }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", errout.str()); + } + + void addNull() { + check("void foo(char *s) {\n" + " char * p = s + 20;\n" + "}\n" + "void bar() { foo(0); }\n"); + ASSERT_EQUALS("[test.cpp:2]: (error) Pointer arithmetic with NULL pointer.\n", errout.str()); + + check("void foo(char *s) {\n" + " if (!s) {}\n" + " char * p = s + 20;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is pointer arithmetic with NULL pointer.\n", errout.str()); + + check("void foo(char *s) {\n" + " char * p = 20 + s;\n" + "}\n" + "void bar() { foo(0); }\n"); + ASSERT_EQUALS("[test.cpp:2]: (error) Pointer arithmetic with NULL pointer.\n", errout.str()); + + check("void foo(char *s) {\n" + " if (!s) {}\n" + " char * p = 20 + s;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is pointer arithmetic with NULL pointer.\n", errout.str()); + + check("void foo(char *s) {\n" + " s += 20;\n" + "}\n" + "void bar() { foo(0); }\n"); + ASSERT_EQUALS("[test.cpp:2]: (error) Pointer arithmetic with NULL pointer.\n", errout.str()); + + check("void foo(char *s) {\n" + " if (!s) {}\n" + " s += 20;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is pointer arithmetic with NULL pointer.\n", errout.str()); + + check("int* f7() { int *x = NULL; return ++x; }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Pointer arithmetic with NULL pointer.\n", errout.str()); + + check("int* f10() { int *x = NULL; return x++; } "); + ASSERT_EQUALS("[test.cpp:1]: (error) Pointer arithmetic with NULL pointer.\n", errout.str()); } };