walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Refactoring CheckType checkers. Use ValueType.

This commit is contained in:
Daniel Marjamäki 2015-12-31 12:05:23 +01:00
parent 5dc42ccd49
commit 9f6890512c
2 changed files with 51 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

170
lib/checktype.cpp
View File

@ -30,82 +30,6 @@ namespace {
CheckType instance;
}
static bool astIsIntResult(const Token *tok)
{
if (!tok)
return false;
if (tok->astOperand1())
return astIsIntResult(tok->astOperand1()) && astIsIntResult(tok->astOperand2());
// todo: handle numbers
if (!tok->variable())
return false;
const Token *type = tok->variable()->typeStartToken();
return Token::Match(type, "char|short|int") && !type->isLong();
}
static bool astGetSizeSign(const Settings *settings, const Token *tok, unsigned int *size, char *sign)
{
if (!tok)
return false;
if (tok->isArithmeticalOp()) {
if (!astGetSizeSign(settings, tok->astOperand1(), size, sign))
return false;
return !tok->astOperand2() || astGetSizeSign(settings, tok->astOperand2(), size, sign);
}
if (tok->isNumber() && MathLib::isInt(tok->str())) {
if (tok->str().find('L') != std::string::npos)
return false;
MathLib::bigint value = MathLib::toLongNumber(tok->str());
unsigned int sz;
if (value >= -(1<<7) && value <= (1<<7)-1)
sz = 8;
else if (value >= -(1<<15) && value <= (1<<15)-1)
sz = 16;
else if (value >= -(1LL<<31) && value <= (1LL<<31)-1)
sz = 32;
else
return false;
if (sz < 8 * settings->sizeof_int)
sz = 8 * settings->sizeof_int;
if (*size < sz)
*size = sz;
if (tok->str().find('U') != std::string::npos)
*sign = 'u';
if (*sign != 'u')
*sign = 's';
return true;
}
if (tok->isName()) {
const Variable *var = tok->variable();
if (!var)
return false;
unsigned int sz = 0;
for (const Token *type = var->typeStartToken(); type; type = type->next()) {
if (type->str() == "*")
return false; // <- FIXME: handle pointers
if (Token::Match(type, "char|short|int")) {
sz = 8 * settings->sizeof_int;
if (type->isUnsigned())
*sign = 'u';
else if (*sign != 'u')
*sign = 's';
} else if (Token::Match(type, "float|double|long")) {
return false;
} else {
// TODO: try to lookup type info in library
}
if (type == var->typeEndToken())
break;
}
if (sz == 0)
return false;
if (*size < sz)
*size = sz;
return true;
}
return false;
}
//---------------------------------------------------------------------------
// Checking for shift by too many bits
//---------------------------------------------------------------------------
@ -124,7 +48,7 @@ void CheckType::checkTooBigBitwiseShift()
for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i];
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
if (tok->str() != "<<" && tok->str() != ">>")
if (!Token::Match(tok, "<<|>>|<<=|>>="))
continue;
if (!tok->astOperand1() || !tok->astOperand2())
@ -198,14 +122,9 @@ void CheckType::checkIntegerOverflow()
if (!value)
continue;
// get size and sign of result..
unsigned int size = 0;
char sign = 0;
if (!astGetSizeSign(_settings, tok, &size, &sign))
continue;
if (sign != 's') // only signed integer overflow is UB
continue;
// is result signed integer?
const ValueType *vt = tok->valueType();
if (vt && vt->type == ValueType::Type::INT && vt->sign == ValueType::Sign::SIGNED)
integerOverflowError(tok, *value);
}
}
@ -214,14 +133,18 @@ void CheckType::checkIntegerOverflow()
void CheckType::integerOverflowError(const Token *tok, const ValueFlow::Value &value)
{
const std::string expr(tok ? tok->expressionString() : "");
const std::string cond(value.condition ?
". See condition at line " + MathLib::toString(value.condition->linenr()) + "." :
"");
std::string msg;
if (value.condition)
msg = ValueFlow::eitherTheConditionIsRedundant(value.condition) +
" or there is signed integer overflow for expression '" + expr + "'.";
else
msg = "Signed integer overflow for expression '" + expr + "'.";
reportError(tok,
value.condition ? Severity::warning : Severity::error,
"integerOverflow",
"Signed integer overflow for expression '"+expr+"'"+cond,
msg,
0U,
value.inconclusive);
}
@ -243,15 +166,11 @@ void CheckType::checkSignConversion()
if (!tok->isArithmeticalOp() || Token::Match(tok,"+|-"))
continue;
unsigned int size = 0;
char sign = 0;
if (!astGetSizeSign(_settings, tok, &size, &sign))
// Is result unsigned?
if (!(tok->valueType() && tok->valueType()->sign == ValueType::Sign::UNSIGNED))
continue;
if (sign != 'u')
continue;
// Check if there are signed operands that can be negative..
// Check if an operand can be negative..
std::stack<const Token *> tokens;
tokens.push(tok->astOperand1());
tokens.push(tok->astOperand2());
@ -260,30 +179,10 @@ void CheckType::checkSignConversion()
tokens.pop();
if (!tok1)
continue;
if (tok1->str() == "(")
continue; // Todo: properly handle casts, function calls, etc
const Variable *var = tok1->variable();
if (var && tok1->getValueLE(-1,_settings)) {
bool signedvar = true; // assume that variable is signed since it can have a negative value
for (const Token *type = var->typeStartToken();; type = type->next()) {
if (type->isUnsigned()) {
signedvar = false;
break;
}
if (type->isSigned())
break;
if (type->isName() && !Token::Match(type, "char|short|int|long|const")) {
signedvar = false;
break;
}
if (type == var->typeEndToken())
break;
}
if (signedvar) {
if (!tok1->getValueLE(-1,_settings))
continue;
if (tok1->valueType() && tok1->valueType()->sign != ValueType::Sign::UNSIGNED)
signConversionError(tok1);
break;
}
}
}
}
}
@ -311,13 +210,23 @@ void CheckType::checkLongCast()
// Assignments..
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (!Token::Match(tok, "%var% ="))
if (tok->str() != "=" || !Token::Match(tok->astOperand2(), "*|<<"))
continue;
if (!tok->variable() || !tok->variable()->isConst() || tok->variable()->typeStartToken()->str() != "long")
const ValueType *lhstype = tok->astOperand1() ? tok->astOperand1()->valueType() : nullptr;
const ValueType *rhstype = tok->astOperand2()->valueType();
if (!lhstype || !rhstype)
continue;
if (!tok->variable()->typeStartToken()->originalName().empty())
continue;
if (Token::Match(tok->next()->astOperand2(), "*|<<") && astIsIntResult(tok->next()->astOperand2()))
// assign int result to long/longlong const nonpointer?
if (rhstype->type == ValueType::Type::INT &&
rhstype->pointer == 0U &&
rhstype->originalTypeName.empty() &&
(lhstype->type == ValueType::Type::LONG || lhstype->type == ValueType::Type::LONGLONG) &&
lhstype->pointer == 0U &&
lhstype->constness == 1U &&
lhstype->originalTypeName.empty())
longCastAssignError(tok);
}
@ -340,21 +249,24 @@ void CheckType::checkLongCast()
if (!islong)
continue;
// find return statement
// todo.. this is slow, we should only check last statement in each child scope
// return statements
const Token *ret = nullptr;
for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
if (tok->str() == "return") {
if (!ret)
if (Token::Match(tok->astOperand1(), "<<|*")) {
const ValueType *type = tok->astOperand1()->valueType();
if (type->type == ValueType::Type::INT && type->pointer == 0U && type->originalTypeName.empty())
ret = tok;
else {
}
// All return statements must have problem otherwise no warning
if (ret != tok) {
ret = nullptr;
break;
}
}
}
if (ret && Token::Match(ret->astOperand1(), "*|<<") && astIsIntResult(ret->astOperand1()))
if (ret)
longCastReturnError(ret);
}
}

11
test/testtype.cpp
View File

@ -99,20 +99,21 @@ private:
void checkIntegerOverflow() {
Settings settings;
settings.platform(Settings::Unix32);
settings.addEnabled("warning");
check("int foo(int x) {\n"
check("int foo(signed int x) {\n"
" if (x==123456) {}\n"
" return x * x;\n"
"}",&settings);
ASSERT_EQUALS("[test.cpp:3]: (warning) Signed integer overflow for expression 'x*x'. See condition at line 2.\n", errout.str());
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());
check("int foo(int x) {\n"
check("int foo(signed int x) {\n"
" if (x==123456) {}\n"
" return -123456 * x;\n"
"}",&settings);
ASSERT_EQUALS("[test.cpp:3]: (warning) Signed integer overflow for expression '-123456*x'. See condition at line 2.\n", errout.str());
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());
check("int foo(int x) {\n"
check("int foo(signed int x) {\n"
" if (x==123456) {}\n"
" return 123456U * x;\n"
"}",&settings);