Skip to content

Commit 920f386

Browse files
committed
Extended checkLongCast function with expression max min check and
added astutils functions for getting expression max min values and a check if bigint within int range.
1 parent fe0381f commit 920f386

4 files changed

Lines changed: 178 additions & 12 deletions

File tree

lib/astutils.cpp

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,15 @@
4141
#include <functional>
4242
#include <initializer_list>
4343
#include <iterator>
44+
#include <limits>
4445
#include <list>
4546
#include <set>
4647
#include <type_traits>
4748
#include <unordered_map>
4849
#include <utility>
4950

51+
#define INTEGER_SHIFT_LIMIT (sizeof(int) * 8 - 1) // The number of bits, where a left shift cannot be guaranteed to be within int range.
52+
5053
const Token* findExpression(const nonneg int exprid,
5154
const Token* start,
5255
const Token* end,
@@ -926,6 +929,137 @@ const Token* getCondTokFromEnd(const Token* endBlock)
926929
return getCondTokFromEndImpl(endBlock);
927930
}
928931

932+
static std::pair<MathLib::bigint, MathLib::bigint> getIntegralMinMaxValues(int bits, ValueType::Sign sign)
933+
{
934+
using bigint = MathLib::bigint;
935+
using biguint = MathLib::biguint;
936+
937+
if (bits <= 0)
938+
return { bigint(0), bigint(0) };
939+
940+
// Unsigned: [0, 2^bits - 1]
941+
if (sign == ValueType::Sign::UNSIGNED) {
942+
// If bits exceed what MathLib can safely shift, saturate to max representable
943+
if (bits >= MathLib::bigint_bits) {
944+
biguint max_u = std::numeric_limits<biguint>::max();
945+
return { bigint(0), bigint(max_u) };
946+
}
947+
biguint max_u = (biguint(1) << bits) - 1;
948+
return { bigint(0), bigint(max_u) };
949+
}
950+
951+
// Signed: [-(2^(bits-1)), 2^(bits-1)-1]
952+
if (bits >= MathLib::bigint_bits) {
953+
bigint min_s = std::numeric_limits<bigint>::min();
954+
bigint max_s = std::numeric_limits<bigint>::max();
955+
return { min_s, max_s };
956+
}
957+
bigint max_s = (bigint(1) << (bits - 1)) - 1;
958+
bigint min_s = -(bigint(1) << (bits - 1));
959+
return { min_s, max_s };
960+
}
961+
962+
static bool getIntegralTypeRange(const ValueType* type, const Settings& settings, std::pair<MathLib::bigint, MathLib::bigint>& range)
963+
{
964+
if (!type || !type->isIntegral())
965+
return false;
966+
967+
const int bits = type->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer) * 8;
968+
if (bits <= 0 || bits > 64)
969+
return false;
970+
971+
range = getIntegralMinMaxValues(bits, type->sign);
972+
973+
return true;
974+
}
975+
976+
bool getExpressionResultRange(const Token* expr, const Settings& settings, std::pair<MathLib::bigint, MathLib::bigint>& exprRange)
977+
{
978+
if (!expr)
979+
return false;
980+
981+
// Early return for known values
982+
if (expr->hasKnownIntValue()) {
983+
exprRange = { expr->getKnownIntValue(), expr->getKnownIntValue() };
984+
return true;
985+
}
986+
987+
// Early return for non-integral expressions
988+
if (!expr->valueType() || !expr->valueType()->isIntegral())
989+
return false;
990+
991+
//Check binary op - bitwise and
992+
if (expr->isBinaryOp() && expr->str() == "&") {
993+
std::pair<MathLib::bigint, MathLib::bigint> leftRange, rightRange;
994+
if (getExpressionResultRange(expr->astOperand1(), settings, leftRange) &&
995+
getExpressionResultRange(expr->astOperand2(), settings, rightRange)) {
996+
997+
if (leftRange.second >= INT64_MAX || rightRange.second >= INT64_MAX)
998+
// Abort for values larger than INT64_MAX since bigint do not handle them well
999+
return false;
1000+
1001+
exprRange.first = leftRange.first & rightRange.first;
1002+
exprRange.second = leftRange.second & rightRange.second;
1003+
1004+
// Return false if negative values after bitwise &
1005+
return !(exprRange.first < 0 || exprRange.second < 0);
1006+
}
1007+
}
1008+
1009+
// Find original type before casts
1010+
const Token* exprToCheck = expr;
1011+
while (exprToCheck->isCast()) {
1012+
const Token* castFrom = exprToCheck->astOperand2() ? exprToCheck->astOperand2() : exprToCheck->astOperand1();
1013+
if (!castFrom || !castFrom->valueType() || !castFrom->valueType()->isIntegral())
1014+
break;
1015+
if (castFrom->valueType()->pointer >= 1)
1016+
break;
1017+
if (castFrom->valueType()->type >= exprToCheck->valueType()->type &&
1018+
castFrom->valueType()->sign == ValueType::Sign::SIGNED)
1019+
break;
1020+
exprToCheck = castFrom;
1021+
}
1022+
1023+
return getIntegralTypeRange(exprToCheck->valueType(), settings, exprRange);
1024+
}
1025+
1026+
template<typename Op>
1027+
static bool checkAllRangeOperations(const std::pair<MathLib::bigint, MathLib::bigint>& left,
1028+
const std::pair<MathLib::bigint, MathLib::bigint>& right,
1029+
const Settings& settings,
1030+
Op operation)
1031+
{
1032+
return settings.platform.isIntValue(operation(left.first, right.first)) &&
1033+
settings.platform.isIntValue(operation(left.first, right.second)) &&
1034+
settings.platform.isIntValue(operation(left.second, right.first)) &&
1035+
settings.platform.isIntValue(operation(left.second, right.second));
1036+
}
1037+
1038+
bool isOperationResultWithinIntRange(const Token* op, const Settings& settings, std::pair<MathLib::bigint, MathLib::bigint>* leftRange, std::pair<MathLib::bigint, MathLib::bigint>* rightRange)
1039+
{
1040+
if (!op || !leftRange || !rightRange)
1041+
return false;
1042+
1043+
if (op->str() == "<<") {
1044+
// If the lefthand operand is 31 or higher the resulting shift will be a negative value or greater than int range.
1045+
if ((rightRange->first >= INTEGER_SHIFT_LIMIT) || rightRange->second >= INTEGER_SHIFT_LIMIT)
1046+
return false;
1047+
1048+
return checkAllRangeOperations(*leftRange, *rightRange, settings,
1049+
[](auto a, auto b) {
1050+
return a << b;
1051+
});
1052+
}
1053+
1054+
if (op->str() == "*")
1055+
return checkAllRangeOperations(*leftRange, *rightRange, settings,
1056+
[](auto a, auto b) {
1057+
return a * b;
1058+
});
1059+
1060+
return false;
1061+
}
1062+
9291063
Token* getInitTok(Token* tok) {
9301064
return getInitTokImpl(tok);
9311065
}

lib/astutils.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,10 @@ const Token* getStepTok(const Token* tok);
225225
Token* getCondTokFromEnd(Token* endBlock);
226226
const Token* getCondTokFromEnd(const Token* endBlock);
227227

228+
bool getExpressionResultRange(const Token* expr, const Settings& settings, std::pair<MathLib::bigint, MathLib::bigint>& exprRange);
229+
bool isOperationResultWithinIntRange(const Token* op, const Settings& settings, std::pair<MathLib::bigint, MathLib::bigint>* leftRange, std::pair<MathLib::bigint, MathLib::bigint>* rightRange);
230+
231+
228232
/// For a "break" token, locate the next token to execute. The token will
229233
/// be either a "}" or a ";".
230234
const Token *findNextTokenFromBreak(const Token *breakToken);

lib/checktype.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,24 @@ void CheckType::checkLongCast()
384384
if (type && checkTypeCombination(*type, *retVt, *mSettings) &&
385385
type->pointer == 0U &&
386386
type->originalTypeName.empty() &&
387-
!tok->astOperand1()->hasKnownIntValue())
388-
ret = tok;
387+
!tok->astOperand1()->hasKnownIntValue()) {
388+
std::pair<MathLib::bigint, MathLib::bigint> opRange1, opRange2;
389+
if (!getExpressionResultRange(tok->astOperand1()->astOperand1(), *mSettings, opRange1) || !getExpressionResultRange(tok->astOperand1()->astOperand2(), *mSettings, opRange2)) {
390+
ret = tok;
391+
break;
392+
}
393+
394+
if (!mSettings->platform.isIntValue(opRange1.first) || !mSettings->platform.isIntValue(opRange1.second) ||
395+
!mSettings->platform.isIntValue(opRange2.first) || !mSettings->platform.isIntValue(opRange2.second)) {
396+
ret = tok;
397+
break;
398+
}
399+
400+
if (!isOperationResultWithinIntRange(tok->astOperand1(), *mSettings, &opRange1, &opRange2)) {
401+
ret = tok;
402+
break;
403+
}
404+
}
389405
}
390406
// All return statements must have problem otherwise no warning
391407
if (ret != tok) {

test/testtype.cpp

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -465,19 +465,31 @@ class TestType : public TestFixture {
465465
check(code2, dinit(CheckOptions, $.settings = &settingsWin));
466466
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());
467467

468-
const char code3[] = "long f() {\n"
469-
" int n = 1;\n"
470-
" return n << 12;\n"
471-
"}\n";
472-
check(code3, dinit(CheckOptions, $.settings = &settings));
468+
const char code3a[] = "long f() {\n"
469+
" int n = 1;\n"
470+
" return n << 12;\n"
471+
"}\n";
472+
check(code3a, dinit(CheckOptions, $.settings = &settings));
473473
ASSERT_EQUALS("", errout_str());
474-
475-
const char code4[] = "long f(int n) {\n"
476-
" return n << 12;\n"
477-
"}\n";
478-
check(code4, dinit(CheckOptions, $.settings = &settings));
474+
const char code3b[] = "long f(int n) {\n"
475+
" return n << 12;\n"
476+
"}\n";
477+
check(code3b, dinit(CheckOptions, $.settings = &settings));
479478
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());
480479

480+
const char code4a[] = "long f(int x) {\n"
481+
" int y = 0x07FFFF;\n"
482+
" return ((x & y) << (12 & x));\n"
483+
"}\n";
484+
check(code4a, dinit(CheckOptions, $.settings = &settings));
485+
ASSERT_EQUALS("", errout_str());
486+
const char code4b[] = "long f(int x) {\n"
487+
" int y = 0x080000;\n"
488+
" return ((x & y) << (12 & x));\n"
489+
"}\n";
490+
check(code4b, dinit(CheckOptions, $.settings = &settings));
491+
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());
492+
481493
// typedef
482494
check("size_t f(int x, int y) {\n"
483495
" return x * y;\n"

0 commit comments

Comments
 (0)