Valueflow: Fix right shift with more than 31 bits (#1553)

When comparing if the shift is large enough to make the result zero, use
an unsigned long long to make sure the result fits. Also, a check that
avoids setting the value if the shift is equal to or larger than the
number of bits in the operand (this is undefined behaviour). Finally,
add a check to make sure the calculated value is not too large to store.

Add test cases to cover this.

This was detected by an MSVC warning.

valueflow.cpp(1350): warning C4334: '<<' : result of 32-bit shift implicitly
                     converted to 64 bits (was 64-bit shift intended?)
This commit is contained in:
rikardfalkeborn 2019-01-01 14:15:50 +01:00 committed by Daniel Marjamäki
parent a73e989d1a
commit 13ffefc8b8
2 changed files with 82 additions and 3 deletions

View File

@ -1315,7 +1315,7 @@ static bool getExpressionRange(const Token *expr, MathLib::bigint *minvalue, Mat
return false; return false;
} }
static void valueFlowRightShift(TokenList *tokenList) static void valueFlowRightShift(TokenList *tokenList, const Settings* settings)
{ {
for (Token *tok = tokenList->front(); tok; tok = tok->next()) { for (Token *tok = tokenList->front(); tok; tok = tok->next()) {
if (tok->str() != ">>") if (tok->str() != ">>")
@ -1345,7 +1345,19 @@ static void valueFlowRightShift(TokenList *tokenList)
continue; continue;
if (lhsmax < 0) if (lhsmax < 0)
continue; continue;
if ((1 << rhsvalue) <= lhsmax) int lhsbits;
if ((tok->astOperand1()->valueType()->type == ValueType::Type::CHAR) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::SHORT) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::BOOL) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::INT))
lhsbits = settings->int_bit;
else if (tok->astOperand1()->valueType()->type == ValueType::Type::LONG)
lhsbits = settings->long_bit;
else if (tok->astOperand1()->valueType()->type == ValueType::Type::LONGLONG)
lhsbits = settings->long_long_bit;
else
continue;
if (rhsvalue >= lhsbits || rhsvalue >= MathLib::bigint_bits || (1ULL << rhsvalue) <= lhsmax)
continue; continue;
ValueFlow::Value val(0); ValueFlow::Value val(0);
@ -4764,7 +4776,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
std::size_t values = 0; std::size_t values = 0;
while (std::time(0) < timeout && values < getTotalValues(tokenlist)) { while (std::time(0) < timeout && values < getTotalValues(tokenlist)) {
values = getTotalValues(tokenlist); values = getTotalValues(tokenlist);
valueFlowRightShift(tokenlist); valueFlowRightShift(tokenlist, settings);
valueFlowOppositeCondition(symboldatabase, settings); valueFlowOppositeCondition(symboldatabase, settings);
valueFlowTerminatingCondition(tokenlist, symboldatabase, settings); valueFlowTerminatingCondition(tokenlist, symboldatabase, settings);
valueFlowBeforeCondition(tokenlist, symboldatabase, errorLogger, settings); valueFlowBeforeCondition(tokenlist, symboldatabase, errorLogger, settings);

View File

@ -2341,6 +2341,11 @@ private:
void valueFlowRightShift() { void valueFlowRightShift() {
const char *code; const char *code;
/* Set some temporary fixed values to simplify testing */
const Settings settingsTmp = settings;
settings.int_bit = 32;
settings.long_bit = 64;
settings.long_long_bit = MathLib::bigint_bits * 2;
code = "int f(int a) {\n" code = "int f(int a) {\n"
" int x = (a & 0xff) >> 16;\n" " int x = (a & 0xff) >> 16;\n"
@ -2353,6 +2358,68 @@ private:
" return x;\n" " return x;\n"
"}"; "}";
ASSERT_EQUALS(true, testValueOfX(code,3U,0)); ASSERT_EQUALS(true, testValueOfX(code,3U,0));
code = "int f(int y) {\n"
" int x = (y & 0xFFFFFFF) >> 31;\n"
" return x;\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 3u, 0));
code = "int f(int y) {\n"
" int x = (y & 0xFFFFFFF) >> 32;\n"
" return x;\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3u, 0));
code = "int f(short y) {\n"
" int x = (y & 0xFFFFFF) >> 31;\n"
" return x;\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 3u, 0));
code = "int f(short y) {\n"
" int x = (y & 0xFFFFFF) >> 32;\n"
" return x;\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3u, 0));
code = "int f(long y) {\n"
" int x = (y & 0xFFFFFF) >> 63;\n"
" return x;\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 3u, 0));
code = "int f(long y) {\n"
" int x = (y & 0xFFFFFF) >> 64;\n"
" return x;\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3u, 0));
code = "int f(long long y) {\n"
" int x = (y & 0xFFFFFF) >> 63;\n"
" return x;\n"
"}";
ASSERT_EQUALS(true, testValueOfX(code, 3u, 0));
code = "int f(long long y) {\n"
" int x = (y & 0xFFFFFF) >> 64;\n"
" return x;\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3u, 0));
code = "int f(long long y) {\n"
" int x = (y & 0xFFFFFF) >> 121;\n"
" return x;\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3u, 0));
code = "int f(long long y) {\n"
" int x = (y & 0xFFFFFF) >> 128;\n"
" return x;\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 3u, 0));
settings = settingsTmp;
} }
void valueFlowFwdAnalysis() { void valueFlowFwdAnalysis() {