CheckOther: Removed non-valueflow check for division by possible zero before condition
This commit is contained in:
Daniel Marjamäki
parent
0203a4a6f5
commit
3facc06cb7
|
|
@ -2208,137 +2208,6 @@ void CheckOther::zerodivError(const Token *tok)
|
||||||
reportError(tok, Severity::error, "zerodiv", "Division by zero.");
|
reportError(tok, Severity::error, "zerodiv", "Division by zero.");
|
||||||
}
|
}
|
||||||
|
|
||||||
//---------------------------------------------------------------------------
|
|
||||||
void CheckOther::checkZeroDivisionOrUselessCondition()
|
|
||||||
{
|
|
||||||
if (!_settings->isEnabled("warning"))
|
|
||||||
return;
|
|
||||||
|
|
||||||
// Use experimental checking instead based on value flow analysis
|
|
||||||
if (_settings->valueFlow)
|
|
||||||
return;
|
|
||||||
|
|
||||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
|
||||||
const std::size_t numberOfFunctions = symbolDatabase->functionScopes.size();
|
|
||||||
for (std::size_t functionIndex = 0; functionIndex < numberOfFunctions; ++functionIndex) {
|
|
||||||
const Scope * scope = symbolDatabase->functionScopes[functionIndex];
|
|
||||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
|
||||||
if (Token::Match(tok, "[/%] %var% !!.") || Token::Match(tok, "[(,] %var% [,)]")) {
|
|
||||||
const unsigned int varid = tok->next()->varId();
|
|
||||||
const Variable *var = tok->next()->variable();
|
|
||||||
if (!var)
|
|
||||||
continue;
|
|
||||||
bool isVarUnsigned = var->typeEndToken()->isUnsigned();
|
|
||||||
for (const Token *typetok = var->typeStartToken(); typetok; typetok = typetok->next()) {
|
|
||||||
if (!typetok->isName() || typetok == var->typeEndToken())
|
|
||||||
break;
|
|
||||||
if (typetok->isUnsigned()) {
|
|
||||||
isVarUnsigned = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const Token *divtok = tok;
|
|
||||||
|
|
||||||
// Check if variable is divided by function..
|
|
||||||
if (Token::Match(tok, "[(,]")) {
|
|
||||||
const Token *ftok = tok;
|
|
||||||
unsigned int parnum = 0U;
|
|
||||||
while (ftok && ftok->str() != "(") {
|
|
||||||
if (ftok->str() == ")")
|
|
||||||
ftok = ftok->link();
|
|
||||||
else if (ftok->str() == ",")
|
|
||||||
parnum++;
|
|
||||||
ftok = ftok ? ftok->previous() : NULL;
|
|
||||||
}
|
|
||||||
ftok = ftok ? ftok->previous() : NULL;
|
|
||||||
if (!ftok)
|
|
||||||
continue;
|
|
||||||
if (!Token::Match(ftok, "%var% (") && ftok->function())
|
|
||||||
continue;
|
|
||||||
|
|
||||||
const Function * const function = ftok->function();
|
|
||||||
const Variable * const arg = function ? function->getArgumentVar(parnum) : NULL;
|
|
||||||
const Token *argtok = arg ? arg->typeStartToken() : NULL;
|
|
||||||
|
|
||||||
if (!argtok)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
if (argtok->str() == "const")
|
|
||||||
argtok = argtok->next();
|
|
||||||
|
|
||||||
if (!Token::Match(argtok,"%type% %var% ,|)"))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
const Scope * const functionScope = function ? function->functionScope : NULL;
|
|
||||||
if (!functionScope)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
const unsigned int varid2 = argtok->next()->varId();
|
|
||||||
divtok = NULL;
|
|
||||||
bool use = false;
|
|
||||||
for (const Token *tok2 = functionScope->classStart->next(); tok2 != functionScope->classEnd; tok2 = tok2->next()) {
|
|
||||||
if (Token::Match(tok2, "[%/] %varid%", varid2)) {
|
|
||||||
divtok = tok2;
|
|
||||||
tok2 = tok2->next();
|
|
||||||
} else if (tok2->str() != "&" && Token::Match(tok2, "%cop% %varid%",varid2)) {
|
|
||||||
tok2 = tok2->next();
|
|
||||||
} else if (tok2->varId() == varid2 || tok2->str() == "{") {
|
|
||||||
use = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (!divtok || use)
|
|
||||||
continue;
|
|
||||||
} else {
|
|
||||||
// Check if this division is guarded by a ?:
|
|
||||||
bool guard = false;
|
|
||||||
for (const Token *tok2 = divtok; tok2; tok2 = tok2->previous()) {
|
|
||||||
if (Token::Match(tok2, "[,;{}]"))
|
|
||||||
break;
|
|
||||||
if (Token::Match(tok2, "[?:]")) {
|
|
||||||
guard = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
if (tok2->str() == ")")
|
|
||||||
tok2 = tok2->link();
|
|
||||||
}
|
|
||||||
if (guard)
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Look for if condition
|
|
||||||
const Token *tok2;
|
|
||||||
for (tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) {
|
|
||||||
if (tok2->varId() == varid)
|
|
||||||
break;
|
|
||||||
if (Token::Match(tok2, "{|}"))
|
|
||||||
break;
|
|
||||||
if (Token::Match(tok2, "%var% (") && (var->isGlobal() || !tok2->function()))
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
// Parse if condition
|
|
||||||
if (Token::simpleMatch(tok2, "if (")) {
|
|
||||||
while (NULL != (tok2 = tok2->next())) {
|
|
||||||
if (tok2->str() == "{")
|
|
||||||
break;
|
|
||||||
if (isVarUnsigned && Token::Match(tok2, "(|%oror%|&& 0 < %varid% &&|%oror%|)", varid))
|
|
||||||
zerodivcondError(tok2,divtok);
|
|
||||||
else if (isVarUnsigned && Token::Match(tok2, "(|%oror%|&& 1 <= %varid% &&|%oror%|)", varid))
|
|
||||||
zerodivcondError(tok2,divtok);
|
|
||||||
else if (Token::Match(tok2, "%var% ("))
|
|
||||||
// Todo: continue looking in condition unless variable might be
|
|
||||||
// changed by the function
|
|
||||||
break;
|
|
||||||
else if (Token::Match(tok2, "(|%oror%|&& !| %varid% &&|%oror%|)", varid))
|
|
||||||
zerodivcondError(tok2,divtok);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TODO: this utility function should probably be moved to some common file
|
// TODO: this utility function should probably be moved to some common file
|
||||||
static std::string astStringify(const Token *top)
|
static std::string astStringify(const Token *top)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -90,7 +90,6 @@ public:
|
||||||
|
|
||||||
checkOther.invalidFunctionUsage();
|
checkOther.invalidFunctionUsage();
|
||||||
checkOther.checkZeroDivision();
|
checkOther.checkZeroDivision();
|
||||||
checkOther.checkZeroDivisionOrUselessCondition();
|
|
||||||
checkOther.checkMathFunctions();
|
checkOther.checkMathFunctions();
|
||||||
|
|
||||||
checkOther.redundantGetAndSetUserId();
|
checkOther.redundantGetAndSetUserId();
|
||||||
|
|
|
||||||
|
|
@ -549,49 +549,6 @@ private:
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
// ticket #5045 segmentation fault when SymbolDatabase is corrupt
|
|
||||||
{
|
|
||||||
// We don't use the "check" function because we need to
|
|
||||||
// make sure the symboldatabase is inconsistent..
|
|
||||||
|
|
||||||
const char code[] = "namespace {\n"
|
|
||||||
" void get() { source = create(context); }\n"
|
|
||||||
" void create( something const & context)\n"
|
|
||||||
" SAL_THROW((css::uno::Exception))\n"
|
|
||||||
" { return new Server(context); }\n"
|
|
||||||
"}\n"
|
|
||||||
"void component_getFactory()\n"
|
|
||||||
"{ component_getFactoryHelper(); }";
|
|
||||||
|
|
||||||
Settings settings;
|
|
||||||
settings.addEnabled("warning");
|
|
||||||
Tokenizer tokenizer(&settings, this);
|
|
||||||
std::istringstream istr(code);
|
|
||||||
tokenizer.tokenize(istr,"test.cpp");
|
|
||||||
tokenizer.simplifyTokenList2();
|
|
||||||
|
|
||||||
// Assert that the symboldatabase is inconsistent..
|
|
||||||
const SymbolDatabase *symbolDatabase = tokenizer.getSymbolDatabase();
|
|
||||||
ASSERT_EQUALS(2U, symbolDatabase->getVariableListSize());
|
|
||||||
const Variable *var = symbolDatabase->getVariableFromVarId(1U);
|
|
||||||
ASSERT(!!var->typeStartToken());
|
|
||||||
bool invalid = true;
|
|
||||||
for (const Token *tok = var->typeStartToken(); tok; tok = tok->next()) {
|
|
||||||
invalid = true;
|
|
||||||
if (tok == var->typeEndToken()) {
|
|
||||||
invalid = false;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
ASSERT_EQUALS(true, invalid);
|
|
||||||
|
|
||||||
// Make sure there is no crash with inconsistent symboldatabase..
|
|
||||||
// typeStartToken() is not before typeEndToken()
|
|
||||||
errout.str("");
|
|
||||||
CheckOther checkOther(&tokenizer, &settings, this);
|
|
||||||
checkOther.checkZeroDivisionOrUselessCondition(); // don't crash
|
|
||||||
}
|
|
||||||
|
|
||||||
// #5105 - FP
|
// #5105 - FP
|
||||||
check("int f(int a, int b) {\n"
|
check("int f(int a, int b) {\n"
|
||||||
" int r = a / b;\n"
|
" int r = a / b;\n"
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue