diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e140d965114..6ee3e32ad2e 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -41,12 +41,15 @@ #include #include #include +#include #include #include #include #include #include +#define INTEGER_SHIFT_LIMIT (sizeof(int) * 8 - 1) // The number of bits, where a left shift cannot be guaranteed to be within int range. + const Token* findExpression(const nonneg int exprid, const Token* start, const Token* end, @@ -926,6 +929,137 @@ const Token* getCondTokFromEnd(const Token* endBlock) return getCondTokFromEndImpl(endBlock); } +static std::pair getIntegralMinMaxValues(int bits, ValueType::Sign sign) +{ + using bigint = MathLib::bigint; + using biguint = MathLib::biguint; + + if (bits <= 0) + return { bigint(0), bigint(0) }; + + // Unsigned: [0, 2^bits - 1] + if (sign == ValueType::Sign::UNSIGNED) { + // If bits exceed what MathLib can safely shift, saturate to max representable + if (bits >= MathLib::bigint_bits) { + biguint max_u = std::numeric_limits::max(); + return { bigint(0), bigint(max_u) }; + } + biguint max_u = (biguint(1) << bits) - 1; + return { bigint(0), bigint(max_u) }; + } + + // Signed: [-(2^(bits-1)), 2^(bits-1)-1] + if (bits >= MathLib::bigint_bits) { + bigint min_s = std::numeric_limits::min(); + bigint max_s = std::numeric_limits::max(); + return { min_s, max_s }; + } + bigint max_s = (bigint(1) << (bits - 1)) - 1; + bigint min_s = -(bigint(1) << (bits - 1)); + return { min_s, max_s }; +} + +static bool getIntegralTypeRange(const ValueType* type, const Settings& settings, std::pair& range) +{ + if (!type || !type->isIntegral()) + return false; + + const int bits = type->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer) * 8; + if (bits <= 0 || bits > 64) + return false; + + range = getIntegralMinMaxValues(bits, type->sign); + + return true; +} + +bool getExpressionResultRange(const Token* expr, const Settings& settings, std::pair& exprRange) +{ + if (!expr) + return false; + + // Early return for known values + if (expr->hasKnownIntValue()) { + exprRange = { expr->getKnownIntValue(), expr->getKnownIntValue() }; + return true; + } + + // Early return for non-integral expressions + if (!expr->valueType() || !expr->valueType()->isIntegral()) + return false; + + //Check binary op - bitwise and + if (expr->isBinaryOp() && expr->str() == "&") { + std::pair leftRange, rightRange; + if (getExpressionResultRange(expr->astOperand1(), settings, leftRange) && + getExpressionResultRange(expr->astOperand2(), settings, rightRange)) { + + if (leftRange.second >= INT64_MAX || rightRange.second >= INT64_MAX) + // Abort for values larger than INT64_MAX since bigint do not handle them well + return false; + + exprRange.first = leftRange.first & rightRange.first; + exprRange.second = leftRange.second & rightRange.second; + + // Return false if negative values after bitwise & + return !(exprRange.first < 0 || exprRange.second < 0); + } + } + + // Find original type before casts + const Token* exprToCheck = expr; + while (exprToCheck->isCast()) { + const Token* castFrom = exprToCheck->astOperand2() ? exprToCheck->astOperand2() : exprToCheck->astOperand1(); + if (!castFrom || !castFrom->valueType() || !castFrom->valueType()->isIntegral()) + break; + if (castFrom->valueType()->pointer >= 1) + break; + if (castFrom->valueType()->type >= exprToCheck->valueType()->type && + castFrom->valueType()->sign == ValueType::Sign::SIGNED) + break; + exprToCheck = castFrom; + } + + return getIntegralTypeRange(exprToCheck->valueType(), settings, exprRange); +} + +template +static bool checkAllRangeOperations(const std::pair& left, + const std::pair& right, + const Settings& settings, + Op operation) +{ + return settings.platform.isIntValue(operation(left.first, right.first)) && + settings.platform.isIntValue(operation(left.first, right.second)) && + settings.platform.isIntValue(operation(left.second, right.first)) && + settings.platform.isIntValue(operation(left.second, right.second)); +} + +bool isOperationResultWithinIntRange(const Token* op, const Settings& settings, std::pair* leftRange, std::pair* rightRange) +{ + if (!op || !leftRange || !rightRange) + return false; + + if (op->str() == "<<") { + // If the lefthand operand is 31 or higher the resulting shift will be a negative value or greater than int range. + if ((rightRange->first >= INTEGER_SHIFT_LIMIT) || rightRange->second >= INTEGER_SHIFT_LIMIT) + return false; + + return checkAllRangeOperations(*leftRange, *rightRange, settings, + [](MathLib::bigint a, MathLib::bigint b) { + return a << b; + }); + } + + if (op->str() == "*") + return checkAllRangeOperations(*leftRange, *rightRange, settings, + [](MathLib::bigint a, MathLib::bigint b) { + return a * b; + }); + + return false; +} + Token* getInitTok(Token* tok) { return getInitTokImpl(tok); } diff --git a/lib/astutils.h b/lib/astutils.h index 4002e065ab5..54de1944d1b 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -225,6 +225,10 @@ const Token* getStepTok(const Token* tok); Token* getCondTokFromEnd(Token* endBlock); const Token* getCondTokFromEnd(const Token* endBlock); +bool getExpressionResultRange(const Token* expr, const Settings& settings, std::pair& exprRange); +bool isOperationResultWithinIntRange(const Token* op, const Settings& settings, std::pair* leftRange, std::pair* rightRange); + + /// For a "break" token, locate the next token to execute. The token will /// be either a "}" or a ";". const Token *findNextTokenFromBreak(const Token *breakToken); diff --git a/lib/checktype.cpp b/lib/checktype.cpp index 68b6580d270..159cda51b93 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -383,8 +383,20 @@ void CheckType::checkLongCast() const ValueType *type = tok->astOperand1()->valueType(); if (type && checkTypeCombination(*type, *retVt, *mSettings) && type->pointer == 0U && - type->originalTypeName.empty()) - ret = tok; + type->originalTypeName.empty()) { + std::pair opRange1, opRange2; + if (tok->astOperand1()->hasKnownIntValue()) { + if (!mSettings->platform.isIntValue(tok->astOperand1()->getKnownIntValue())) + ret = tok; + } else if (!getExpressionResultRange(tok->astOperand1()->astOperand1(), *mSettings, opRange1) || !getExpressionResultRange(tok->astOperand1()->astOperand2(), *mSettings, opRange2)) { + ret = tok; + } else if (!mSettings->platform.isIntValue(opRange1.first) || !mSettings->platform.isIntValue(opRange1.second) || + !mSettings->platform.isIntValue(opRange2.first) || !mSettings->platform.isIntValue(opRange2.second)) { + ret = tok; + } else if (!isOperationResultWithinIntRange(tok->astOperand1(), *mSettings, &opRange1, &opRange2)) { + ret = tok; + } + } } // All return statements must have problem otherwise no warning if (ret != tok) { diff --git a/test/testtype.cpp b/test/testtype.cpp index b8cfae820d5..eadb56dedac 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -465,6 +465,38 @@ class TestType : public TestFixture { check(code2, dinit(CheckOptions, $.settings = &settingsWin)); ASSERT_EQUALS("[test.cpp:2:3]: (style) int result is returned as long long value. If the return value is long long to avoid loss of information, then you have loss of information. [truncLongCastReturn]\n", errout_str()); + const char code3a[] = "long f() {\n" + " int n = 1;\n" + " return n << 12;\n" + "}\n"; + check(code3a, dinit(CheckOptions, $.settings = &settings)); + ASSERT_EQUALS("", errout_str()); + const char code3b[] = "long f(int n) {\n" + " return n << 12;\n" + "}\n"; + check(code3b, dinit(CheckOptions, $.settings = &settings)); + ASSERT_EQUALS("[test.cpp:2:5]: (style) int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]\n", errout_str()); + + const char code3c[] = "long f() {\n" + " unsigned int n = 1U << 20;\n" + " return n << 20;\n" + "}\n"; + check(code3c, dinit(CheckOptions, $.settings = &settings)); + ASSERT_EQUALS("[test.cpp:3:5]: (style) int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]\n", errout_str()); + + const char code4a[] = "long f(int x) {\n" + " int y = 0x07FFFF;\n" + " return ((x & y) << (12 & x));\n" + "}\n"; + check(code4a, dinit(CheckOptions, $.settings = &settings)); + ASSERT_EQUALS("", errout_str()); + const char code4b[] = "long f(int x) {\n" + " int y = 0x080000;\n" + " return ((x & y) << (12 & x));\n" + "}\n"; + check(code4b, dinit(CheckOptions, $.settings = &settings)); + ASSERT_EQUALS("[test.cpp:3:5]: (style) int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]\n", errout_str()); + // typedef check("size_t f(int x, int y) {\n" " return x * y;\n"