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

Merge pull request #326 from simartin/ticket_5615_avoid_useless_similar_calls 

Ticket #5615: Avoid calling the same function n times when once is enough
This commit is contained in:
amai2012 2014-05-29 21:52:22 +02:00
parent 707ee97b9e 139f87af18
commit 92759dfd9d
1 changed files with 33 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
lib/checkbufferoverrun.cpp
View File

@ -454,7 +454,8 @@ static bool for_bailout(const Token * const tok1, unsigned int varid)
void CheckBufferOverrun::parse_for_body(const Token *tok, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value) void CheckBufferOverrun::parse_for_body(const Token *tok, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value)
{ {
const std::string pattern = (arrayInfo.declarationId() ? std::string("%varid%") : arrayInfo.varname()) + " [ " + strindex + " ]"; unsigned int arrayInfoDeclarationId = arrayInfo.declarationId();
const std::string pattern = (arrayInfoDeclarationId ? std::string("%varid%") : arrayInfo.varname()) + " [ " + strindex + " ]";
for (const Token* tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) { for (const Token* tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) {
// TestBufferOverrun::array_index_for_question // TestBufferOverrun::array_index_for_question
@ -493,16 +494,16 @@ void CheckBufferOverrun::parse_for_body(const Token *tok, const ArrayInfo &array
} }
if (Token::Match(tok2, "if|switch")) { if (Token::Match(tok2, "if|switch")) {
if (bailoutIfSwitch(tok2, arrayInfo.declarationId())) if (bailoutIfSwitch(tok2, arrayInfoDeclarationId))
break; break;
} }
if (condition_out_of_bounds && Token::Match(tok2, pattern.c_str(), arrayInfo.declarationId())) { if (condition_out_of_bounds && Token::Match(tok2, pattern.c_str(), arrayInfoDeclarationId)) {
bufferOverrunError(tok2, arrayInfo.varname()); bufferOverrunError(tok2, arrayInfo.varname());
break; break;
} }
else if (arrayInfo.declarationId() && tok2->varId() && counter_varid > 0 && !min_counter_value.empty() && !max_counter_value.empty()) { else if (arrayInfoDeclarationId && tok2->varId() && counter_varid > 0 && !min_counter_value.empty() && !max_counter_value.empty()) {
// Is the loop variable used to calculate the array index? // Is the loop variable used to calculate the array index?
// In this scope it is determined if such calculated // In this scope it is determined if such calculated
// array indexes are out of bounds. // array indexes are out of bounds.
@ -515,7 +516,7 @@ void CheckBufferOverrun::parse_for_body(const Token *tok, const ArrayInfo &array
// Maximum calculated array index // Maximum calculated array index
int max_index = 0; int max_index = 0;
if (Token::Match(tok2, "%varid% [ %var% +|-|*|/ %num% ]", arrayInfo.declarationId()) && if (Token::Match(tok2, "%varid% [ %var% +|-|*|/ %num% ]", arrayInfoDeclarationId) &&
tok2->tokAt(2)->varId() == counter_varid) { tok2->tokAt(2)->varId() == counter_varid) {
// operator: +-*/ // operator: +-*/
const char action = tok2->strAt(3)[0]; const char action = tok2->strAt(3)[0];
@ -527,7 +528,7 @@ void CheckBufferOverrun::parse_for_body(const Token *tok, const ArrayInfo &array
//printf("max_index: %s %c %s\n", max_counter_value.c_str(), action, second.c_str()); //printf("max_index: %s %c %s\n", max_counter_value.c_str(), action, second.c_str());
min_index = std::atoi(MathLib::calculate(min_counter_value, second, action).c_str()); min_index = std::atoi(MathLib::calculate(min_counter_value, second, action).c_str());
max_index = std::atoi(MathLib::calculate(max_counter_value, second, action).c_str()); max_index = std::atoi(MathLib::calculate(max_counter_value, second, action).c_str());
} else if (Token::Match(tok2, "%varid% [ %num% +|-|*|/ %var% ]", arrayInfo.declarationId()) && } else if (Token::Match(tok2, "%varid% [ %num% +|-|*|/ %var% ]", arrayInfoDeclarationId) &&
tok2->tokAt(4)->varId() == counter_varid) { tok2->tokAt(4)->varId() == counter_varid) {
// operator: +-*/ // operator: +-*/
const char action = tok2->strAt(3)[0]; const char action = tok2->strAt(3)[0];
@ -768,20 +769,22 @@ void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &ar
} }
callstack.push_back(tok); callstack.push_back(tok);
const unsigned int declarationId = arrayInfo.declarationId();
const Token *tok2 = tok->tokAt(2); const Token *tok2 = tok->tokAt(2);
// 1st parameter.. // 1st parameter..
if (Token::Match(tok2, "%varid% ,|)", arrayInfo.declarationId())) if (Token::Match(tok2, "%varid% ,|)", declarationId))
checkFunctionParameter(*tok, 1, arrayInfo, callstack); checkFunctionParameter(*tok, 1, arrayInfo, callstack);
else if (Token::Match(tok2, "%varid% + %num% ,|)", arrayInfo.declarationId())) { else if (Token::Match(tok2, "%varid% + %num% ,|)", declarationId)) {
const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok2->strAt(2)))); const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok2->strAt(2))));
checkFunctionParameter(*tok, 1, ai, callstack); checkFunctionParameter(*tok, 1, ai, callstack);
} }
// goto 2nd parameter and check it.. // goto 2nd parameter and check it..
tok2 = tok2->nextArgument(); tok2 = tok2->nextArgument();
if (Token::Match(tok2, "%varid% ,|)", arrayInfo.declarationId())) if (Token::Match(tok2, "%varid% ,|)", declarationId))
checkFunctionParameter(*tok, 2, arrayInfo, callstack); checkFunctionParameter(*tok, 2, arrayInfo, callstack);
else if (Token::Match(tok2, "%varid% + %num% ,|)", arrayInfo.declarationId())) { else if (Token::Match(tok2, "%varid% + %num% ,|)", declarationId)) {
const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok2->strAt(2)))); const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok2->strAt(2))));
checkFunctionParameter(*tok, 2, ai, callstack); checkFunctionParameter(*tok, 2, ai, callstack);
} }
@ -948,6 +951,8 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
// out of bounds then this flag will be set. // out of bounds then this flag will be set.
bool pointerIsOutOfBounds = false; bool pointerIsOutOfBounds = false;
const bool isPortabilityEnabled = _settings->isEnabled("portability");
for (const Token* const end = tok->scope()->classEnd; tok != end; tok = tok->next()) { for (const Token* const end = tok->scope()->classEnd; tok != end; tok = tok->next()) {
if (declarationId != 0 && Token::Match(tok, "%varid% = new|malloc|realloc", declarationId)) { if (declarationId != 0 && Token::Match(tok, "%varid% = new|malloc|realloc", declarationId)) {
// Abort // Abort
@ -1125,7 +1130,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
// undefined behaviour: result of pointer arithmetic is out of bounds // undefined behaviour: result of pointer arithmetic is out of bounds
else if (declarationId && Token::Match(tok, "= %varid% + %num% ;", declarationId)) { else if (declarationId && Token::Match(tok, "= %varid% + %num% ;", declarationId)) {
const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3)); const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3));
if (index > size && _settings->isEnabled("portability")) if (isPortabilityEnabled && index > size)
pointerOutOfBoundsError(tok->next(), "buffer"); pointerOutOfBoundsError(tok->next(), "buffer");
if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", declarationId)) if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", declarationId))
pointerIsOutOfBounds = true; pointerIsOutOfBounds = true;
@ -1245,6 +1250,11 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
const Token *scope_begin = tok->previous(); const Token *scope_begin = tok->previous();
assert(scope_begin != 0); assert(scope_begin != 0);
const unsigned int declarationId = arrayInfo.declarationId();
const bool isPortabilityEnabled = _settings->isEnabled("portability");
const bool isWarningEnabled = _settings->isEnabled("warning");
for (const Token* const end = tok->scope()->classEnd; tok != end; tok = tok->next()) { for (const Token* const end = tok->scope()->classEnd; tok != end; tok = tok->next()) {
// Skip array declarations // Skip array declarations
if (Token::Match(tok, "[;{}] %type% *| %var% [") && tok->strAt(1) != "return") { if (Token::Match(tok, "[;{}] %type% *| %var% [") && tok->strAt(1) != "return") {
@ -1252,7 +1262,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
continue; continue;
} }
else if (Token::Match(tok, "%varid% [", arrayInfo.declarationId())) { else if (Token::Match(tok, "%varid% [", declarationId)) {
valueFlowCheckArrayIndex(tok->next(), arrayInfo); valueFlowCheckArrayIndex(tok->next(), arrayInfo);
} }
@ -1273,7 +1283,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
checkFunctionCall(tok, arrayInfo, callstack); checkFunctionCall(tok, arrayInfo, callstack);
} }
if (Token::Match(tok, "strncpy|memcpy|memmove ( %varid% , %str% , %num% )", arrayInfo.declarationId())) { if (Token::Match(tok, "strncpy|memcpy|memmove ( %varid% , %str% , %num% )", declarationId)) {
const unsigned int num = (unsigned int)MathLib::toLongNumber(tok->strAt(6)); const unsigned int num = (unsigned int)MathLib::toLongNumber(tok->strAt(6));
if (Token::getStrLength(tok->tokAt(4)) >= (unsigned int)total_size && (unsigned int)total_size == num) { if (Token::getStrLength(tok->tokAt(4)) >= (unsigned int)total_size && (unsigned int)total_size == num) {
if (_settings->inconclusive) if (_settings->inconclusive)
@ -1281,7 +1291,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
} }
if ((Token::Match(tok, "strncpy|strncat ( %varid% ,", arrayInfo.declarationId()) && Token::Match(tok->linkAt(1)->tokAt(-2), ", %num% )"))) { if ((Token::Match(tok, "strncpy|strncat ( %varid% ,", declarationId) && Token::Match(tok->linkAt(1)->tokAt(-2), ", %num% )"))) {
const Token* param3 = tok->linkAt(1)->previous(); const Token* param3 = tok->linkAt(1)->previous();
// check for strncpy which is not terminated // check for strncpy which is not terminated
@ -1290,7 +1300,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
unsigned int num = (unsigned int)MathLib::toLongNumber(param3->str()); unsigned int num = (unsigned int)MathLib::toLongNumber(param3->str());
// this is currently 'inconclusive'. See TestBufferOverrun::terminateStrncpy3 // this is currently 'inconclusive'. See TestBufferOverrun::terminateStrncpy3
if (num >= total_size && _settings->isEnabled("warning") && _settings->inconclusive) { if (isWarningEnabled && num >= total_size && _settings->inconclusive) {
const Token *tok2 = tok->next()->link()->next(); const Token *tok2 = tok->next()->link()->next();
for (; tok2; tok2 = tok2->next()) { for (; tok2; tok2 = tok2->next()) {
if (tok2->varId() == tok->tokAt(2)->varId()) { if (tok2->varId() == tok->tokAt(2)->varId()) {
@ -1312,7 +1322,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
// Dangerous usage of strncpy + strncat.. // Dangerous usage of strncpy + strncat..
if (Token::Match(param3->tokAt(2), "; strncat ( %varid% ,", arrayInfo.declarationId()) && Token::Match(param3->linkAt(4)->tokAt(-2), ", %num% )")) { if (Token::Match(param3->tokAt(2), "; strncat ( %varid% ,", declarationId) && Token::Match(param3->linkAt(4)->tokAt(-2), ", %num% )")) {
const MathLib::bigint n = MathLib::toLongNumber(param3->str()) + MathLib::toLongNumber(param3->linkAt(4)->strAt(-1)); const MathLib::bigint n = MathLib::toLongNumber(param3->str()) + MathLib::toLongNumber(param3->linkAt(4)->strAt(-1));
if (n > total_size) if (n > total_size)
strncatUsageError(param3->tokAt(3)); strncatUsageError(param3->tokAt(3));
@ -1320,7 +1330,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
// Writing data into array.. // Writing data into array..
if (Token::Match(tok, "strcpy|strcat ( %varid% , %str% )", arrayInfo.declarationId())) { if (Token::Match(tok, "strcpy|strcat ( %varid% , %str% )", declarationId)) {
const std::size_t len = Token::getStrLength(tok->tokAt(4)); const std::size_t len = Token::getStrLength(tok->tokAt(4));
if (total_size > 0 && len >= (unsigned int)total_size) { if (total_size > 0 && len >= (unsigned int)total_size) {
bufferOverrunError(tok, arrayInfo.varname()); bufferOverrunError(tok, arrayInfo.varname());
@ -1329,11 +1339,11 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
// Detect few strcat() calls // Detect few strcat() calls
if (total_size > 0 && Token::Match(tok, "strcat ( %varid% , %str% ) ;", arrayInfo.declarationId())) { if (total_size > 0 && Token::Match(tok, "strcat ( %varid% , %str% ) ;", declarationId)) {
std::size_t charactersAppend = 0; std::size_t charactersAppend = 0;
const Token *tok2 = tok; const Token *tok2 = tok;
while (tok2 && Token::Match(tok2, "strcat ( %varid% , %str% ) ;", arrayInfo.declarationId())) { while (tok2 && Token::Match(tok2, "strcat ( %varid% , %str% ) ;", declarationId)) {
charactersAppend += Token::getStrLength(tok2->tokAt(4)); charactersAppend += Token::getStrLength(tok2->tokAt(4));
if (charactersAppend >= (unsigned int)total_size) { if (charactersAppend >= (unsigned int)total_size) {
bufferOverrunError(tok2, arrayInfo.varname()); bufferOverrunError(tok2, arrayInfo.varname());
@ -1344,12 +1354,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
if (Token::Match(tok, "sprintf ( %varid% , %str% [,)]", arrayInfo.declarationId())) { if (Token::Match(tok, "sprintf ( %varid% , %str% [,)]", declarationId)) {
checkSprintfCall(tok, total_size); checkSprintfCall(tok, total_size);
} }
// snprintf.. // snprintf..
if (total_size > 0 && Token::Match(tok, "snprintf ( %varid% , %num% ,", arrayInfo.declarationId())) { if (total_size > 0 && Token::Match(tok, "snprintf ( %varid% , %num% ,", declarationId)) {
const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(4)); const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(4));
if (n > total_size) if (n > total_size)
outOfBoundsError(tok->tokAt(4), "snprintf size", true, n, total_size); outOfBoundsError(tok->tokAt(4), "snprintf size", true, n, total_size);
@ -1357,10 +1367,10 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
// readlink() / readlinkat() buffer usage // readlink() / readlinkat() buffer usage
if (_settings->standards.posix && Token::Match(tok, "readlink|readlinkat (")) if (_settings->standards.posix && Token::Match(tok, "readlink|readlinkat ("))
checkReadlinkBufferUsage(tok, scope_begin, arrayInfo.declarationId(), total_size); checkReadlinkBufferUsage(tok, scope_begin, declarationId, total_size);
// undefined behaviour: result of pointer arithmetic is out of bounds // undefined behaviour: result of pointer arithmetic is out of bounds
if (_settings->isEnabled("portability") && Token::Match(tok, "= %varid% + %num% ;", arrayInfo.declarationId())) { if (isPortabilityEnabled && Token::Match(tok, "= %varid% + %num% ;", declarationId)) {
const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3)); const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3));
if (index < 0 || index > arrayInfo.num(0)) { if (index < 0 || index > arrayInfo.num(0)) {
pointerOutOfBoundsError(tok->next(), "array"); pointerOutOfBoundsError(tok->next(), "array");