Skip to content

Commit a1c4a25

Browse files
author
Jean-François DEVERGE
committed
CheckCondition: detect compareValueOutOfTypeRange for bounded cast/arithmetic expressions
1 parent 8357500 commit a1c4a25

2 files changed

Lines changed: 233 additions & 0 deletions

File tree

‎lib/checkcondition.cpp‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1972,6 +1972,180 @@ void CheckConditionImpl::assignmentInCondition(const Token *eq)
19721972
Certainty::normal);
19731973
}
19741974

1975+
static bool getIntegerTypeRange(const Token* tok,
1976+
const Settings& settings,
1977+
MathLib::bigint& lower,
1978+
MathLib::bigint& upper)
1979+
{
1980+
if (!tok || !tok->valueType() || tok->valueType()->pointer)
1981+
return false;
1982+
1983+
std::uint8_t bits = 0;
1984+
switch (tok->valueType()->type) {
1985+
case ValueType::Type::BOOL:
1986+
bits = 1;
1987+
break;
1988+
case ValueType::Type::CHAR:
1989+
bits = settings.platform.char_bit;
1990+
break;
1991+
case ValueType::Type::SHORT:
1992+
bits = settings.platform.short_bit;
1993+
break;
1994+
case ValueType::Type::INT:
1995+
bits = settings.platform.int_bit;
1996+
break;
1997+
case ValueType::Type::LONG:
1998+
bits = settings.platform.long_bit;
1999+
break;
2000+
case ValueType::Type::LONGLONG:
2001+
bits = settings.platform.long_long_bit;
2002+
break;
2003+
default:
2004+
return false;
2005+
}
2006+
if (bits == 0 || bits > 64)
2007+
return false;
2008+
2009+
if (bits == 64 && tok->valueType()->sign == ValueType::Sign::UNSIGNED)
2010+
return false;
2011+
const MathLib::bigint max = bits == 64 ? std::numeric_limits<MathLib::bigint>::max() : (MathLib::bigint(1) << bits) - 1;
2012+
if (tok->valueType()->sign == ValueType::Sign::SIGNED) {
2013+
if (bits == 64) {
2014+
lower = std::numeric_limits<MathLib::bigint>::min();
2015+
upper = std::numeric_limits<MathLib::bigint>::max();
2016+
} else {
2017+
lower = -(MathLib::bigint(1) << (bits - 1));
2018+
upper = max / 2;
2019+
}
2020+
} else {
2021+
lower = 0;
2022+
upper = max;
2023+
}
2024+
return true;
2025+
}
2026+
2027+
static bool getIntegerExpressionRange(const Token* tok,
2028+
const Settings& settings,
2029+
MathLib::bigint& lower,
2030+
MathLib::bigint& upper)
2031+
{
2032+
if (!tok)
2033+
return false;
2034+
if (tok->hasKnownIntValue()) {
2035+
lower = upper = tok->getKnownIntValue();
2036+
return true;
2037+
}
2038+
2039+
if (tok->isCast() && tok->astOperand1()) {
2040+
MathLib::bigint typeLower;
2041+
MathLib::bigint typeUpper;
2042+
const bool hasSourceRange = getIntegerExpressionRange(tok->astOperand1(), settings, lower, upper);
2043+
const bool hasTypeRange = getIntegerTypeRange(tok, settings, typeLower, typeUpper);
2044+
if (!hasSourceRange && !hasTypeRange)
2045+
return false;
2046+
if (!hasSourceRange) {
2047+
lower = typeLower;
2048+
upper = typeUpper;
2049+
return true;
2050+
}
2051+
if (!hasTypeRange)
2052+
return true;
2053+
lower = std::max(lower, typeLower);
2054+
upper = std::min(upper, typeUpper);
2055+
return lower <= upper;
2056+
}
2057+
2058+
if (Token::simpleMatch(tok, "(") && tok->astOperand1())
2059+
return getIntegerExpressionRange(tok->astOperand1(), settings, lower, upper);
2060+
2061+
if (Token::Match(tok, "+|-")) {
2062+
MathLib::bigint operandLower;
2063+
MathLib::bigint operandUpper;
2064+
MathLib::bigint constantLower;
2065+
MathLib::bigint constantUpper;
2066+
if (!getIntegerExpressionRange(tok->astOperand1(), settings, operandLower, operandUpper) ||
2067+
!getIntegerExpressionRange(tok->astOperand2(), settings, constantLower, constantUpper) ||
2068+
constantLower != constantUpper)
2069+
return false;
2070+
const MathLib::bigint minimum = std::numeric_limits<MathLib::bigint>::min();
2071+
const MathLib::bigint maximum = std::numeric_limits<MathLib::bigint>::max();
2072+
if (tok->str() == "+" &&
2073+
((constantLower > 0 && operandUpper > maximum - constantLower) ||
2074+
(constantLower < 0 && operandLower < minimum - constantLower)))
2075+
return false;
2076+
if (tok->str() == "-" &&
2077+
((constantUpper > 0 && operandLower < minimum + constantUpper) ||
2078+
(constantUpper < 0 && operandUpper > maximum + constantUpper)))
2079+
return false;
2080+
if (tok->str() == "+") {
2081+
lower = operandLower + constantLower;
2082+
upper = operandUpper + constantUpper;
2083+
} else {
2084+
lower = operandLower - constantUpper;
2085+
upper = operandUpper - constantLower;
2086+
}
2087+
return true;
2088+
}
2089+
2090+
return getIntegerTypeRange(tok, settings, lower, upper);
2091+
}
2092+
2093+
static bool compareIntegerRange(const Token* comparison,
2094+
MathLib::bigint lower,
2095+
MathLib::bigint upper,
2096+
MathLib::bigint value,
2097+
bool& result)
2098+
{
2099+
if (!comparison || !comparison->isComparisonOp())
2100+
return false;
2101+
if (comparison->str() == "<") {
2102+
if (upper < value)
2103+
result = true;
2104+
else if (lower >= value)
2105+
result = false;
2106+
else
2107+
return false;
2108+
} else if (comparison->str() == "<=") {
2109+
if (upper <= value)
2110+
result = true;
2111+
else if (lower > value)
2112+
result = false;
2113+
else
2114+
return false;
2115+
} else if (comparison->str() == ">") {
2116+
if (lower > value)
2117+
result = true;
2118+
else if (upper <= value)
2119+
result = false;
2120+
else
2121+
return false;
2122+
} else if (comparison->str() == ">=") {
2123+
if (lower >= value)
2124+
result = true;
2125+
else if (upper < value)
2126+
result = false;
2127+
else
2128+
return false;
2129+
} else if (comparison->str() == "==") {
2130+
if (lower == upper && lower == value)
2131+
result = true;
2132+
else if (value < lower || value > upper)
2133+
result = false;
2134+
else
2135+
return false;
2136+
} else if (comparison->str() == "!=") {
2137+
if (value < lower || value > upper)
2138+
result = true;
2139+
else if (lower == upper && lower == value)
2140+
result = false;
2141+
else
2142+
return false;
2143+
} else {
2144+
return false;
2145+
}
2146+
return true;
2147+
}
2148+
19752149
void CheckConditionImpl::checkCompareValueOutOfTypeRange()
19762150
{
19772151
if (!mSettings.severity.isEnabled(Severity::style) && !mSettings.isPremiumEnabled("compareValueOutOfTypeRangeError"))
@@ -1988,6 +2162,26 @@ void CheckConditionImpl::checkCompareValueOutOfTypeRange()
19882162
if (!tok->isComparisonOp() || !tok->isBinaryOp())
19892163
continue;
19902164

2165+
for (int i = 0; i < 2; ++i) {
2166+
const Token* const expressionTok = (i == 0) ? tok->astOperand2() : tok->astOperand1();
2167+
const Token* const valueTok = (i == 0) ? tok->astOperand1() : tok->astOperand2();
2168+
if (!expressionTok || !valueTok || !valueTok->hasKnownIntValue() || expressionTok->hasKnownIntValue())
2169+
continue;
2170+
if (expressionTok->str() != "(" && !expressionTok->isCast() && !expressionTok->isArithmeticalOp())
2171+
continue;
2172+
MathLib::bigint lower;
2173+
MathLib::bigint upper;
2174+
if (!getIntegerExpressionRange(expressionTok, mSettings, lower, upper))
2175+
continue;
2176+
bool result;
2177+
if (!compareIntegerRange(tok, lower, upper, valueTok->getKnownIntValue(), result) || diag(tok))
2178+
continue;
2179+
compareValueOutOfTypeRangeError(valueTok,
2180+
expressionTok->valueType() ? expressionTok->valueType()->str() : "",
2181+
valueTok->getKnownIntValue(),
2182+
result);
2183+
}
2184+
19912185
for (int i = 0; i < 2; ++i) {
19922186
const Token * const valueTok = (i == 0) ? tok->astOperand1() : tok->astOperand2();
19932187
const Token * const typeTok = valueTok->astSibling();

‎test/testcondition.cpp‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6546,6 +6546,45 @@ class TestCondition : public TestFixture {
65466546
"[test.cpp:4:13]: (style) Comparing expression of type 'const unsigned int &' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
65476547
errout_str());
65486548

6549+
check("typedef unsigned int uint32;\n"
6550+
"typedef unsigned long long uint64;\n"
6551+
"typedef long long sint64;\n"
6552+
"void f(uint32 x) {\n"
6553+
" uint64 tmp = ((uint64)x) + 1ULL;\n"
6554+
" if (tmp > 4294967295ULL)\n"
6555+
" tmp = 4294967295ULL;\n"
6556+
" if ((((sint64)((uint32)tmp)) - 1LL) < 0LL) {}\n"
6557+
" if ((((sint64)((uint32)tmp)) - 1LL) > 4294967295LL) {}\n"
6558+
"}\n", settingsUnix64);
6559+
ASSERT_EQUALS("[test.cpp:9:43]: (style) Comparing expression of type 'signed long long' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
6560+
errout_str());
6561+
6562+
// cast directly around variable: both the range-based and the declared-type
6563+
// analysis can prove the condition invariant; diag() must prevent duplicates
6564+
check("void f(unsigned char c) {\n"
6565+
" if ((unsigned char)c > 255) {}\n"
6566+
"}\n", settingsUnix64);
6567+
ASSERT_EQUALS("[test.cpp:2:28]: (style) Comparing expression of type 'unsigned char' against value 255. Condition is always false. [compareValueOutOfTypeRangeError]\n",
6568+
errout_str());
6569+
6570+
check("void f(unsigned char c) {\n"
6571+
" if ((unsigned char)c == 256) {}\n"
6572+
"}\n", settingsUnix64);
6573+
ASSERT_EQUALS("[test.cpp:2:29]: (style) Comparing expression of type 'unsigned char' against value 256. Condition is always false. [compareValueOutOfTypeRangeError]\n",
6574+
errout_str());
6575+
6576+
check("void f(unsigned int u) {\n"
6577+
" if ((unsigned int)u > 4294967295ULL) {}\n"
6578+
"}\n", settingsUnix64);
6579+
ASSERT_EQUALS("[test.cpp:2:27]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
6580+
errout_str());
6581+
6582+
check("void f(unsigned short s) {\n"
6583+
" if ((unsigned int)s > 4294967295ULL) {}\n"
6584+
"}\n", settingsUnix64);
6585+
ASSERT_EQUALS("[test.cpp:2:27]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
6586+
errout_str());
6587+
65496588
check("void f() {\n"
65506589
" long long ll = 1024 * 1024 * 1024;\n"
65516590
" if (ll * 8 < INT_MAX) {}\n"

0 commit comments

Comments
 (0)