From 26a07265a85b7108e0f45a6d04ff6bc3a1a57ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 29 Nov 2015 20:59:50 +0100 Subject: [PATCH] Fixed #7184 (Invalid test for overflow 'p + x < p') --- lib/checkother.cpp | 56 ++++++++++++++++++++++++++++++++++++++++++++++ lib/checkother.h | 7 ++++++ test/testother.cpp | 19 ++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 0ceec88ab..6c58130a0 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2455,3 +2455,59 @@ void CheckOther::unusedLabelError(const Token* tok) reportError(tok, Severity::style, "unusedLabel", "Label '" + (tok?tok->str():emptyString) + "' is not used."); } + +void CheckOther::checkInvalidTestForOverflow() +{ + 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 (Token::Match(tok, "<|<=") && tok->astOperand1()->str() == "+") { + calcToken = tok->astOperand1(); + exprToken = tok->astOperand2(); + } else if (Token::Match(tok, ">|>=") && tok->astOperand2()->str() == "+") { + calcToken = tok->astOperand2(); + exprToken = tok->astOperand1(); + } else + continue; + + // Only warn for signed integer overflows and pointer overflows. + if (!(calcToken->valueType() && (calcToken->valueType()->pointer || calcToken->valueType()->sign == ValueType::Sign::SIGNED))) + continue; + if (!(exprToken->valueType() && (exprToken->valueType()->pointer || exprToken->valueType()->sign == ValueType::Sign::SIGNED))) + continue; + + const Token *termToken; + if (isSameExpression(_tokenizer->isCPP(), exprToken, calcToken->astOperand1(), _settings->library.functionpure)) + termToken = calcToken->astOperand2(); + else if (isSameExpression(_tokenizer->isCPP(), exprToken, calcToken->astOperand2(), _settings->library.functionpure)) + termToken = calcToken->astOperand1(); + else + continue; + + if (!termToken) + continue; + + // Only warn when termToken is always positive + if (termToken->valueType() && termToken->valueType()->sign == ValueType::Sign::UNSIGNED) + invalidTestForOverflow(tok); + else if (termToken->isNumber() && MathLib::isPositive(termToken->str())) + invalidTestForOverflow(tok); + } + } +} + +void CheckOther::invalidTestForOverflow(const Token* tok) +{ + std::string errmsg; + errmsg = "Invalid test for overflow '" + + (tok ? tok->expressionString() : std::string("x + u < x")) + + "'. Condition is always false unless there is overflow, and overflow is UB."; + reportError(tok, Severity::warning, "invalidTestForOverflow", errmsg); +} diff --git a/lib/checkother.h b/lib/checkother.h index 731147172..1ccff97ec 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -70,6 +70,7 @@ public: checkOther.checkZeroDivision(); checkOther.checkInterlockedDecrement(); checkOther.checkUnusedLabel(); + checkOther.checkInvalidTestForOverflow(); } /** @brief Run checks against the simplified token list */ @@ -203,6 +204,9 @@ public: /** @brief %Check for unused labels */ void checkUnusedLabel(); + /** @brief %Check for invalid test for overflow 'x+100 < x' */ + void checkInvalidTestForOverflow(); + private: // Error messages.. void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result); @@ -251,6 +255,7 @@ private: void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive); void raceAfterInterlockedDecrementError(const Token* tok); void unusedLabelError(const Token* tok); + void invalidTestForOverflow(const Token* tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckOther c(0, settings, errorLogger); @@ -305,6 +310,7 @@ private: c.commaSeparatedReturnError(0); c.redundantPointerOpError(0, "varname", false); c.unusedLabelError(0); + c.invalidTestForOverflow(0); } static std::string myName() { @@ -327,6 +333,7 @@ private: // warning "- either division by zero or useless condition\n" "- memset() with a value out of range as the 2nd parameter\n" + "- invalid test for overflow\n" // performance "- redundant data copying for const variable\n" diff --git a/test/testother.cpp b/test/testother.cpp index b7b18964d..b0f6dcd10 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -157,6 +157,8 @@ private: TEST_CASE(raceAfterInterlockedDecrement); TEST_CASE(testUnusedLabel); + + TEST_CASE(checkInvalidTestForOverflow); } void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) { @@ -6017,6 +6019,23 @@ private: "}"); ASSERT_EQUALS("", errout.str()); } + + void checkInvalidTestForOverflow() { + 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) dont warn + " assert(x + 100U < x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestOther)