From e69377d5a8efc045438341962c6fb4fd69e26458 Mon Sep 17 00:00:00 2001 From: Alexander Mai Date: Sat, 5 Dec 2015 18:22:01 +0100 Subject: [PATCH] #7183 CheckClass::checkMemset() uint overflow. Plus some minor refactoring --- lib/checkclass.cpp | 35 +++++++++--------- lib/checkmemoryleak.cpp | 78 ++++++++++++++++++++--------------------- lib/settings.cpp | 20 +++++------ lib/token.cpp | 4 +-- test/testclass.cpp | 10 ++++++ 5 files changed, 78 insertions(+), 69 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index b0dd6cb32..da3ff5d74 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -987,7 +987,6 @@ static const Scope* findFunctionOf(const Scope* scope) void CheckClass::checkMemset() { const bool printWarnings = _settings->isEnabled("warning"); - const std::size_t functions = symbolDatabase->functionScopes.size(); for (std::size_t i = 0; i < functions; ++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 ,")) { type = findFunctionOf(arg3->scope()); } 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()) { if (arg1->str() == "&") ++numIndirToVariableType; @@ -1024,7 +1023,7 @@ void CheckClass::checkMemset() break; } - const Variable *var = arg1->variable(); + const Variable * const var = arg1->variable(); if (var && arg1->strAt(1) == ",") { if (var->isArrayOrPointer()) { const Token *endTok = var->typeEndToken(); @@ -1035,7 +1034,7 @@ void CheckClass::checkMemset() } if (var->isArray()) - numIndirToVariableType += var->dimensions().size(); + numIndirToVariableType += int(var->dimensions().size()); if (numIndirToVariableType == 1) 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 parsedTypes) { - const bool printPortability = _settings->isEnabled("portability"); - // If type has been checked there is no need to check it again if (std::find(parsedTypes.begin(), parsedTypes.end(), type) != parsedTypes.end()) return; parsedTypes.push_back(type); + const bool printPortability = _settings->isEnabled("portability"); + // recursively check all parent classes for (std::size_t i = 0; i < type->definedType->derivedFrom.size(); i++) { const Type* derivedFrom = type->definedType->derivedFrom[i].type; @@ -1862,7 +1861,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& continue; if (tok1->str() == "this" && tok1->previous()->isAssignmentOp()) - return (false); + return false; 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"))) ; else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end)) - return (false); + return false; } // Assignment else if (end->next()->tokType() == Token::eAssignmentOp) - return (false); + return false; // Streaming else if (end->strAt(1) == "<<" && tok1->strAt(-1) != "<<") - return (false); + return false; else if (tok1->strAt(-1) == ">>") - return (false); + return false; // ++/-- else if (end->next()->tokType() == Token::eIncDecOp || tok1->previous()->tokType() == Token::eIncDecOp) - return (false); + return false; const Token* start = tok1; @@ -1937,7 +1936,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& tok1 = tok1->linkAt(-1); 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 } @@ -1947,7 +1946,7 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool& isMemberVar(scope, tok1->tokAt(-2))) { const Variable* var = tok1->tokAt(-2)->variable(); 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")) { if (isMemberFunc(scope, tok1) && tok1->strAt(-1) != ".") { if (!isConstMemberFunc(scope, tok1)) - return (false); + return false; memberAccessed = true; } // 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)) { const Variable* var = tok2->variable(); 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"))) { - return (false); + return false; } } - return (true); + return true; } void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic) diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index b8f5efdce..00e1674d9 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -36,47 +36,47 @@ namespace { CheckMemoryLeakInClass instance2; CheckMemoryLeakStructMember instance3; 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 in .cfg files. + */ + const std::set call_func_white_list = make_container < std::set > () + << "_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 in .cfg files. - */ -static const std::set call_func_white_list = make_container < std::set > () - << "_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 diff --git a/lib/settings.cpp b/lib/settings.cpp index 43055d095..e2c711288 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -61,18 +61,18 @@ Settings::Settings() } namespace { - static const std::set id = make_container< std::set > () - << "warning" - << "style" - << "performance" - << "portability" - << "information" - << "missingInclude" - << "unusedFunction" + const std::set id = make_container< std::set > () + << "warning" + << "style" + << "performance" + << "portability" + << "information" + << "missingInclude" + << "unusedFunction" #ifdef CHECK_INTERNAL - << "internal" + << "internal" #endif - ; + ; } std::string Settings::addEnabled(const std::string &str) { diff --git a/lib/token.cpp b/lib/token.cpp index fcb550fd8..39ba323d9 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -114,8 +114,8 @@ void Token::update_property_info() } namespace { - static const std::set stdTypes = make_container >() << - "bool" << "char" << "char16_t" << "char32_t" << "double" << "float" << "int" << "long" << "short" << "size_t" << "void" << "wchar_t"; + const std::set stdTypes = make_container >() << + "bool" << "char" << "char16_t" << "char32_t" << "double" << "float" << "int" << "long" << "short" << "size_t" << "void" << "wchar_t"; } void Token::update_property_isStandardType() diff --git a/test/testclass.cpp b/test/testclass.cpp index 089fd5927..68ecdfff1 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -78,6 +78,7 @@ private: TEST_CASE(memsetOnInvalid); // Ticket #5425: Crash upon invalid TEST_CASE(memsetOnStdPodType); // Ticket #5901 - std::uint8_t TEST_CASE(memsetOnFloat); // Ticket #5421 + TEST_CASE(memsetOnUnknown); // Ticket #7183 TEST_CASE(mallocOnClass); TEST_CASE(this_subtraction); // warn about "this-x" @@ -2259,6 +2260,7 @@ private: " memset(&fred, 0, sizeof(fred));\n" "}"); ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("", errout.str()); checkNoMemset("class Fred\n" "{\n" @@ -2689,6 +2691,14 @@ private: 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() { checkNoMemset("class C { C() {} };\n" "void foo(C*& p) {\n"