Refactorization: Avoid iterations over whole token list, limited several checks to function scopes.
This commit is contained in:
parent
5bc775e43e
commit
662283cab8
|
@ -172,7 +172,7 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2,
|
||||||
return No;
|
return No;
|
||||||
|
|
||||||
// is there a user function with this name?
|
// is there a user function with this name?
|
||||||
if (tokenizer && Token::findmatch(tokenizer->tokens(), ("%type% *|&| " + tok2->str()).c_str()))
|
if (tok2->function())
|
||||||
return No;
|
return No;
|
||||||
return Fd;
|
return Fd;
|
||||||
}
|
}
|
||||||
|
|
|
@ -21,6 +21,7 @@
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
|
||||||
#include "checknonreentrantfunctions.h"
|
#include "checknonreentrantfunctions.h"
|
||||||
|
#include "symboldatabase.h"
|
||||||
|
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
@ -35,30 +36,34 @@ void CheckNonReentrantFunctions::nonReentrantFunctions()
|
||||||
if (!_settings->standards.posix || !_settings->isEnabled("portability"))
|
if (!_settings->standards.posix || !_settings->isEnabled("portability"))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
std::map<std::string,std::string>::const_iterator nonReentrant_end = _nonReentrantFunctions.end();
|
const SymbolDatabase * const symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||||
// Look for function invocations
|
for (std::size_t i = 0; i < functions; ++i) {
|
||||||
if (!tok->isName() || tok->strAt(1) != "(" || tok->varId() != 0)
|
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||||
continue;
|
for (const Token *tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||||
|
// Look for function invocations
|
||||||
// Check for non-reentrant function name
|
if (tok->varId() != 0 || !tok->isName() || tok->strAt(1) != "(")
|
||||||
std::map<std::string,std::string>::const_iterator it = _nonReentrantFunctions.find(tok->str());
|
|
||||||
if (it == nonReentrant_end)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
const Token *prev = tok->previous();
|
|
||||||
if (prev) {
|
|
||||||
// Ignore function definitions, class members or class definitions
|
|
||||||
if (prev->isName() || Token::Match(prev, ".|:"))
|
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
// Check for "std" or global namespace, ignore other namespaces
|
// Check for non-reentrant function name
|
||||||
if (prev->str() == "::" && prev->previous() && prev->previous()->str() != "std" && prev->previous()->isName())
|
std::map<std::string, std::string>::const_iterator it = _nonReentrantFunctions.find(tok->str());
|
||||||
|
if (it == _nonReentrantFunctions.end())
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
|
const Token *prev = tok->previous();
|
||||||
|
if (prev) {
|
||||||
|
// Ignore function definitions, class members or class definitions
|
||||||
|
if (prev->str() == ".")
|
||||||
|
continue;
|
||||||
|
|
||||||
|
// Check for "std" or global namespace, ignore other namespaces
|
||||||
|
if (prev->str() == "::" && prev->previous() && prev->previous()->str() != "std" && prev->previous()->isName())
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Only affecting multi threaded code, therefore this is "portability"
|
||||||
|
reportError(tok, Severity::portability, "nonreentrantFunctions" + it->first, it->second);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Only affecting multi threaded code, therefore this is "portability"
|
|
||||||
reportError(tok, Severity::portability, "nonreentrantFunctions"+it->first, it->second);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
|
|
@ -998,45 +998,49 @@ void CheckOther::checkSuspiciousEqualityComparison()
|
||||||
if (!_settings->isEnabled("warning") || !_settings->inconclusive)
|
if (!_settings->isEnabled("warning") || !_settings->inconclusive)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
|
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||||
|
for (std::size_t i = 0; i < functions; ++i) {
|
||||||
|
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||||
|
for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
|
||||||
|
if (Token::simpleMatch(tok, "for (")) {
|
||||||
|
const Token* const openParen = tok->next();
|
||||||
|
const Token* const closeParen = tok->linkAt(1);
|
||||||
|
|
||||||
if (Token::simpleMatch(tok, "for (")) {
|
// Search for any suspicious equality comparison in the initialization
|
||||||
const Token* const openParen = tok->next();
|
// or increment-decrement parts of the for() loop.
|
||||||
const Token* const closeParen = tok->linkAt(1);
|
// For example:
|
||||||
|
// for (i == 2; i < 10; i++)
|
||||||
|
// or
|
||||||
|
// for (i = 0; i < 10; i == a)
|
||||||
|
if (Token::Match(openParen->next(), "%var% =="))
|
||||||
|
suspiciousEqualityComparisonError(openParen->tokAt(2));
|
||||||
|
if (Token::Match(closeParen->tokAt(-2), "== %any%"))
|
||||||
|
suspiciousEqualityComparisonError(closeParen->tokAt(-2));
|
||||||
|
|
||||||
// Search for any suspicious equality comparison in the initialization
|
// Equality comparisons with 0 are simplified to negation. For instance,
|
||||||
// or increment-decrement parts of the for() loop.
|
// (x == 0) is simplified to (!x), so also check for suspicious negation
|
||||||
// For example:
|
// in the initialization or increment-decrement parts of the for() loop.
|
||||||
// for (i == 2; i < 10; i++)
|
// For example:
|
||||||
// or
|
// for (!i; i < 10; i++)
|
||||||
// for (i = 0; i < 10; i == a)
|
if (Token::Match(openParen->next(), "! %var%"))
|
||||||
if (Token::Match(openParen->next(), "%var% =="))
|
suspiciousEqualityComparisonError(openParen->next());
|
||||||
suspiciousEqualityComparisonError(openParen->tokAt(2));
|
if (Token::Match(closeParen->tokAt(-2), "! %var%"))
|
||||||
if (Token::Match(closeParen->tokAt(-2), "== %any%"))
|
suspiciousEqualityComparisonError(closeParen->tokAt(-2));
|
||||||
suspiciousEqualityComparisonError(closeParen->tokAt(-2));
|
|
||||||
|
|
||||||
// Equality comparisons with 0 are simplified to negation. For instance,
|
// Skip over for() loop conditions because "for (;running==1;)"
|
||||||
// (x == 0) is simplified to (!x), so also check for suspicious negation
|
// is a bit strange, but not necessarily incorrect.
|
||||||
// in the initialization or increment-decrement parts of the for() loop.
|
tok = closeParen;
|
||||||
// For example:
|
} else if (Token::Match(tok, "[;{}] *| %var% == %any% ;")) {
|
||||||
// for (!i; i < 10; i++)
|
|
||||||
if (Token::Match(openParen->next(), "! %var%"))
|
|
||||||
suspiciousEqualityComparisonError(openParen->next());
|
|
||||||
if (Token::Match(closeParen->tokAt(-2), "! %var%"))
|
|
||||||
suspiciousEqualityComparisonError(closeParen->tokAt(-2));
|
|
||||||
|
|
||||||
// Skip over for() loop conditions because "for (;running==1;)"
|
// Exclude compound statements surrounded by parentheses, such as
|
||||||
// is a bit strange, but not necessarily incorrect.
|
// printf("%i\n", ({x==0;}));
|
||||||
tok = closeParen;
|
// because they may appear as an expression in GNU C/C++.
|
||||||
} else if (Token::Match(tok, "[;{}] *| %var% == %any% ;")) {
|
// See http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
|
||||||
|
const Token* afterStatement = tok->strAt(1) == "*" ? tok->tokAt(6) : tok->tokAt(5);
|
||||||
// Exclude compound statements surrounded by parentheses, such as
|
if (!Token::simpleMatch(afterStatement, "} )"))
|
||||||
// printf("%i\n", ({x==0;}));
|
suspiciousEqualityComparisonError(tok->next());
|
||||||
// because they may appear as an expression in GNU C/C++.
|
}
|
||||||
// See http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
|
|
||||||
const Token* afterStatement = tok->strAt(1) == "*" ? tok->tokAt(6) : tok->tokAt(5);
|
|
||||||
if (!Token::simpleMatch(afterStatement, "} )"))
|
|
||||||
suspiciousEqualityComparisonError(tok->next());
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -2027,58 +2031,64 @@ void CheckOther::duplicateBranchError(const Token *tok1, const Token *tok2)
|
||||||
void CheckOther::checkInvalidFree()
|
void CheckOther::checkInvalidFree()
|
||||||
{
|
{
|
||||||
std::map<unsigned int, bool> allocatedVariables;
|
std::map<unsigned int, bool> allocatedVariables;
|
||||||
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
|
||||||
|
|
||||||
// Keep track of which variables were assigned addresses to newly-allocated memory
|
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
if (Token::Match(tok, "%var% = malloc|g_malloc|new")) {
|
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||||
allocatedVariables.insert(std::make_pair(tok->varId(), false));
|
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 a previously-allocated pointer is incremented or decremented, any subsequent
|
// Keep track of which variables were assigned addresses to newly-allocated memory
|
||||||
// free involving pointer arithmetic may or may not be invalid, so we should only
|
if (Token::Match(tok, "%var% = malloc|g_malloc|new")) {
|
||||||
// report an inconclusive result.
|
allocatedVariables.insert(std::make_pair(tok->varId(), false));
|
||||||
else if (Token::Match(tok, "%var% = %var% +|-") &&
|
|
||||||
tok->varId() == tok->tokAt(2)->varId() &&
|
|
||||||
allocatedVariables.find(tok->varId()) != allocatedVariables.end()) {
|
|
||||||
if (_settings->inconclusive)
|
|
||||||
allocatedVariables[tok->varId()] = true;
|
|
||||||
else
|
|
||||||
allocatedVariables.erase(tok->varId());
|
|
||||||
}
|
|
||||||
|
|
||||||
// If a previously-allocated pointer is assigned a completely new value,
|
|
||||||
// we can't know if any subsequent free() on that pointer is valid or not.
|
|
||||||
else if (Token::Match(tok, "%var% =")) {
|
|
||||||
allocatedVariables.erase(tok->varId());
|
|
||||||
}
|
|
||||||
|
|
||||||
// If a variable that was previously assigned a newly-allocated memory location is
|
|
||||||
// added or subtracted from when used to free the memory, report an error.
|
|
||||||
else if (Token::Match(tok, "free|g_free|delete ( %any% +|- %any%") ||
|
|
||||||
Token::Match(tok, "delete [ ] ( %any% +|- %any%") ||
|
|
||||||
Token::Match(tok, "delete %any% +|- %any%")) {
|
|
||||||
|
|
||||||
const int varIndex = tok->strAt(1) == "(" ? 2 :
|
|
||||||
tok->strAt(3) == "(" ? 4 : 1;
|
|
||||||
const unsigned int var1 = tok->tokAt(varIndex)->varId();
|
|
||||||
const unsigned int var2 = tok->tokAt(varIndex + 2)->varId();
|
|
||||||
const std::map<unsigned int, bool>::iterator alloc1 = allocatedVariables.find(var1);
|
|
||||||
const std::map<unsigned int, bool>::iterator alloc2 = allocatedVariables.find(var2);
|
|
||||||
if (alloc1 != allocatedVariables.end()) {
|
|
||||||
invalidFreeError(tok, alloc1->second);
|
|
||||||
} else if (alloc2 != allocatedVariables.end()) {
|
|
||||||
invalidFreeError(tok, alloc2->second);
|
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// If the previously-allocated variable is passed in to another function
|
// If a previously-allocated pointer is incremented or decremented, any subsequent
|
||||||
// as a parameter, it might be modified, so we shouldn't report an error
|
// free involving pointer arithmetic may or may not be invalid, so we should only
|
||||||
// if it is later used to free memory
|
// report an inconclusive result.
|
||||||
else if (Token::Match(tok, "%var% (") && _settings->library.functionpure.find(tok->str()) == _settings->library.functionpure.end()) {
|
else if (Token::Match(tok, "%var% = %var% +|-") &&
|
||||||
const Token* tok2 = Token::findmatch(tok->next(), "%var%", tok->linkAt(1));
|
tok->varId() == tok->tokAt(2)->varId() &&
|
||||||
while (tok2 != nullptr) {
|
allocatedVariables.find(tok->varId()) != allocatedVariables.end()) {
|
||||||
allocatedVariables.erase(tok2->varId());
|
if (_settings->inconclusive)
|
||||||
tok2 = Token::findmatch(tok2->next(), "%var%", tok->linkAt(1));
|
allocatedVariables[tok->varId()] = true;
|
||||||
|
else
|
||||||
|
allocatedVariables.erase(tok->varId());
|
||||||
|
}
|
||||||
|
|
||||||
|
// If a previously-allocated pointer is assigned a completely new value,
|
||||||
|
// we can't know if any subsequent free() on that pointer is valid or not.
|
||||||
|
else if (Token::Match(tok, "%var% =")) {
|
||||||
|
allocatedVariables.erase(tok->varId());
|
||||||
|
}
|
||||||
|
|
||||||
|
// If a variable that was previously assigned a newly-allocated memory location is
|
||||||
|
// added or subtracted from when used to free the memory, report an error.
|
||||||
|
else if (Token::Match(tok, "free|g_free|delete ( %any% +|- %any%") ||
|
||||||
|
Token::Match(tok, "delete [ ] ( %any% +|- %any%") ||
|
||||||
|
Token::Match(tok, "delete %any% +|- %any%")) {
|
||||||
|
|
||||||
|
const int varIndex = tok->strAt(1) == "(" ? 2 :
|
||||||
|
tok->strAt(3) == "(" ? 4 : 1;
|
||||||
|
const unsigned int var1 = tok->tokAt(varIndex)->varId();
|
||||||
|
const unsigned int var2 = tok->tokAt(varIndex + 2)->varId();
|
||||||
|
const std::map<unsigned int, bool>::iterator alloc1 = allocatedVariables.find(var1);
|
||||||
|
const std::map<unsigned int, bool>::iterator alloc2 = allocatedVariables.find(var2);
|
||||||
|
if (alloc1 != allocatedVariables.end()) {
|
||||||
|
invalidFreeError(tok, alloc1->second);
|
||||||
|
} else if (alloc2 != allocatedVariables.end()) {
|
||||||
|
invalidFreeError(tok, alloc2->second);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// If the previously-allocated variable is passed in to another function
|
||||||
|
// as a parameter, it might be modified, so we shouldn't report an error
|
||||||
|
// if it is later used to free memory
|
||||||
|
else if (Token::Match(tok, "%var% (") && _settings->library.functionpure.find(tok->str()) == _settings->library.functionpure.end()) {
|
||||||
|
const Token* tok2 = Token::findmatch(tok->next(), "%var%", tok->linkAt(1));
|
||||||
|
while (tok2 != nullptr) {
|
||||||
|
allocatedVariables.erase(tok2->varId());
|
||||||
|
tok2 = Token::findmatch(tok2->next(), "%var%", tok->linkAt(1));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -2099,92 +2109,97 @@ void CheckOther::checkDoubleFree()
|
||||||
std::set<unsigned int> freedVariables;
|
std::set<unsigned int> freedVariables;
|
||||||
std::set<unsigned int> closeDirVariables;
|
std::set<unsigned int> closeDirVariables;
|
||||||
|
|
||||||
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
// Keep track of any variables passed to "free()", "g_free()" or "closedir()",
|
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||||
// and report an error if the same variable is passed twice.
|
for (std::size_t i = 0; i < functions; ++i) {
|
||||||
if (Token::Match(tok, "free|g_free|closedir ( %var% )")) {
|
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||||
const unsigned int varId = tok->tokAt(2)->varId();
|
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||||
if (varId) {
|
// Keep track of any variables passed to "free()", "g_free()" or "closedir()",
|
||||||
if (Token::Match(tok, "free|g_free")) {
|
// and report an error if the same variable is passed twice.
|
||||||
if (freedVariables.find(varId) != freedVariables.end())
|
if (Token::Match(tok, "free|g_free|closedir ( %var% )")) {
|
||||||
doubleFreeError(tok, tok->strAt(2));
|
const unsigned int varId = tok->tokAt(2)->varId();
|
||||||
else
|
if (varId) {
|
||||||
freedVariables.insert(varId);
|
if (Token::Match(tok, "free|g_free")) {
|
||||||
} else if (tok->str() == "closedir") {
|
if (freedVariables.find(varId) != freedVariables.end())
|
||||||
if (closeDirVariables.find(varId) != closeDirVariables.end())
|
doubleFreeError(tok, tok->strAt(2));
|
||||||
doubleCloseDirError(tok, tok->strAt(2));
|
else
|
||||||
else
|
freedVariables.insert(varId);
|
||||||
closeDirVariables.insert(varId);
|
} else if (tok->str() == "closedir") {
|
||||||
}
|
if (closeDirVariables.find(varId) != closeDirVariables.end())
|
||||||
}
|
doubleCloseDirError(tok, tok->strAt(2));
|
||||||
}
|
else
|
||||||
|
closeDirVariables.insert(varId);
|
||||||
// Keep track of any variables operated on by "delete" or "delete[]"
|
|
||||||
// and report an error if the same variable is delete'd twice.
|
|
||||||
else if (Token::Match(tok, "delete %var% ;") || Token::Match(tok, "delete [ ] %var% ;")) {
|
|
||||||
const int varIndex = (tok->strAt(1) == "[") ? 3 : 1;
|
|
||||||
const unsigned int varId = tok->tokAt(varIndex)->varId();
|
|
||||||
if (varId) {
|
|
||||||
if (freedVariables.find(varId) != freedVariables.end())
|
|
||||||
doubleFreeError(tok, tok->strAt(varIndex));
|
|
||||||
else
|
|
||||||
freedVariables.insert(varId);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// If this scope doesn't return, clear the set of previously freed variables
|
|
||||||
else if (tok->str() == "}" && _tokenizer->IsScopeNoReturn(tok)) {
|
|
||||||
freedVariables.clear();
|
|
||||||
closeDirVariables.clear();
|
|
||||||
}
|
|
||||||
|
|
||||||
// If this scope is a "for" or "while" loop that contains "break" or "continue",
|
|
||||||
// give up on trying to figure out the flow of execution and just clear the set
|
|
||||||
// of previously freed variables.
|
|
||||||
// TODO: There are false negatives. This bailout is only needed when the
|
|
||||||
// loop will exit without free()'ing the memory on the last iteration.
|
|
||||||
else if (tok->str() == "}" && tok->link() && tok->link()->previous() &&
|
|
||||||
tok->link()->linkAt(-1) &&
|
|
||||||
Token::Match(tok->link()->linkAt(-1)->previous(), "while|for") &&
|
|
||||||
Token::findmatch(tok->link()->linkAt(-1), "break|continue ;", tok) != nullptr) {
|
|
||||||
freedVariables.clear();
|
|
||||||
closeDirVariables.clear();
|
|
||||||
}
|
|
||||||
|
|
||||||
// If a variable is passed to a function, remove it from the set of previously freed variables
|
|
||||||
else if (Token::Match(tok, "%var% (") && _settings->library.leakignore.find(tok->str()) == _settings->library.leakignore.end()) {
|
|
||||||
|
|
||||||
// If this is a new function definition, clear all variables
|
|
||||||
if (Token::simpleMatch(tok->next()->link(), ") {")) {
|
|
||||||
freedVariables.clear();
|
|
||||||
closeDirVariables.clear();
|
|
||||||
}
|
|
||||||
// If it is a function call, then clear those variables in its argument list
|
|
||||||
else if (Token::simpleMatch(tok->next()->link(), ") ;")) {
|
|
||||||
for (const Token* tok2 = tok->tokAt(2); tok2 != tok->linkAt(1); tok2 = tok2->next()) {
|
|
||||||
const unsigned int varId = tok2->varId();
|
|
||||||
if (varId) {
|
|
||||||
freedVariables.erase(varId);
|
|
||||||
closeDirVariables.erase(varId);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// If a pointer is assigned a new value, remove it from the set of previously freed variables
|
// Keep track of any variables operated on by "delete" or "delete[]"
|
||||||
else if (Token::Match(tok, "%var% =")) {
|
// and report an error if the same variable is delete'd twice.
|
||||||
const unsigned int varId = tok->varId();
|
else if (Token::Match(tok, "delete %var% ;") || Token::Match(tok, "delete [ ] %var% ;")) {
|
||||||
if (varId) {
|
const int varIndex = (tok->strAt(1) == "[") ? 3 : 1;
|
||||||
freedVariables.erase(varId);
|
const unsigned int varId = tok->tokAt(varIndex)->varId();
|
||||||
closeDirVariables.erase(varId);
|
if (varId) {
|
||||||
|
if (freedVariables.find(varId) != freedVariables.end())
|
||||||
|
doubleFreeError(tok, tok->strAt(varIndex));
|
||||||
|
else
|
||||||
|
freedVariables.insert(varId);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// Any control statements in-between delete, free() or closedir() statements
|
// If this scope doesn't return, clear the set of previously freed variables
|
||||||
// makes it unclear whether any subsequent statements would be redundant.
|
else if (tok->str() == "}" && _tokenizer->IsScopeNoReturn(tok)) {
|
||||||
if (Token::Match(tok, "if|else|for|while|break|continue|goto|return|throw|switch")) {
|
freedVariables.clear();
|
||||||
freedVariables.clear();
|
closeDirVariables.clear();
|
||||||
closeDirVariables.clear();
|
}
|
||||||
|
|
||||||
|
// If this scope is a "for" or "while" loop that contains "break" or "continue",
|
||||||
|
// give up on trying to figure out the flow of execution and just clear the set
|
||||||
|
// of previously freed variables.
|
||||||
|
// TODO: There are false negatives. This bailout is only needed when the
|
||||||
|
// loop will exit without free()'ing the memory on the last iteration.
|
||||||
|
else if (tok->str() == "}" && tok->link() && tok->link()->previous() &&
|
||||||
|
tok->link()->linkAt(-1) &&
|
||||||
|
Token::Match(tok->link()->linkAt(-1)->previous(), "while|for") &&
|
||||||
|
Token::findmatch(tok->link()->linkAt(-1), "break|continue ;", tok) != nullptr) {
|
||||||
|
freedVariables.clear();
|
||||||
|
closeDirVariables.clear();
|
||||||
|
}
|
||||||
|
|
||||||
|
// If a variable is passed to a function, remove it from the set of previously freed variables
|
||||||
|
else if (Token::Match(tok, "%var% (") && _settings->library.leakignore.find(tok->str()) == _settings->library.leakignore.end()) {
|
||||||
|
|
||||||
|
// If this is a new function definition, clear all variables
|
||||||
|
if (Token::simpleMatch(tok->next()->link(), ") {")) {
|
||||||
|
freedVariables.clear();
|
||||||
|
closeDirVariables.clear();
|
||||||
|
}
|
||||||
|
// If it is a function call, then clear those variables in its argument list
|
||||||
|
else if (Token::simpleMatch(tok->next()->link(), ") ;")) {
|
||||||
|
for (const Token* tok2 = tok->tokAt(2); tok2 != tok->linkAt(1); tok2 = tok2->next()) {
|
||||||
|
const unsigned int varId = tok2->varId();
|
||||||
|
if (varId) {
|
||||||
|
freedVariables.erase(varId);
|
||||||
|
closeDirVariables.erase(varId);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// If a pointer is assigned a new value, remove it from the set of previously freed variables
|
||||||
|
else if (Token::Match(tok, "%var% =")) {
|
||||||
|
const unsigned int varId = tok->varId();
|
||||||
|
if (varId) {
|
||||||
|
freedVariables.erase(varId);
|
||||||
|
closeDirVariables.erase(varId);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Any control statements in-between delete, free() or closedir() statements
|
||||||
|
// makes it unclear whether any subsequent statements would be redundant.
|
||||||
|
if (Token::Match(tok, "if|else|for|while|break|continue|goto|return|throw|switch")) {
|
||||||
|
freedVariables.clear();
|
||||||
|
closeDirVariables.clear();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -2375,14 +2390,20 @@ void CheckOther::checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* to
|
||||||
//-----------------------------------------------------------------------------
|
//-----------------------------------------------------------------------------
|
||||||
void CheckOther::redundantGetAndSetUserId()
|
void CheckOther::redundantGetAndSetUserId()
|
||||||
{
|
{
|
||||||
if (_settings->isEnabled("warning")
|
if (!_settings->standards.posix || !_settings->isEnabled("warning"))
|
||||||
&& _settings->standards.posix) {
|
return;
|
||||||
|
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
|
|
||||||
|
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||||
|
for (std::size_t i = 0; i < functions; ++i) {
|
||||||
|
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||||
|
// check all the code in the function
|
||||||
|
for (const Token *tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||||
if (Token::simpleMatch(tok, "setuid ( getuid ( ) )")
|
if (Token::simpleMatch(tok, "setuid ( getuid ( ) )")
|
||||||
|| Token::simpleMatch(tok, "seteuid ( geteuid ( ) )")
|
|| Token::simpleMatch(tok, "seteuid ( geteuid ( ) )")
|
||||||
|| Token::simpleMatch(tok, "setgid ( getgid ( ) )")
|
|| Token::simpleMatch(tok, "setgid ( getgid ( ) )")
|
||||||
|| Token::simpleMatch(tok, "setegid ( getegid ( ) )")) {
|
|| Token::simpleMatch(tok, "setegid ( getegid ( ) )")) {
|
||||||
redundantGetAndSetUserIdError(tok);
|
redundantGetAndSetUserIdError(tok);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -270,35 +270,40 @@ void CheckString::incorrectStringBooleanError(const Token *tok, const std::strin
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
void CheckString::sprintfOverlappingData()
|
void CheckString::sprintfOverlappingData()
|
||||||
{
|
{
|
||||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
// Get variable id of target buffer..
|
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||||
unsigned int varid = 0;
|
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()) {
|
||||||
|
// Get variable id of target buffer..
|
||||||
|
unsigned int varid = 0;
|
||||||
|
|
||||||
if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% ,"))
|
if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% ,"))
|
||||||
varid = tok->tokAt(2)->varId();
|
varid = tok->tokAt(2)->varId();
|
||||||
|
|
||||||
else if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% . %var% ,"))
|
else if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% . %var% ,"))
|
||||||
varid = tok->tokAt(4)->varId();
|
varid = tok->tokAt(4)->varId();
|
||||||
|
|
||||||
if (varid == 0)
|
if (varid == 0)
|
||||||
continue;
|
|
||||||
|
|
||||||
// goto next argument
|
|
||||||
const Token *tok2 = tok->tokAt(2)->nextArgument();
|
|
||||||
|
|
||||||
if (tok->str() == "snprintf" || tok->str() == "swprintf") { // Jump over second parameter for snprintf and swprintf
|
|
||||||
tok2 = tok2->nextArgument();
|
|
||||||
if (!tok2)
|
|
||||||
continue;
|
continue;
|
||||||
}
|
|
||||||
|
|
||||||
// is any source buffer overlapping the target buffer?
|
// goto next argument
|
||||||
do {
|
const Token *tok2 = tok->tokAt(2)->nextArgument();
|
||||||
if (Token::Match(tok2, "%varid% [,)]", varid)) {
|
|
||||||
sprintfOverlappingDataError(tok2, tok2->str());
|
if (tok->str() == "snprintf" || tok->str() == "swprintf") { // Jump over second parameter for snprintf and swprintf
|
||||||
break;
|
tok2 = tok2->nextArgument();
|
||||||
|
if (!tok2)
|
||||||
|
continue;
|
||||||
}
|
}
|
||||||
} while (nullptr != (tok2 = tok2->nextArgument()));
|
|
||||||
|
// is any source buffer overlapping the target buffer?
|
||||||
|
do {
|
||||||
|
if (Token::Match(tok2, "%varid% [,)]", varid)) {
|
||||||
|
sprintfOverlappingDataError(tok2, tok2->str());
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
} while (nullptr != (tok2 = tok2->nextArgument()));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -3682,20 +3682,20 @@ private:
|
||||||
}
|
}
|
||||||
|
|
||||||
void redundantGetAndSetUserId() {
|
void redundantGetAndSetUserId() {
|
||||||
check("seteuid(geteuid());\n", nullptr, false , false, true);
|
check("void foo() { seteuid(geteuid()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
||||||
check("setuid(getuid());\n", nullptr, false , false, true);
|
check("void foo() { setuid(getuid()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
||||||
check("setgid(getgid());\n", nullptr, false , false, true);
|
check("void foo() { setgid(getgid()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
||||||
check("setegid(getegid());\n", nullptr, false , false, true);
|
check("void foo() { setegid(getegid()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str());
|
||||||
|
|
||||||
check("seteuid(getuid());\n", nullptr, false , false, true);
|
check("void foo() { seteuid(getuid()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
check("seteuid(foo());\n", nullptr, false , false, true);
|
check("void foo() { seteuid(foo()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
check("foo(getuid());\n", nullptr, false , false, true);
|
check("void foo() { foo(getuid()); }", nullptr, false , false, true);
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue