Skip to content

Commit 9f68905

Browse files
danmardanmar
committed
Refactoring CheckType checkers. Use ValueType.
1 parent 5dc42cc commit 9f68905

2 files changed

Lines changed: 51 additions & 138 deletions

File tree

‎lib/checktype.cpp‎

Lines changed: 45 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -30,82 +30,6 @@ namespace {
3030
CheckType instance;
3131
}
3232

33-
static bool astIsIntResult(const Token *tok)
34-
{
35-
if (!tok)
36-
return false;
37-
if (tok->astOperand1())
38-
return astIsIntResult(tok->astOperand1()) && astIsIntResult(tok->astOperand2());
39-
// todo: handle numbers
40-
if (!tok->variable())
41-
return false;
42-
const Token *type = tok->variable()->typeStartToken();
43-
return Token::Match(type, "char|short|int") && !type->isLong();
44-
}
45-
46-
static bool astGetSizeSign(const Settings *settings, const Token *tok, unsigned int *size, char *sign)
47-
{
48-
if (!tok)
49-
return false;
50-
if (tok->isArithmeticalOp()) {
51-
if (!astGetSizeSign(settings, tok->astOperand1(), size, sign))
52-
return false;
53-
return !tok->astOperand2() || astGetSizeSign(settings, tok->astOperand2(), size, sign);
54-
}
55-
if (tok->isNumber() && MathLib::isInt(tok->str())) {
56-
if (tok->str().find('L') != std::string::npos)
57-
return false;
58-
MathLib::bigint value = MathLib::toLongNumber(tok->str());
59-
unsigned int sz;
60-
if (value >= -(1<<7) && value <= (1<<7)-1)
61-
sz = 8;
62-
else if (value >= -(1<<15) && value <= (1<<15)-1)
63-
sz = 16;
64-
else if (value >= -(1LL<<31) && value <= (1LL<<31)-1)
65-
sz = 32;
66-
else
67-
return false;
68-
if (sz < 8 * settings->sizeof_int)
69-
sz = 8 * settings->sizeof_int;
70-
if (*size < sz)
71-
*size = sz;
72-
if (tok->str().find('U') != std::string::npos)
73-
*sign = 'u';
74-
if (*sign != 'u')
75-
*sign = 's';
76-
return true;
77-
}
78-
if (tok->isName()) {
79-
const Variable *var = tok->variable();
80-
if (!var)
81-
return false;
82-
unsigned int sz = 0;
83-
for (const Token *type = var->typeStartToken(); type; type = type->next()) {
84-
if (type->str() == "*")
85-
return false; // <- FIXME: handle pointers
86-
if (Token::Match(type, "char|short|int")) {
87-
sz = 8 * settings->sizeof_int;
88-
if (type->isUnsigned())
89-
*sign = 'u';
90-
else if (*sign != 'u')
91-
*sign = 's';
92-
} else if (Token::Match(type, "float|double|long")) {
93-
return false;
94-
} else {
95-
// TODO: try to lookup type info in library
96-
}
97-
if (type == var->typeEndToken())
98-
break;
99-
}
100-
if (sz == 0)
101-
return false;
102-
if (*size < sz)
103-
*size = sz;
104-
return true;
105-
}
106-
return false;
107-
}
108-
10933
//---------------------------------------------------------------------------
11034
// Checking for shift by too many bits
11135
//---------------------------------------------------------------------------
@@ -124,7 +48,7 @@ void CheckType::checkTooBigBitwiseShift()
12448
for (std::size_t i = 0; i < functions; ++i) {
12549
const Scope * scope = symbolDatabase->functionScopes[i];
12650
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
127-
if (tok->str() != "<<" && tok->str() != ">>")
51+
if (!Token::Match(tok, "<<|>>|<<=|>>="))
12852
continue;
12953

13054
if (!tok->astOperand1() || !tok->astOperand2())
@@ -198,30 +122,29 @@ void CheckType::checkIntegerOverflow()
198122
if (!value)
199123
continue;
200124

201-
// get size and sign of result..
202-
unsigned int size = 0;
203-
char sign = 0;
204-
if (!astGetSizeSign(_settings, tok, &size, &sign))
205-
continue;
206-
if (sign != 's') // only signed integer overflow is UB
207-
continue;
208-
209-
integerOverflowError(tok, *value);
125+
// is result signed integer?
126+
const ValueType *vt = tok->valueType();
127+
if (vt && vt->type == ValueType::Type::INT && vt->sign == ValueType::Sign::SIGNED)
128+
integerOverflowError(tok, *value);
210129
}
211130
}
212131
}
213132

214133
void CheckType::integerOverflowError(const Token *tok, const ValueFlow::Value &value)
215134
{
216135
const std::string expr(tok ? tok->expressionString() : "");
217-
const std::string cond(value.condition ?
218-
". See condition at line " + MathLib::toString(value.condition->linenr()) + "." :
219-
"");
136+
137+
std::string msg;
138+
if (value.condition)
139+
msg = ValueFlow::eitherTheConditionIsRedundant(value.condition) +
140+
" or there is signed integer overflow for expression '" + expr + "'.";
141+
else
142+
msg = "Signed integer overflow for expression '" + expr + "'.";
220143

221144
reportError(tok,
222145
value.condition ? Severity::warning : Severity::error,
223146
"integerOverflow",
224-
"Signed integer overflow for expression '"+expr+"'"+cond,
147+
msg,
225148
0U,
226149
value.inconclusive);
227150
}
@@ -243,15 +166,11 @@ void CheckType::checkSignConversion()
243166
if (!tok->isArithmeticalOp() || Token::Match(tok,"+|-"))
244167
continue;
245168

246-
unsigned int size = 0;
247-
char sign = 0;
248-
if (!astGetSizeSign(_settings, tok, &size, &sign))
169+
// Is result unsigned?
170+
if (!(tok->valueType() && tok->valueType()->sign == ValueType::Sign::UNSIGNED))
249171
continue;
250172

251-
if (sign != 'u')
252-
continue;
253-
254-
// Check if there are signed operands that can be negative..
173+
// Check if an operand can be negative..
255174
std::stack<const Token *> tokens;
256175
tokens.push(tok->astOperand1());
257176
tokens.push(tok->astOperand2());
@@ -260,30 +179,10 @@ void CheckType::checkSignConversion()
260179
tokens.pop();
261180
if (!tok1)
262181
continue;
263-
if (tok1->str() == "(")
264-
continue; // Todo: properly handle casts, function calls, etc
265-
const Variable *var = tok1->variable();
266-
if (var && tok1->getValueLE(-1,_settings)) {
267-
bool signedvar = true; // assume that variable is signed since it can have a negative value
268-
for (const Token *type = var->typeStartToken();; type = type->next()) {
269-
if (type->isUnsigned()) {
270-
signedvar = false;
271-
break;
272-
}
273-
if (type->isSigned())
274-
break;
275-
if (type->isName() && !Token::Match(type, "char|short|int|long|const")) {
276-
signedvar = false;
277-
break;
278-
}
279-
if (type == var->typeEndToken())
280-
break;
281-
}
282-
if (signedvar) {
283-
signConversionError(tok1);
284-
break;
285-
}
286-
}
182+
if (!tok1->getValueLE(-1,_settings))
183+
continue;
184+
if (tok1->valueType() && tok1->valueType()->sign != ValueType::Sign::UNSIGNED)
185+
signConversionError(tok1);
287186
}
288187
}
289188
}
@@ -311,13 +210,23 @@ void CheckType::checkLongCast()
311210

312211
// Assignments..
313212
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
314-
if (!Token::Match(tok, "%var% ="))
315-
continue;
316-
if (!tok->variable() || !tok->variable()->isConst() || tok->variable()->typeStartToken()->str() != "long")
213+
if (tok->str() != "=" || !Token::Match(tok->astOperand2(), "*|<<"))
317214
continue;
318-
if (!tok->variable()->typeStartToken()->originalName().empty())
215+
216+
const ValueType *lhstype = tok->astOperand1() ? tok->astOperand1()->valueType() : nullptr;
217+
const ValueType *rhstype = tok->astOperand2()->valueType();
218+
219+
if (!lhstype || !rhstype)
319220
continue;
320-
if (Token::Match(tok->next()->astOperand2(), "*|<<") && astIsIntResult(tok->next()->astOperand2()))
221+
222+
// assign int result to long/longlong const nonpointer?
223+
if (rhstype->type == ValueType::Type::INT &&
224+
rhstype->pointer == 0U &&
225+
rhstype->originalTypeName.empty() &&
226+
(lhstype->type == ValueType::Type::LONG || lhstype->type == ValueType::Type::LONGLONG) &&
227+
lhstype->pointer == 0U &&
228+
lhstype->constness == 1U &&
229+
lhstype->originalTypeName.empty())
321230
longCastAssignError(tok);
322231
}
323232

@@ -340,21 +249,24 @@ void CheckType::checkLongCast()
340249
if (!islong)
341250
continue;
342251

343-
// find return statement
344-
// todo.. this is slow, we should only check last statement in each child scope
252+
// return statements
345253
const Token *ret = nullptr;
346254
for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
347255
if (tok->str() == "return") {
348-
if (!ret)
349-
ret = tok;
350-
else {
351-
ret = nullptr;
256+
if (Token::Match(tok->astOperand1(), "<<|*")) {
257+
const ValueType *type = tok->astOperand1()->valueType();
258+
if (type->type == ValueType::Type::INT && type->pointer == 0U && type->originalTypeName.empty())
259+
ret = tok;
260+
}
261+
// All return statements must have problem otherwise no warning
262+
if (ret != tok) {
263+
ret = nullptr;
352264
break;
353265
}
354266
}
355267
}
356268

357-
if (ret && Token::Match(ret->astOperand1(), "*|<<") && astIsIntResult(ret->astOperand1()))
269+
if (ret)
358270
longCastReturnError(ret);
359271
}
360272
}

‎test/testtype.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,20 +99,21 @@ class TestType : public TestFixture {
9999
void checkIntegerOverflow() {
100100
Settings settings;
101101
settings.platform(Settings::Unix32);
102+
settings.addEnabled("warning");
102103

103-
check("int foo(int x) {\n"
104+
check("int foo(signed int x) {\n"
104105
" if (x==123456) {}\n"
105106
" return x * x;\n"
106107
"}",&settings);
107-
ASSERT_EQUALS("[test.cpp:3]: (warning) Signed integer overflow for expression 'x*x'. See condition at line 2.\n", errout.str());
108+
ASSERT_EQUALS("[test.cpp:3]: (warning) Either the condition 'x==123456' is redundant or there is signed integer overflow for expression 'x*x'.\n", errout.str());
108109

109-
check("int foo(int x) {\n"
110+
check("int foo(signed int x) {\n"
110111
" if (x==123456) {}\n"
111112
" return -123456 * x;\n"
112113
"}",&settings);
113-
ASSERT_EQUALS("[test.cpp:3]: (warning) Signed integer overflow for expression '-123456*x'. See condition at line 2.\n", errout.str());
114+
ASSERT_EQUALS("[test.cpp:3]: (warning) Either the condition 'x==123456' is redundant or there is signed integer overflow for expression '-123456*x'.\n", errout.str());
114115

115-
check("int foo(int x) {\n"
116+
check("int foo(signed int x) {\n"
116117
" if (x==123456) {}\n"
117118
" return 123456U * x;\n"
118119
"}",&settings);

0 commit comments

Comments
 (0)