From 28cfee2d4f813fbcbf20d9214428fb93b10b4958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 22 Oct 2017 14:32:54 +0200 Subject: [PATCH] Fixed #8250 (New check: Pointer calculation result cant be NULL unless there is overflow) --- lib/checkcondition.cpp | 46 +++++++++++++++++++++++++++++++++++++++++- lib/checkcondition.h | 8 +++++++- test/testcondition.cpp | 18 ++++++++++++----- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index fb32e5240..dd21fa328 100644 --- 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 index 99e7f88e1..44aff05d3 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -59,6 +59,7 @@ public: checkCondition.checkIncorrectLogicOperator(); checkCondition.checkInvalidTestForOverflow(); checkCondition.alwaysTrueFalse(); + checkCondition.checkPointerAdditionResultNotNull(); } /** @brief Run checks against the simplified token list */ @@ -113,6 +114,9 @@ public: /** @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 &vars) const; bool isOverlappingCond(const Token * const cond1, const Token * const cond2, bool pure) const; @@ -141,6 +145,7 @@ private: 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 @@ private: 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 @@ private: "- 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 index 889cdd6c9..836bbd351 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -98,6 +98,7 @@ private: 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 @@ private: 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);\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 don't warn " assert(x + 100U < x);\n" @@ -2266,6 +2267,13 @@ private: "}"); 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)