#7183 CheckClass::checkMemset() uint overflow. Plus some minor refactoring

This commit is contained in:
Alexander Mai 2015-12-05 18:22:01 +01:00
parent 6ae1533cb4
commit e69377d5a8
5 changed files with 78 additions and 69 deletions

View File

@ -987,7 +987,6 @@ static const Scope* findFunctionOf(const Scope* scope)
void CheckClass::checkMemset() void CheckClass::checkMemset()
{ {
const bool printWarnings = _settings->isEnabled("warning"); const bool printWarnings = _settings->isEnabled("warning");
const std::size_t functions = symbolDatabase->functionScopes.size(); const std::size_t functions = symbolDatabase->functionScopes.size();
for (std::size_t i = 0; i < functions; ++i) { for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i]; const Scope * scope = symbolDatabase->functionScopes[i];
@ -1014,7 +1013,7 @@ void CheckClass::checkMemset()
else if (Token::simpleMatch(arg3, "sizeof ( * this ) )") || Token::simpleMatch(arg1, "this ,")) { else if (Token::simpleMatch(arg3, "sizeof ( * this ) )") || Token::simpleMatch(arg1, "this ,")) {
type = findFunctionOf(arg3->scope()); type = findFunctionOf(arg3->scope());
} else if (Token::Match(arg1, "&|*|%var%")) { } else if (Token::Match(arg1, "&|*|%var%")) {
std::size_t numIndirToVariableType = 0; // Offset to the actual type in terms of dereference/addressof int numIndirToVariableType = 0; // Offset to the actual type in terms of dereference/addressof
for (;; arg1 = arg1->next()) { for (;; arg1 = arg1->next()) {
if (arg1->str() == "&") if (arg1->str() == "&")
++numIndirToVariableType; ++numIndirToVariableType;
@ -1024,7 +1023,7 @@ void CheckClass::checkMemset()
break; break;
} }
const Variable *var = arg1->variable(); const Variable * const var = arg1->variable();
if (var && arg1->strAt(1) == ",") { if (var && arg1->strAt(1) == ",") {
if (var->isArrayOrPointer()) { if (var->isArrayOrPointer()) {
const Token *endTok = var->typeEndToken(); const Token *endTok = var->typeEndToken();
@ -1035,7 +1034,7 @@ void CheckClass::checkMemset()
} }
if (var->isArray()) if (var->isArray())
numIndirToVariableType += var->dimensions().size(); numIndirToVariableType += int(var->dimensions().size());
if (numIndirToVariableType == 1) if (numIndirToVariableType == 1)
type = var->typeScope(); type = var->typeScope();
@ -1069,13 +1068,13 @@ void CheckClass::checkMemset()
void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation, std::list<const Scope *> parsedTypes) void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation, std::list<const Scope *> parsedTypes)
{ {
const bool printPortability = _settings->isEnabled("portability");
// If type has been checked there is no need to check it again // If type has been checked there is no need to check it again
if (std::find(parsedTypes.begin(), parsedTypes.end(), type) != parsedTypes.end()) if (std::find(parsedTypes.begin(), parsedTypes.end(), type) != parsedTypes.end())
return; return;
parsedTypes.push_back(type); parsedTypes.push_back(type);
const bool printPortability = _settings->isEnabled("portability");
// recursively check all parent classes // recursively check all parent classes
for (std::size_t i = 0; i < type->definedType->derivedFrom.size(); i++) { for (std::size_t i = 0; i < type->definedType->derivedFrom.size(); i++) {
const Type* derivedFrom = type->definedType->derivedFrom[i].type; const Type* derivedFrom = type->definedType->derivedFrom[i].type;
@ -1862,7 +1861,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
continue; continue;
if (tok1->str() == "this" && tok1->previous()->isAssignmentOp()) if (tok1->str() == "this" && tok1->previous()->isAssignmentOp())
return (false); return false;
const Token* lhs = tok1->previous(); const Token* lhs = tok1->previous();
@ -1914,22 +1913,22 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
&& (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) && (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy")))
; ;
else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end)) else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end))
return (false); return false;
} }
// Assignment // Assignment
else if (end->next()->tokType() == Token::eAssignmentOp) else if (end->next()->tokType() == Token::eAssignmentOp)
return (false); return false;
// Streaming // Streaming
else if (end->strAt(1) == "<<" && tok1->strAt(-1) != "<<") else if (end->strAt(1) == "<<" && tok1->strAt(-1) != "<<")
return (false); return false;
else if (tok1->strAt(-1) == ">>") else if (tok1->strAt(-1) == ">>")
return (false); return false;
// ++/-- // ++/--
else if (end->next()->tokType() == Token::eIncDecOp || tok1->previous()->tokType() == Token::eIncDecOp) else if (end->next()->tokType() == Token::eIncDecOp || tok1->previous()->tokType() == Token::eIncDecOp)
return (false); return false;
const Token* start = tok1; const Token* start = tok1;
@ -1937,7 +1936,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
tok1 = tok1->linkAt(-1); tok1 = tok1->linkAt(-1);
if (start->strAt(-1) == "delete") if (start->strAt(-1) == "delete")
return (false); return false;
tok1 = jumpBackToken?jumpBackToken:end; // Jump back to first [ to check inside, or jump to end of expression tok1 = jumpBackToken?jumpBackToken:end; // Jump back to first [ to check inside, or jump to end of expression
} }
@ -1947,7 +1946,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
isMemberVar(scope, tok1->tokAt(-2))) { isMemberVar(scope, tok1->tokAt(-2))) {
const Variable* var = tok1->tokAt(-2)->variable(); const Variable* var = tok1->tokAt(-2)->variable();
if (!var || !var->isMutable()) if (!var || !var->isMutable())
return (false); return false;
} }
@ -1956,7 +1955,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
!Token::Match(tok1, "return|if|string|switch|while|catch|for")) { !Token::Match(tok1, "return|if|string|switch|while|catch|for")) {
if (isMemberFunc(scope, tok1) && tok1->strAt(-1) != ".") { if (isMemberFunc(scope, tok1) && tok1->strAt(-1) != ".") {
if (!isConstMemberFunc(scope, tok1)) if (!isConstMemberFunc(scope, tok1))
return (false); return false;
memberAccessed = true; memberAccessed = true;
} }
// Member variable given as parameter // Member variable given as parameter
@ -1966,15 +1965,15 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
else if (tok2->isName() && isMemberVar(scope, tok2)) { else if (tok2->isName() && isMemberVar(scope, tok2)) {
const Variable* var = tok2->variable(); const Variable* var = tok2->variable();
if (!var || !var->isMutable()) if (!var || !var->isMutable())
return (false); // TODO: Only bailout if function takes argument as non-const reference return false; // TODO: Only bailout if function takes argument as non-const reference
} }
} }
} else if (Token::simpleMatch(tok1, "> (") && (!tok1->link() || !Token::Match(tok1->link()->previous(), "static_cast|const_cast|dynamic_cast|reinterpret_cast"))) { } else if (Token::simpleMatch(tok1, "> (") && (!tok1->link() || !Token::Match(tok1->link()->previous(), "static_cast|const_cast|dynamic_cast|reinterpret_cast"))) {
return (false); return false;
} }
} }
return (true); return true;
} }
void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic) void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic)

View File

@ -36,47 +36,47 @@ namespace {
CheckMemoryLeakInClass instance2; CheckMemoryLeakInClass instance2;
CheckMemoryLeakStructMember instance3; CheckMemoryLeakStructMember instance3;
CheckMemoryLeakNoVar instance4; CheckMemoryLeakNoVar instance4;
/**
* Count function parameters
* \param tok Function name token before the '('
*/
unsigned int countParameters(const Token *tok)
{
tok = tok->tokAt(2);
if (tok->str() == ")")
return 0;
unsigned int numpar = 1;
while (nullptr != (tok = tok->nextArgument()))
numpar++;
return numpar;
}
/** List of functions that can be ignored when searching for memory leaks.
* These functions don't take the address of the given pointer
* This list contains function names with const parameters e.g.: atof(const char *)
* TODO: This list should be replaced by <leak-ignore/> in .cfg files.
*/
const std::set<std::string> call_func_white_list = make_container < std::set<std::string> > ()
<< "_open" << "_wopen" << "access" << "adjtime" << "asctime_r" << "asprintf" << "chdir" << "chmod" << "chown"
<< "creat" << "ctime_r" << "execl" << "execle" << "execlp" << "execv" << "execve" << "fchmod" << "fcntl"
<< "fdatasync" << "fclose" << "flock" << "fmemopen" << "fnmatch" << "fopen" << "fopencookie" << "for" << "free"
<< "freopen"<< "fseeko" << "fstat" << "fsync" << "ftello" << "ftruncate" << "getgrnam" << "gethostbyaddr" << "gethostbyname"
<< "getnetbyname" << "getopt" << "getopt_long" << "getprotobyname" << "getpwnam" << "getservbyname" << "getservbyport"
<< "glob" << "gmtime" << "gmtime_r" << "if" << "index" << "inet_addr" << "inet_aton" << "inet_network" << "initgroups"
<< "ioctl" << "link" << "localtime_r" << "lockf" << "lseek" << "lstat" << "mkdir" << "mkfifo" << "mknod" << "mkstemp"
<< "obstack_printf" << "obstack_vprintf" << "open" << "opendir" << "parse_printf_format" << "pathconf"
<< "perror" << "popen" << "posix_fadvise" << "posix_fallocate" << "pread" << "psignal" << "pwrite" << "read" << "readahead"
<< "readdir" << "readdir_r" << "readlink" << "readv" << "realloc" << "regcomp" << "return" << "rewinddir" << "rindex"
<< "rmdir" << "scandir" << "seekdir" << "setbuffer" << "sethostname" << "setlinebuf" << "sizeof" << "strdup"
<< "stat" << "stpcpy" << "strcasecmp" << "stricmp" << "strncasecmp" << "switch"
<< "symlink" << "sync_file_range" << "telldir" << "tempnam" << "time" << "typeid" << "unlink"
<< "utime" << "utimes" << "vasprintf" << "while" << "wordexp" << "write" << "writev";
} }
/**
* Count function parameters
* \param tok Function name token before the '('
*/
static unsigned int countParameters(const Token *tok)
{
tok = tok->tokAt(2);
if (tok->str() == ")")
return 0;
unsigned int numpar = 1;
while (nullptr != (tok = tok->nextArgument()))
numpar++;
return numpar;
}
/** List of functions that can be ignored when searching for memory leaks.
* These functions don't take the address of the given pointer
* This list contains function names with const parameters e.g.: atof(const char *)
* TODO: This list should be replaced by <leak-ignore/> in .cfg files.
*/
static const std::set<std::string> call_func_white_list = make_container < std::set<std::string> > ()
<< "_open" << "_wopen" << "access" << "adjtime" << "asctime_r" << "asprintf" << "chdir" << "chmod" << "chown"
<< "creat" << "ctime_r" << "execl" << "execle" << "execlp" << "execv" << "execve" << "fchmod" << "fcntl"
<< "fdatasync" << "fclose" << "flock" << "fmemopen" << "fnmatch" << "fopen" << "fopencookie" << "for" << "free"
<< "freopen"<< "fseeko" << "fstat" << "fsync" << "ftello" << "ftruncate" << "getgrnam" << "gethostbyaddr" << "gethostbyname"
<< "getnetbyname" << "getopt" << "getopt_long" << "getprotobyname" << "getpwnam" << "getservbyname" << "getservbyport"
<< "glob" << "gmtime" << "gmtime_r" << "if" << "index" << "inet_addr" << "inet_aton" << "inet_network" << "initgroups"
<< "ioctl" << "link" << "localtime_r" << "lockf" << "lseek" << "lstat" << "mkdir" << "mkfifo" << "mknod" << "mkstemp"
<< "obstack_printf" << "obstack_vprintf" << "open" << "opendir" << "parse_printf_format" << "pathconf"
<< "perror" << "popen" << "posix_fadvise" << "posix_fallocate" << "pread" << "psignal" << "pwrite" << "read" << "readahead"
<< "readdir" << "readdir_r" << "readlink" << "readv" << "realloc" << "regcomp" << "return" << "rewinddir" << "rindex"
<< "rmdir" << "scandir" << "seekdir" << "setbuffer" << "sethostname" << "setlinebuf" << "sizeof" << "strdup"
<< "stat" << "stpcpy" << "strcasecmp" << "stricmp" << "strncasecmp" << "switch"
<< "symlink" << "sync_file_range" << "telldir" << "tempnam" << "time" << "typeid" << "unlink"
<< "utime" << "utimes" << "vasprintf" << "while" << "wordexp" << "write" << "writev";
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
bool CheckMemoryLeak::isclass(const Token *tok, unsigned int varid) const bool CheckMemoryLeak::isclass(const Token *tok, unsigned int varid) const

View File

@ -61,18 +61,18 @@ Settings::Settings()
} }
namespace { namespace {
static const std::set<std::string> id = make_container< std::set<std::string> > () const std::set<std::string> id = make_container< std::set<std::string> > ()
<< "warning" << "warning"
<< "style" << "style"
<< "performance" << "performance"
<< "portability" << "portability"
<< "information" << "information"
<< "missingInclude" << "missingInclude"
<< "unusedFunction" << "unusedFunction"
#ifdef CHECK_INTERNAL #ifdef CHECK_INTERNAL
<< "internal" << "internal"
#endif #endif
; ;
} }
std::string Settings::addEnabled(const std::string &str) std::string Settings::addEnabled(const std::string &str)
{ {

View File

@ -114,8 +114,8 @@ void Token::update_property_info()
} }
namespace { namespace {
static const std::set<std::string> stdTypes = make_container<std::set<std::string> >() << const std::set<std::string> stdTypes = make_container<std::set<std::string> >() <<
"bool" << "char" << "char16_t" << "char32_t" << "double" << "float" << "int" << "long" << "short" << "size_t" << "void" << "wchar_t"; "bool" << "char" << "char16_t" << "char32_t" << "double" << "float" << "int" << "long" << "short" << "size_t" << "void" << "wchar_t";
} }
void Token::update_property_isStandardType() void Token::update_property_isStandardType()

View File

@ -78,6 +78,7 @@ private:
TEST_CASE(memsetOnInvalid); // Ticket #5425: Crash upon invalid TEST_CASE(memsetOnInvalid); // Ticket #5425: Crash upon invalid
TEST_CASE(memsetOnStdPodType); // Ticket #5901 - std::uint8_t TEST_CASE(memsetOnStdPodType); // Ticket #5901 - std::uint8_t
TEST_CASE(memsetOnFloat); // Ticket #5421 TEST_CASE(memsetOnFloat); // Ticket #5421
TEST_CASE(memsetOnUnknown); // Ticket #7183
TEST_CASE(mallocOnClass); TEST_CASE(mallocOnClass);
TEST_CASE(this_subtraction); // warn about "this-x" TEST_CASE(this_subtraction); // warn about "this-x"
@ -2259,6 +2260,7 @@ private:
" memset(&fred, 0, sizeof(fred));\n" " memset(&fred, 0, sizeof(fred));\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("", errout.str());
checkNoMemset("class Fred\n" checkNoMemset("class Fred\n"
"{\n" "{\n"
@ -2689,6 +2691,14 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void memsetOnUnknown() {
checkNoMemset("void clang_tokenize(CXToken **Tokens) {\n"
" *Tokens = (CXToken *)malloc(sizeof(CXToken) * CXTokens.size());\n"
" memmove(*Tokens, CXTokens.data(), sizeof(CXToken) * CXTokens.size());\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void mallocOnClass() { void mallocOnClass() {
checkNoMemset("class C { C() {} };\n" checkNoMemset("class C { C() {} };\n"
"void foo(C*& p) {\n" "void foo(C*& p) {\n"