From c0b33d2fef310c0315050cd45cf3b72703061983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 25 May 2015 10:02:17 +0200 Subject: [PATCH] Fixed #6707 (new check: possible truncation when assigning int result to long) --- lib/checktype.cpp | 87 +++++++++++++++++++++++++++++++++++++++++++++++ lib/checktype.h | 11 +++++- test/testtype.cpp | 30 ++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/lib/checktype.cpp b/lib/checktype.cpp index 9ad5af416..3e5515097 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -30,6 +30,19 @@ namespace { CheckType instance; } +static bool astIsIntResult(const Token *tok) +{ + if (!tok) + return false; + if (tok->astOperand1()) + return astIsIntResult(tok->astOperand1()) && astIsIntResult(tok->astOperand2()); + // todo: handle numbers + if (!tok->variable()) + return false; + const Token *type = tok->variable()->typeStartToken(); + return Token::Match(type, "char|short|int") && !type->isLong(); +} + static bool astGetSizeSign(const Settings *settings, const Token *tok, unsigned int *size, char *sign) { if (!tok) @@ -287,3 +300,77 @@ void CheckType::signConversionError(const Token *tok) "signConversion", "Suspicious code: sign conversion of " + varname + " in calculation, even though " + varname + " can have a negative value"); } + + +//--------------------------------------------------------------------------- +// Checking for long cast of int result const long x = var1 * var2; +//--------------------------------------------------------------------------- + +void CheckType::checkLongCast() +{ + if (!_settings->isEnabled("style")) + return; + + // Assignments.. + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (!Token::Match(tok, "%var% =")) + continue; + if (!tok->variable() || !tok->variable()->isConst() || tok->variable()->typeStartToken()->str() != "long") + continue; + if (Token::Match(tok->next()->astOperand2(), "*|<<") && astIsIntResult(tok->next()->astOperand2())) + longCastAssignError(tok); + } + + // 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]; + + // function must return long data + const Token * def = scope->classDef; + bool islong = false; + while (Token::Match(def, "%type%|::")) { + if (def->str() == "long") { + islong = true; + break; + } + def = def->previous(); + } + if (!islong) + continue; + + // find return statement + // todo.. this is slow, we should only check last statement in each child scope + const Token *ret = nullptr; + for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + if (tok->str() == "return") { + if (!ret) + ret = tok; + else { + ret = nullptr; + break; + } + } + } + + if (ret && Token::Match(ret->astOperand1(), "*|<<") && astIsIntResult(ret->astOperand1())) + longCastReturnError(ret); + } +} + +void CheckType::longCastAssignError(const Token *tok) +{ + reportError(tok, + Severity::style, + "truncLongCastAssignment", + "possible loss of information, int result is assigned to long variable"); +} + +void CheckType::longCastReturnError(const Token *tok) +{ + reportError(tok, + Severity::style, + "truncLongCastReturn", + "possible loss of information, int result is returned as long value"); +} diff --git a/lib/checktype.h b/lib/checktype.h index 023928cfd..c72d1d560 100644 --- a/lib/checktype.h +++ b/lib/checktype.h @@ -49,6 +49,7 @@ public: checkType.checkTooBigBitwiseShift(); checkType.checkIntegerOverflow(); checkType.checkSignConversion(); + checkType.checkLongCast(); } /** @brief Run checks against the simplified token list */ @@ -67,6 +68,8 @@ public: /** @brief %Check for dangerous sign conversion */ void checkSignConversion(); + /** @brief %Check for implicit long cast of int result */ + void checkLongCast(); private: bool isUnsigned(const Variable *var) const; static bool isSigned(const Variable *var); @@ -75,12 +78,16 @@ private: void tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits); void integerOverflowError(const Token *tok, const ValueFlow::Value &value); void signConversionError(const Token *tok); + void longCastAssignError(const Token *tok); + void longCastReturnError(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckType c(0, settings, errorLogger); c.tooBigBitwiseShiftError(0, 32, ValueFlow::Value(64)); c.integerOverflowError(0, ValueFlow::Value(1LL<<32)); c.signConversionError(0); + c.longCastAssignError(0); + c.longCastReturnError(0); } static std::string myName() { @@ -91,7 +98,9 @@ private: return "Type checks\n" "- bitwise shift by too many bits (only enabled when --platform is used)\n" "- signed integer overflow (only enabled when --platform is used)\n" - "- dangerous sign conversion, when signed value can be negative\n"; + "- dangerous sign conversion, when signed value can be negative\n" + "- possible loss of information when assigning int result to long variable\n" + "- possible loss of information when returning int result as long return value\n"; } }; /// @} diff --git a/test/testtype.cpp b/test/testtype.cpp index b56014208..c4f1b2878 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -37,6 +37,8 @@ private: TEST_CASE(checkTooBigShift); TEST_CASE(checkIntegerOverflow); TEST_CASE(signConversion); + TEST_CASE(longCastAssign); + TEST_CASE(longCastReturn); } void check(const char code[], Settings* settings = 0) { @@ -134,6 +136,34 @@ private: "void f2() { f1(-4); }"); ASSERT_EQUALS("", errout.str()); } + + void longCastAssign() { + Settings settings; + settings.addEnabled("style"); + + check("long f(int x, int y) {\n" + " const long ret = x * y;\n" + " return ret;\n" + "}\n", &settings); + ASSERT_EQUALS("[test.cpp:2]: (style) possible loss of information, int result is assigned to long variable\n", errout.str()); + + // astIsIntResult + check("long f(int x, int y) {\n" + " const long ret = (long)x * y;\n" + " return ret;\n" + "}\n", &settings); + ASSERT_EQUALS("", errout.str()); + } + + void longCastReturn() { + Settings settings; + settings.addEnabled("style"); + + check("long f(int x, int y) {\n" + " return x * y;\n" + "}\n", &settings); + ASSERT_EQUALS("[test.cpp:2]: (style) possible loss of information, int result is returned as long value\n", errout.str()); + } }; REGISTER_TEST(TestType)