diff --git a/Makefile b/Makefile index 37156d357af..7cac9fc3efb 100644 --- a/Makefile +++ b/Makefile @@ -487,7 +487,7 @@ $(libcppdir)/addoninfo.o: lib/addoninfo.cpp externals/picojson/picojson.h lib/ad $(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h lib/utils.h lib/xml.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/analyzerinfo.cpp -$(libcppdir)/astutils.o: lib/astutils.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkclass.h lib/checkers.h lib/config.h lib/errortypes.h lib/findtoken.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vfvalue.h +$(libcppdir)/astutils.o: lib/astutils.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkclass.h lib/checkers.h lib/config.h lib/errortypes.h lib/findtoken.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vf_common.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/astutils.cpp $(libcppdir)/check.o: lib/check.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 2330a9d2b94..4795b9689f0 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -32,6 +32,7 @@ #include "utils.h" #include "valueflow.h" #include "valueptr.h" +#include "vf_common.h" #include "vfvalue.h" #include "checkclass.h" @@ -41,12 +42,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 +930,97 @@ const Token* getCondTokFromEnd(const Token* endBlock) return getCondTokFromEndImpl(endBlock); } +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 ValueFlow::getMinMaxValues(exprToCheck->valueType(), settings.platform, exprRange.first, exprRange.second); +} + +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; + + // Leftshift with negative values is undefined behavior. + if ((rightRange->first < 0) || (rightRange->second < 0) || (leftRange->first < 0) || (leftRange->second < 0)) + 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 509d13b54e0..159cda51b93 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -384,10 +384,18 @@ void CheckType::checkLongCast() if (type && checkTypeCombination(*type, *retVt, *mSettings) && type->pointer == 0U && type->originalTypeName.empty()) { - if (!tok->astOperand1()->hasKnownIntValue()) { + 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(tok->astOperand1()->getKnownIntValue())) + } 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 diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index 5055ba71a8e..b2dc26d051a 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -167,7 +167,7 @@ $(libcppdir)/addoninfo.o: ../lib/addoninfo.cpp ../externals/picojson/picojson.h $(libcppdir)/analyzerinfo.o: ../lib/analyzerinfo.cpp ../externals/tinyxml2/tinyxml2.h ../lib/analyzerinfo.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/filesettings.h ../lib/mathlib.h ../lib/path.h ../lib/platform.h ../lib/standards.h ../lib/utils.h ../lib/xml.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/analyzerinfo.cpp -$(libcppdir)/astutils.o: ../lib/astutils.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkclass.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/findtoken.h ../lib/infer.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/valueflow.h ../lib/valueptr.h ../lib/vfvalue.h +$(libcppdir)/astutils.o: ../lib/astutils.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkclass.h ../lib/checkers.h ../lib/config.h ../lib/errortypes.h ../lib/findtoken.h ../lib/infer.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/valueflow.h ../lib/valueptr.h ../lib/vf_common.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/astutils.cpp $(libcppdir)/check.o: ../lib/check.cpp ../lib/addoninfo.h ../lib/check.h ../lib/checkers.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/standards.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h diff --git a/test/testtype.cpp b/test/testtype.cpp index ea31fbdc313..eadb56dedac 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -465,24 +465,36 @@ 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 code3[] = "long f() {\n" - " int n = 1;\n" - " return n << 12;\n" - "}\n"; - check(code3, dinit(CheckOptions, $.settings = &settings)); + 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 code4[] = "long f(int n) {\n" - " return n << 12;\n" - "}\n"; - check(code4, dinit(CheckOptions, $.settings = &settings)); + 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 code5[] = "long f() {\n" - " unsigned int n = 1U << 20;\n" - " return n << 20;\n" - "}\n"; - check(code5, dinit(CheckOptions, $.settings = &settings)); + 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