From 994c429b7d4f67bdff0a6bd6a37f9f357c6ed7e2 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Wed, 10 Apr 2013 09:49:38 -0700 Subject: [PATCH] Moved checks related to sizeof usage from checkother into new file --- lib/checkother.cpp | 252 ---------------- lib/checkother.h | 44 --- lib/checksizeof.cpp | 280 ++++++++++++++++++ lib/checksizeof.h | 124 ++++++++ lib/cppcheck.vcxproj | 2 + lib/cppcheck.vcxproj.filters | 6 + test/testother.cpp | 447 ----------------------------- test/testrunner.vcxproj | 1 + test/testrunner.vcxproj.filters | 3 + test/testsizeof.cpp | 491 ++++++++++++++++++++++++++++++++ 10 files changed, 907 insertions(+), 743 deletions(-) create mode 100644 lib/checksizeof.cpp create mode 100644 lib/checksizeof.h create mode 100644 test/testsizeof.cpp diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a862f9c9c..e97855973 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -499,35 +499,6 @@ void CheckOther::invalidPointerCastError(const Token* tok, const std::string& fr reportError(tok, Severity::warning, "invalidPointerCast", "Casting between " + from + "* and " + to + "* which have an incompatible binary data representation."); } -//--------------------------------------------------------------------------- -//--------------------------------------------------------------------------- -void CheckOther::checkSizeofForNumericParameter() -{ - if (!_settings->isEnabled("warning")) - return; - - 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->next(); tok != scope->classEnd; tok = tok->next()) { - if (Token::Match(tok, "sizeof ( %num% )") || - Token::Match(tok, "sizeof %num%")) { - sizeofForNumericParameterError(tok); - } - } - } -} - -void CheckOther::sizeofForNumericParameterError(const Token *tok) -{ - reportError(tok, Severity::warning, - "sizeofwithnumericparameter", "Suspicious usage of 'sizeof' with a numeric constant as parameter.\n" - "It is unusual to use a constant value with sizeof. For example, 'sizeof(10)'" - " returns 4 (in 32-bit systems) or 8 (in 64-bit systems) instead of 10. 'sizeof('A')'" - " and 'sizeof(char)' can return different results."); -} - //--------------------------------------------------------------------------- // This check detects errors on POSIX systems, when a pipe command called // with a wrong dimensioned file descriptor array. The pipe command requires @@ -604,140 +575,6 @@ void CheckOther::checkSleepTimeError(const Token *tok, const std::string &strDim "The argument of usleep must be less than 1000000, but " + strDim + " is provided."); } -//--------------------------------------------------------------------------- -//--------------------------------------------------------------------------- -void CheckOther::checkSizeofForArrayParameter() -{ - 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->next(); tok != scope->classEnd; tok = tok->next()) { - if (Token::Match(tok, "sizeof ( %var% )") || - Token::Match(tok, "sizeof %var% !![")) { - const Token* varTok = tok->next(); - if (varTok->str() == "(") { - varTok = varTok->next(); - } - if (varTok->varId() > 0) { - const Variable *var = varTok->variable(); - if (var && var->isArray() && var->isArgument()) { - sizeofForArrayParameterError(tok); - } - } - } - } - } -} - -void CheckOther::sizeofForArrayParameterError(const Token *tok) -{ - reportError(tok, Severity::error, - "sizeofwithsilentarraypointer", "Using 'sizeof' on array given as function argument " - "returns size of a pointer.\n" - "Using 'sizeof' for array given as function argument returns the size of a pointer. " - "It does not return the size of the whole array in bytes as might be " - "expected. For example, this code:\n" - " int f(char a[100]) {\n" - " return sizeof(a);\n" - " }\n" - "returns 4 (in 32-bit systems) or 8 (in 64-bit systems) instead of 100 (the " - "size of the array in bytes)." - ); -} - -void CheckOther::checkSizeofForPointerSize() -{ - if (!_settings->isEnabled("warning")) - return; - - 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()) { - const Token *tokVar; - const Token *variable; - const Token *variable2 = 0; - - // Find any function that may use sizeof on a pointer - // Once leaving those tests, it is mandatory to have: - // - variable matching the used pointer - // - tokVar pointing on the argument where sizeof may be used - if (Token::Match(tok, "[*;{}] %var% = malloc|alloca (")) { - variable = tok->next(); - tokVar = tok->tokAt(5); - - } else if (Token::Match(tok, "[*;{}] %var% = calloc (")) { - variable = tok->next(); - tokVar = tok->tokAt(5)->nextArgument(); - - } else if (Token::simpleMatch(tok, "memset (")) { - variable = tok->tokAt(2); - tokVar = variable->tokAt(2)->nextArgument(); - - // The following tests can be inconclusive in case the variable in sizeof - // is constant string by intention - } else if (!_settings->inconclusive) { - continue; - - } else if (Token::Match(tok, "memcpy|memcmp|memmove|strncpy|strncmp|strncat (")) { - variable = tok->tokAt(2); - variable2 = variable->nextArgument(); - tokVar = variable2->nextArgument(); - - } else { - continue; - } - - // Ensure the variables are in the symbol database - // Also ensure the variables are pointers - // Only keep variables which are pointers - const Variable *var = variable->variable(); - if (!var || !var->isPointer() || var->isArray()) { - variable = 0; - } - - if (variable2) { - var = variable2->variable(); - if (!var || !var->isPointer() || var->isArray()) { - variable2 = 0; - } - } - - // If there are no pointer variable at this point, there is - // no need to continue - if (variable == 0 && variable2 == 0) { - continue; - } - - // Jump to the next sizeof token in the function and in the parameter - // This is to allow generic operations with sizeof - for (; tokVar && tokVar->str() != ")" && tokVar->str() != "," && tokVar->str() != "sizeof"; tokVar = tokVar->next()) {} - - // Now check for the sizeof usage. Once here, everything using sizeof(varid) or sizeof(&varid) - // looks suspicious - // Do it for first variable - if (variable && (Token::Match(tokVar, "sizeof ( &| %varid% )", variable->varId()) || - Token::Match(tokVar, "sizeof &| %varid%", variable->varId()))) { - sizeofForPointerError(variable, variable->str()); - } else if (variable2 && (Token::Match(tokVar, "sizeof ( &| %varid% )", variable2->varId()) || - Token::Match(tokVar, "sizeof &| %varid%", variable2->varId()))) { - sizeofForPointerError(variable2, variable2->str()); - } - } - } -} - -void CheckOther::sizeofForPointerError(const Token *tok, const std::string &varname) -{ - reportError(tok, Severity::warning, "pointerSize", - "Size of pointer '" + varname + "' used instead of size of its data.\n" - "Size of pointer '" + varname + "' used instead of size of its data. " - "This is likely to lead to a buffer overflow. You probably intend to " - "write 'sizeof(*" + varname + ")'.", true); -} - //--------------------------------------------------------------------------- // Detect redundant assignments: x = 0; x = 4; //--------------------------------------------------------------------------- @@ -3265,59 +3102,6 @@ void CheckOther::moduloAlwaysTrueFalseError(const Token* tok, const std::string& "Comparison of modulo result is predetermined, because it is always less than " + maxVal + "."); } -//----------------------------------------------------------------------------- -//----------------------------------------------------------------------------- -void CheckOther::sizeofsizeof() -{ - if (!_settings->isEnabled("warning")) - return; - - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::Match(tok, "sizeof (| sizeof")) { - sizeofsizeofError(tok); - tok = tok->next(); - } - } -} - -void CheckOther::sizeofsizeofError(const Token *tok) -{ - reportError(tok, Severity::warning, - "sizeofsizeof", "Calling 'sizeof' on 'sizeof'.\n" - "Calling sizeof for 'sizeof looks like a suspicious code and " - "most likely there should be just one 'sizeof'. The current " - "code is equivalent to 'sizeof(size_t)'"); -} - -//----------------------------------------------------------------------------- -//----------------------------------------------------------------------------- -void CheckOther::sizeofCalculation() -{ - if (!_settings->isEnabled("warning")) - return; - - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::simpleMatch(tok, "sizeof (")) { - const Token* const end = tok->linkAt(1); - for (const Token *tok2 = tok->tokAt(2); tok2 != end; tok2 = tok2->next()) { - if (tok2->isConstOp() && (!tok2->isExpandedMacro() || _settings->inconclusive) && !Token::Match(tok2, ">|<|&") && (Token::Match(tok2->previous(), "%var%") || tok2->str() != "*")) { - if (!(Token::Match(tok2->previous(), "%type%") || Token::Match(tok2->next(), "%type%"))) { - sizeofCalculationError(tok2, tok2->isExpandedMacro()); - break; - } - } else if (tok2->type() == Token::eIncDecOp) - sizeofCalculationError(tok2, tok2->isExpandedMacro()); - } - } - } -} - -void CheckOther::sizeofCalculationError(const Token *tok, bool inconclusive) -{ - reportError(tok, Severity::warning, - "sizeofCalculation", "Found calculation inside sizeof().", inconclusive); -} - //----------------------------------------------------------------------------- // Check for code like: // seteuid(geteuid()) or setuid(getuid()), which first gets and then sets the @@ -3347,42 +3131,6 @@ void CheckOther::redundantGetAndSetUserIdError(const Token *tok) "by get(e)uid() and then set with set(e)uid().", false); } -//----------------------------------------------------------------------------- -// Check for code like sizeof()*sizeof() or sizeof(ptr)/value -//----------------------------------------------------------------------------- -void CheckOther::suspiciousSizeofCalculation() -{ - if (!_settings->isEnabled("warning") || !_settings->inconclusive) - return; - - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::simpleMatch(tok, "sizeof (")) { - const Token* const end = tok->linkAt(1); - const Variable* var = end->previous()->variable(); - if (end->strAt(-1) == "*" || (var && var->isPointer() && !var->isArray())) { - if (end->strAt(1) == "/") - divideSizeofError(tok); - } else if (Token::simpleMatch(end, ") * sizeof")) - multiplySizeofError(tok); - } - } -} - -void CheckOther::multiplySizeofError(const Token *tok) -{ - reportError(tok, Severity::warning, - "multiplySizeof", "Multiplying sizeof() with sizeof() indicates a logic error.", true); -} - -void CheckOther::divideSizeofError(const Token *tok) -{ - reportError(tok, Severity::warning, - "divideSizeof", "Division of result of sizeof() on pointer type.\n" - "Division of result of sizeof() on pointer type. sizeof() returns the size of the pointer, " - "not the size of the memory area it points to.", true); -} - - //--------------------------------------------------------------------------- // Check testing sign of unsigned variables and pointers. diff --git a/lib/checkother.h b/lib/checkother.h index 67e173e24..248037ce1 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -57,16 +57,10 @@ public: checkOther.checkUnsignedDivision(); checkOther.checkCharVariable(); checkOther.strPlusChar(); - checkOther.sizeofsizeof(); - checkOther.sizeofCalculation(); - checkOther.suspiciousSizeofCalculation(); checkOther.checkRedundantAssignment(); checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkSuspiciousCaseInSwitch(); checkOther.checkAssignmentInAssert(); - checkOther.checkSizeofForArrayParameter(); - checkOther.checkSizeofForPointerSize(); - checkOther.checkSizeofForNumericParameter(); checkOther.checkSelfAssignment(); checkOther.checkDuplicateIf(); checkOther.checkDuplicateBranch(); @@ -171,18 +165,9 @@ public: /** @brief %Check for parameters given to cctype function that do make error*/ void checkCCTypeFunctions(); - /** @brief %Check for 'sizeof sizeof ..' */ - void sizeofsizeof(); - - /** @brief %Check for calculations inside sizeof */ - void sizeofCalculation(); - /** @brief % Check for seteuid(geteuid()) or setuid(getuid())*/ void redundantGetAndSetUserId(); - /** @brief %Check for suspicious calculations with sizeof results */ - void suspiciousSizeofCalculation(); - /** @brief copying to memory or assigning to a variable twice */ void checkRedundantAssignment(); @@ -213,15 +198,6 @@ public: /** @brief %Check for filling zero bytes with memset() */ void checkMemsetZeroBytes(); - /** @brief %Check for using sizeof with array given as function argument */ - void checkSizeofForArrayParameter(); - - /** @brief %Check for using sizeof of a variable when allocating it */ - void checkSizeofForPointerSize(); - - /** @brief %Check for using sizeof with numeric given as function argument */ - void checkSizeofForNumericParameter(); - /** @brief %Check for using bad usage of strncmp and substr */ void checkIncorrectStringCompare(); @@ -293,11 +269,7 @@ private: void clarifyCalculationError(const Token *tok, const std::string &op); void clarifyConditionError(const Token *tok, bool assign, bool boolop); void clarifyStatementError(const Token* tok); - void sizeofsizeofError(const Token *tok); - void sizeofCalculationError(const Token *tok, bool inconclusive); void redundantGetAndSetUserIdError(const Token *tok); - void multiplySizeofError(const Token *tok); - void divideSizeofError(const Token *tok); void cstyleCastError(const Token *tok); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive); void dangerousUsageStrtolError(const Token *tok, const std::string& funcname); @@ -326,9 +298,6 @@ private: void redundantConditionError(const Token *tok, const std::string &text); void misusedScopeObjectError(const Token *tok, const std::string &varname); void memsetZeroBytesError(const Token *tok, const std::string &varname); - void sizeofForArrayParameterError(const Token *tok); - void sizeofForPointerError(const Token *tok, const std::string &varname); - void sizeofForNumericParameterError(const Token *tok); void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string); void incorrectStringBooleanError(const Token *tok, const std::string& string); void duplicateIfError(const Token *tok1, const Token *tok2); @@ -360,9 +329,6 @@ private: c.zerodivError(0); c.mathfunctionCallError(0); c.misusedScopeObjectError(NULL, "varname"); - c.sizeofForArrayParameterError(0); - c.sizeofForPointerError(0, "varname"); - c.sizeofForNumericParameterError(0); c.doubleFreeError(0, "varname"); c.invalidPointerCastError(0, "float", "double", false); c.negativeBitwiseShiftError(0); @@ -385,10 +351,6 @@ private: c.charBitOpError(0); c.variableScopeError(0, "varname"); c.strPlusCharError(0); - c.sizeofsizeofError(0); - c.sizeofCalculationError(0, false); - c.multiplySizeofError(0); - c.divideSizeofError(0); c.redundantAssignmentInSwitchError(0, 0, "var"); c.redundantCopyInSwitchError(0, 0, "var"); c.switchCaseFallThrough(0); @@ -435,9 +397,6 @@ private: "* division with zero\n" "* scoped object destroyed immediately after construction\n" "* assignment in an assert statement\n" - "* sizeof for array given as function argument\n" - "* sizeof for numeric given as function argument\n" - "* using sizeof(pointer) instead of the size of pointed data\n" "* incorrect length arguments for 'substr' and 'strncmp'\n" "* free() or delete of an invalid memory location\n" "* double free() or double closedir()\n" @@ -467,9 +426,6 @@ private: "* redundant pre/post operation in a switch statement\n" "* redundant bitwise operation in a switch statement\n" "* redundant strcpy in a switch statement\n" - "* look for 'sizeof sizeof ..'\n" - "* look for calculations inside sizeof()\n" - "* look for suspicious calculations with sizeof()\n" "* assignment of a variable to itself\n" "* Suspicious case labels in switch()\n" "* Suspicious equality comparisons\n" diff --git a/lib/checksizeof.cpp b/lib/checksizeof.cpp new file mode 100644 index 000000000..2fe6a32c1 --- /dev/null +++ b/lib/checksizeof.cpp @@ -0,0 +1,280 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2013 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +//--------------------------------------------------------------------------- +#include "checksizeof.h" +#include "symboldatabase.h" +//--------------------------------------------------------------------------- + +// Register this check class (by creating a static instance of it) +namespace { + CheckSizeof instance; +} + +//--------------------------------------------------------------------------- +//--------------------------------------------------------------------------- +void CheckSizeof::checkSizeofForNumericParameter() +{ + if (!_settings->isEnabled("warning")) + return; + + 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->next(); tok != scope->classEnd; tok = tok->next()) { + if (Token::Match(tok, "sizeof ( %num% )") || + Token::Match(tok, "sizeof %num%")) { + sizeofForNumericParameterError(tok); + } + } + } +} + +void CheckSizeof::sizeofForNumericParameterError(const Token *tok) +{ + reportError(tok, Severity::warning, + "sizeofwithnumericparameter", "Suspicious usage of 'sizeof' with a numeric constant as parameter.\n" + "It is unusual to use a constant value with sizeof. For example, 'sizeof(10)'" + " returns 4 (in 32-bit systems) or 8 (in 64-bit systems) instead of 10. 'sizeof('A')'" + " and 'sizeof(char)' can return different results."); +} + + +//--------------------------------------------------------------------------- +//--------------------------------------------------------------------------- +void CheckSizeof::checkSizeofForArrayParameter() +{ + 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->next(); tok != scope->classEnd; tok = tok->next()) { + if (Token::Match(tok, "sizeof ( %var% )") || + Token::Match(tok, "sizeof %var% !![")) { + const Token* varTok = tok->next(); + if (varTok->str() == "(") { + varTok = varTok->next(); + } + if (varTok->varId() > 0) { + const Variable *var = varTok->variable(); + if (var && var->isArray() && var->isArgument()) { + sizeofForArrayParameterError(tok); + } + } + } + } + } +} + +void CheckSizeof::sizeofForArrayParameterError(const Token *tok) +{ + reportError(tok, Severity::error, + "sizeofwithsilentarraypointer", "Using 'sizeof' on array given as function argument " + "returns size of a pointer.\n" + "Using 'sizeof' for array given as function argument returns the size of a pointer. " + "It does not return the size of the whole array in bytes as might be " + "expected. For example, this code:\n" + " int f(char a[100]) {\n" + " return sizeof(a);\n" + " }\n" + "returns 4 (in 32-bit systems) or 8 (in 64-bit systems) instead of 100 (the " + "size of the array in bytes)." + ); +} + +void CheckSizeof::checkSizeofForPointerSize() +{ + if (!_settings->isEnabled("warning")) + return; + + 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()) { + const Token *tokVar; + const Token *variable; + const Token *variable2 = 0; + + // Find any function that may use sizeof on a pointer + // Once leaving those tests, it is mandatory to have: + // - variable matching the used pointer + // - tokVar pointing on the argument where sizeof may be used + if (Token::Match(tok, "[*;{}] %var% = malloc|alloca (")) { + variable = tok->next(); + tokVar = tok->tokAt(5); + + } else if (Token::Match(tok, "[*;{}] %var% = calloc (")) { + variable = tok->next(); + tokVar = tok->tokAt(5)->nextArgument(); + + } else if (Token::simpleMatch(tok, "memset (")) { + variable = tok->tokAt(2); + tokVar = variable->tokAt(2)->nextArgument(); + + // The following tests can be inconclusive in case the variable in sizeof + // is constant string by intention + } else if (!_settings->inconclusive) { + continue; + + } else if (Token::Match(tok, "memcpy|memcmp|memmove|strncpy|strncmp|strncat (")) { + variable = tok->tokAt(2); + variable2 = variable->nextArgument(); + tokVar = variable2->nextArgument(); + + } else { + continue; + } + + // Ensure the variables are in the symbol database + // Also ensure the variables are pointers + // Only keep variables which are pointers + const Variable *var = variable->variable(); + if (!var || !var->isPointer() || var->isArray()) { + variable = 0; + } + + if (variable2) { + var = variable2->variable(); + if (!var || !var->isPointer() || var->isArray()) { + variable2 = 0; + } + } + + // If there are no pointer variable at this point, there is + // no need to continue + if (variable == 0 && variable2 == 0) { + continue; + } + + // Jump to the next sizeof token in the function and in the parameter + // This is to allow generic operations with sizeof + for (; tokVar && tokVar->str() != ")" && tokVar->str() != "," && tokVar->str() != "sizeof"; tokVar = tokVar->next()) {} + + // Now check for the sizeof usage. Once here, everything using sizeof(varid) or sizeof(&varid) + // looks suspicious + // Do it for first variable + if (variable && (Token::Match(tokVar, "sizeof ( &| %varid% )", variable->varId()) || + Token::Match(tokVar, "sizeof &| %varid%", variable->varId()))) { + sizeofForPointerError(variable, variable->str()); + } else if (variable2 && (Token::Match(tokVar, "sizeof ( &| %varid% )", variable2->varId()) || + Token::Match(tokVar, "sizeof &| %varid%", variable2->varId()))) { + sizeofForPointerError(variable2, variable2->str()); + } + } + } +} + +void CheckSizeof::sizeofForPointerError(const Token *tok, const std::string &varname) +{ + reportError(tok, Severity::warning, "pointerSize", + "Size of pointer '" + varname + "' used instead of size of its data.\n" + "Size of pointer '" + varname + "' used instead of size of its data. " + "This is likely to lead to a buffer overflow. You probably intend to " + "write 'sizeof(*" + varname + ")'.", true); +} + +//----------------------------------------------------------------------------- +//----------------------------------------------------------------------------- +void CheckSizeof::sizeofsizeof() +{ + if (!_settings->isEnabled("warning")) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::Match(tok, "sizeof (| sizeof")) { + sizeofsizeofError(tok); + tok = tok->next(); + } + } +} + +void CheckSizeof::sizeofsizeofError(const Token *tok) +{ + reportError(tok, Severity::warning, + "sizeofsizeof", "Calling 'sizeof' on 'sizeof'.\n" + "Calling sizeof for 'sizeof looks like a suspicious code and " + "most likely there should be just one 'sizeof'. The current " + "code is equivalent to 'sizeof(size_t)'"); +} + +//----------------------------------------------------------------------------- +//----------------------------------------------------------------------------- +void CheckSizeof::sizeofCalculation() +{ + if (!_settings->isEnabled("warning")) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::simpleMatch(tok, "sizeof (")) { + const Token* const end = tok->linkAt(1); + for (const Token *tok2 = tok->tokAt(2); tok2 != end; tok2 = tok2->next()) { + if (tok2->isConstOp() && (!tok2->isExpandedMacro() || _settings->inconclusive) && !Token::Match(tok2, ">|<|&") && (Token::Match(tok2->previous(), "%var%") || tok2->str() != "*")) { + if (!(Token::Match(tok2->previous(), "%type%") || Token::Match(tok2->next(), "%type%"))) { + sizeofCalculationError(tok2, tok2->isExpandedMacro()); + break; + } + } else if (tok2->type() == Token::eIncDecOp) + sizeofCalculationError(tok2, tok2->isExpandedMacro()); + } + } + } +} + +void CheckSizeof::sizeofCalculationError(const Token *tok, bool inconclusive) +{ + reportError(tok, Severity::warning, + "sizeofCalculation", "Found calculation inside sizeof().", inconclusive); +} + +//----------------------------------------------------------------------------- +// Check for code like sizeof()*sizeof() or sizeof(ptr)/value +//----------------------------------------------------------------------------- +void CheckSizeof::suspiciousSizeofCalculation() +{ + if (!_settings->isEnabled("warning") || !_settings->inconclusive) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::simpleMatch(tok, "sizeof (")) { + const Token* const end = tok->linkAt(1); + const Variable* var = end->previous()->variable(); + if (end->strAt(-1) == "*" || (var && var->isPointer() && !var->isArray())) { + if (end->strAt(1) == "/") + divideSizeofError(tok); + } else if (Token::simpleMatch(end, ") * sizeof")) + multiplySizeofError(tok); + } + } +} + +void CheckSizeof::multiplySizeofError(const Token *tok) +{ + reportError(tok, Severity::warning, + "multiplySizeof", "Multiplying sizeof() with sizeof() indicates a logic error.", true); +} + +void CheckSizeof::divideSizeofError(const Token *tok) +{ + reportError(tok, Severity::warning, + "divideSizeof", "Division of result of sizeof() on pointer type.\n" + "Division of result of sizeof() on pointer type. sizeof() returns the size of the pointer, " + "not the size of the memory area it points to.", true); +} diff --git a/lib/checksizeof.h b/lib/checksizeof.h new file mode 100644 index 000000000..323649acd --- /dev/null +++ b/lib/checksizeof.h @@ -0,0 +1,124 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2013 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +//--------------------------------------------------------------------------- +#ifndef CheckSizeofH +#define CheckSizeofH +//--------------------------------------------------------------------------- + +#include "config.h" +#include "check.h" +#include "settings.h" + +class Token; +class Function; +class Variable; + +/// @addtogroup Checks +/// @{ + + +/** @brief checks on usage of sizeof() operator */ + +class CPPCHECKLIB CheckSizeof : public Check { +public: + /** @brief This constructor is used when registering the CheckClass */ + CheckSizeof() : Check(myName()) + { } + + /** @brief This constructor is used when running checks. */ + CheckSizeof(const Tokenizer* tokenizer, const Settings* settings, ErrorLogger* errorLogger) + : Check(myName(), tokenizer, settings, errorLogger) + { } + + /** @brief Run checks against the normal token list */ + void runChecks(const Tokenizer* tokenizer, const Settings* settings, ErrorLogger* errorLogger) { + CheckSizeof checkSizeof(tokenizer, settings, errorLogger); + + // Checks + checkSizeof.sizeofsizeof(); + checkSizeof.sizeofCalculation(); + checkSizeof.suspiciousSizeofCalculation(); + checkSizeof.checkSizeofForArrayParameter(); + checkSizeof.checkSizeofForPointerSize(); + checkSizeof.checkSizeofForNumericParameter(); + } + + /** @brief Run checks against the simplified token list */ + void runSimplifiedChecks(const Tokenizer*, const Settings*, ErrorLogger*) { + } + + /** @brief %Check for 'sizeof sizeof ..' */ + void sizeofsizeof(); + + /** @brief %Check for calculations inside sizeof */ + void sizeofCalculation(); + + /** @brief %Check for suspicious calculations with sizeof results */ + void suspiciousSizeofCalculation(); + + /** @brief %Check for using sizeof with array given as function argument */ + void checkSizeofForArrayParameter(); + + /** @brief %Check for using sizeof of a variable when allocating it */ + void checkSizeofForPointerSize(); + + /** @brief %Check for using sizeof with numeric given as function argument */ + void checkSizeofForNumericParameter(); + +private: + // Error messages.. + void sizeofsizeofError(const Token* tok); + void sizeofCalculationError(const Token* tok, bool inconclusive); + void multiplySizeofError(const Token* tok); + void divideSizeofError(const Token* tok); + void sizeofForArrayParameterError(const Token* tok); + void sizeofForPointerError(const Token* tok, const std::string &varname); + void sizeofForNumericParameterError(const Token* tok); + + void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const { + CheckSizeof c(0, settings, errorLogger); + + c.sizeofForArrayParameterError(0); + c.sizeofForPointerError(0, "varname"); + c.sizeofForNumericParameterError(0); + c.sizeofsizeofError(0); + c.sizeofCalculationError(0, false); + c.multiplySizeofError(0); + c.divideSizeofError(0); + } + + static std::string myName() { + return "Sizeof"; + } + + std::string classInfo() const { + return "sizeof() usage checks\n" + + "* sizeof for array given as function argument\n" + "* sizeof for numeric given as function argument\n" + "* using sizeof(pointer) instead of the size of pointed data\n" + "* look for 'sizeof sizeof ..'\n" + "* look for calculations inside sizeof()\n" + "* look for suspicious calculations with sizeof()\n"; + } +}; +/// @} +//--------------------------------------------------------------------------- +#endif diff --git a/lib/cppcheck.vcxproj b/lib/cppcheck.vcxproj index 7d2fe4bcf..131110c3d 100644 --- a/lib/cppcheck.vcxproj +++ b/lib/cppcheck.vcxproj @@ -52,6 +52,7 @@ + @@ -90,6 +91,7 @@ + diff --git a/lib/cppcheck.vcxproj.filters b/lib/cppcheck.vcxproj.filters index b6646dd44..f075b3fb2 100644 --- a/lib/cppcheck.vcxproj.filters +++ b/lib/cppcheck.vcxproj.filters @@ -116,6 +116,9 @@ Source Files + + Source Files + @@ -232,6 +235,9 @@ Header Files + + Header Files + diff --git a/test/testother.cpp b/test/testother.cpp index 98a479e2e..1fbd2b7ad 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -81,9 +81,6 @@ private: TEST_CASE(mathfunctionCall1); TEST_CASE(cctypefunctionCall); - TEST_CASE(sizeofsizeof); - TEST_CASE(sizeofCalculation); - TEST_CASE(switchRedundantAssignmentTest); TEST_CASE(switchRedundantOperationTest); TEST_CASE(switchRedundantBitwiseOperationTest); @@ -123,13 +120,8 @@ private: TEST_CASE(memsetZeroBytes); - TEST_CASE(sizeofForArrayParameter); - TEST_CASE(sizeofForNumericParameter); - TEST_CASE(redundantGetAndSetUserId); - TEST_CASE(suspiciousSizeofCalculation); - TEST_CASE(clarifyCalculation); TEST_CASE(clarifyStatement); @@ -156,7 +148,6 @@ private: TEST_CASE(alwaysTrueFalseStringCompare); TEST_CASE(suspiciousStringCompare); - TEST_CASE(checkPointerSizeof); TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfPointer); @@ -1533,49 +1524,6 @@ private: ASSERT_EQUALS("[test.cpp:2]: (error) Passing value -10000 to isgraph() causes undefined behavior which may lead to a crash.\n", errout.str()); } - void sizeofsizeof() { - check("void foo()\n" - "{\n" - " int i = sizeof sizeof char;\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Calling 'sizeof' on 'sizeof'.\n", errout.str()); - - check("void foo()\n" - "{\n" - " int i = sizeof (sizeof long);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Calling 'sizeof' on 'sizeof'.\n", errout.str()); - - check("void foo(long *p)\n" - "{\n" - " int i = sizeof (sizeof (p));\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Calling 'sizeof' on 'sizeof'.\n", errout.str()); - } - - void sizeofCalculation() { - check("int a, b; int a,sizeof(a+b)"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); - - check("int a, b; sizeof(a*b)"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); - - check("int a, b; sizeof(-a)"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); - - check("int a, b; sizeof(*a)"); - ASSERT_EQUALS("", errout.str()); - - check("sizeof(void * const)"); - ASSERT_EQUALS("", errout.str()); - - check("sizeof(foo++)"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); - - check("sizeof(--foo)"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); - } - void switchRedundantAssignmentTest() { check("void foo()\n" @@ -3891,168 +3839,6 @@ private: ASSERT_EQUALS("", errout.str()); } - void sizeofForArrayParameter() { - check("void f() {\n" - " int a[10];\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " unsigned int a = 2;\n" - " unsigned int b = 2;\n" - " int c[(a+b)];\n" - " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " unsigned int a = { 2 };\n" - " unsigned int b[] = { 0 };\n" - " int c[a[b[0]]];\n" - " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - - check("void f() {\n" - " unsigned int a[] = { 1 };\n" - " unsigned int b = 2;\n" - " int c[(a[0]+b)];\n" - " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int a[] = { 1, 2, 3 };\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int a[3] = { 1, 2, 3 };\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check("void f( int a[]) {\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " - "function argument returns size of a pointer.\n", errout.str()); - - check("void f( int a[]) {\n" - " std::cout << sizeof a / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " - "function argument returns size of a pointer.\n", errout.str()); - - check("void f( int a[3] ) {\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " - "function argument returns size of a pointer.\n", errout.str()); - - check("void f(int *p) {\n" - " p[0] = 0;\n" - " int unused = sizeof(p);\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " char p[] = \"test\";\n" - " int unused = sizeof(p);\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - // ticket #2495 - check("void f() {\n" - " static float col[][3]={\n" - " {1,0,0},\n" - " {0,0,1},\n" - " {0,1,0},\n" - " {1,0,1},\n" - " {1,0,1},\n" - " {1,0,1},\n" - " };\n" - " const int COL_MAX=sizeof(col)/sizeof(col[0]);\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - // ticket #155 - check("void f() {\n" - " char buff1[1024*64],buff2[sizeof(buff1)*2];\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - // ticket #2510 - check("void f( int a[], int b) {\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " - "function argument returns size of a pointer.\n", errout.str()); - - // ticket #2510 - check("void f( int a[3] , int b[2] ) {\n" - " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" - "}\n" - ); - ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " - "function argument returns size of a pointer.\n", errout.str()); - - // ticket #2510 - check("void f() {\n" - " char buff1[1024*64],buff2[sizeof(buff1)*(2+1)];\n" - "}\n" - ); - ASSERT_EQUALS("", errout.str()); - - } - - void sizeofForNumericParameter() { - check("void f() {\n" - " std::cout << sizeof(10) << std::endl;\n" - "}\n", - NULL, true - ); - ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); - - check("void f() {\n" - " std::cout << sizeof(-10) << std::endl;\n" - "}\n", - NULL, true - ); - ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); - - check("void f() {\n" - " std::cout << sizeof 10 << std::endl;\n" - "}\n", - NULL, true - ); - ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); - - check("void f() {\n" - " std::cout << sizeof -10 << std::endl;\n" - "}\n", - NULL, true - ); - ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); - } - void redundantGetAndSetUserId() { check("seteuid(geteuid());\n", NULL, false , false, true); ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str()); @@ -4073,37 +3859,6 @@ private: ASSERT_EQUALS("", errout.str()); } - void suspiciousSizeofCalculation() { - check("int* p;\n" - "return sizeof(p)/5;"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n", errout.str()); - - check("unknown p;\n" - "return sizeof(p)/5;"); - ASSERT_EQUALS("", errout.str()); - - check("return sizeof(unknown)/5;"); - ASSERT_EQUALS("", errout.str()); - - check("int p;\n" - "return sizeof(p)/5;"); - ASSERT_EQUALS("", errout.str()); - - check("int* p[5];\n" - "return sizeof(p)/5;"); - ASSERT_EQUALS("", errout.str()); - - - check("return sizeof(foo)*sizeof(bar);"); - ASSERT_EQUALS("[test.cpp:1]: (warning, inconclusive) Multiplying sizeof() with sizeof() indicates a logic error.\n", errout.str()); - - check("return (foo)*sizeof(bar);"); - ASSERT_EQUALS("", errout.str()); - - check("return sizeof(foo)*bar;"); - ASSERT_EQUALS("", errout.str()); - } - void clarifyCalculation() { check("int f(char c) {\n" " return 10 * (c == 0) ? 1 : 2;\n" @@ -4933,208 +4688,6 @@ private: errout.str()); } - void checkPointerSizeof() { - check("void f() {\n" - " char *x = malloc(10);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(*x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(&x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(100 * sizeof(x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(x) * 100);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof *x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(100 * sizeof x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = calloc(1, sizeof(*x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = calloc(1, sizeof *x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = calloc(1, sizeof(x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = calloc(1, sizeof x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = calloc(1, sizeof(int));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " char x[10];\n" - " memset(x, 0, sizeof(x));\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " char* x[10];\n" - " memset(x, 0, sizeof(x));\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " char x[10];\n" - " memset(x, 0, sizeof x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int));\n" - " memset(x, 0, sizeof(int));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int));\n" - " memset(x, 0, sizeof(*x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int));\n" - " memset(x, 0, sizeof *x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int));\n" - " memset(x, 0, sizeof x);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int));\n" - " memset(x, 0, sizeof(x));\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int) * 10);\n" - " memset(x, 0, sizeof(x) * 10);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int) * 10);\n" - " memset(x, 0, sizeof x * 10);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int) * 10);\n" - " memset(x, 0, sizeof(*x) * 10);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int) * 10);\n" - " memset(x, 0, sizeof *x * 10);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " int *x = malloc(sizeof(int) * 10);\n" - " memset(x, 0, sizeof(int) * 10);\n" - " free(x);\n" - "}"); - ASSERT_EQUALS("", errout.str()); - - check( - "int fun(const char *buf1)\n" - "{\n" - " const char *buf1_ex = \"foobarbaz\";\n" - " return strncmp(buf1, buf1_ex, sizeof(buf1_ex)) == 0;\n" - "}"); - ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Size of pointer 'buf1_ex' used instead of size of its data.\n", errout.str()); - - check( - "int fun(const char *buf1) {\n" - " return strncmp(buf1, foo(buf2), sizeof(buf1)) == 0;\n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'buf1' used instead of size of its data.\n", errout.str()); - - // #ticket 3874 - check("void f()\n" - "{\n" - " int * pIntArray[10];\n" - " memset(pIntArray, 0, sizeof(pIntArray));\n" - "}"); - ASSERT_EQUALS("", errout.str()); - } - void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) { // Clear the error buffer.. errout.str(""); diff --git a/test/testrunner.vcxproj b/test/testrunner.vcxproj index 204318eb7..fe2a85217 100644 --- a/test/testrunner.vcxproj +++ b/test/testrunner.vcxproj @@ -62,6 +62,7 @@ + diff --git a/test/testrunner.vcxproj.filters b/test/testrunner.vcxproj.filters index 6d3b86624..0173d744b 100644 --- a/test/testrunner.vcxproj.filters +++ b/test/testrunner.vcxproj.filters @@ -169,6 +169,9 @@ Source Files + + Source Files + diff --git a/test/testsizeof.cpp b/test/testsizeof.cpp new file mode 100644 index 000000000..140816c61 --- /dev/null +++ b/test/testsizeof.cpp @@ -0,0 +1,491 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2013 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +#include "tokenize.h" +#include "checksizeof.h" +#include "testsuite.h" +#include + +extern std::ostringstream errout; + +class TestSizeof : public TestFixture { +public: + TestSizeof() : TestFixture("TestSizeof") + { } + +private: + + + void run() { + TEST_CASE(sizeofsizeof); + TEST_CASE(sizeofCalculation); + TEST_CASE(checkPointerSizeof); + TEST_CASE(sizeofForArrayParameter); + TEST_CASE(sizeofForNumericParameter); + TEST_CASE(suspiciousSizeofCalculation); + } + + void check(const char code[]) { + // Clear the error buffer.. + errout.str(""); + + Settings settings; + settings.addEnabled("warning"); + settings.inconclusive = true; + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + // Check... + CheckSizeof checkSizeof(&tokenizer, &settings, this); + checkSizeof.runChecks(&tokenizer, &settings, this); + } + + void sizeofsizeof() { + check("void foo()\n" + "{\n" + " int i = sizeof sizeof char;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Calling 'sizeof' on 'sizeof'.\n", errout.str()); + + check("void foo()\n" + "{\n" + " int i = sizeof (sizeof long);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Calling 'sizeof' on 'sizeof'.\n", errout.str()); + + check("void foo(long *p)\n" + "{\n" + " int i = sizeof (sizeof (p));\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Calling 'sizeof' on 'sizeof'.\n", errout.str()); + } + + void sizeofCalculation() { + check("int a, b; int a,sizeof(a+b)"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); + + check("int a, b; sizeof(a*b)"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); + + check("int a, b; sizeof(-a)"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); + + check("int a, b; sizeof(*a)"); + ASSERT_EQUALS("", errout.str()); + + check("sizeof(void * const)"); + ASSERT_EQUALS("", errout.str()); + + check("sizeof(foo++)"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); + + check("sizeof(--foo)"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found calculation inside sizeof().\n", errout.str()); + } + + void sizeofForArrayParameter() { + check("void f() {\n" + " int a[10];\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " unsigned int a = 2;\n" + " unsigned int b = 2;\n" + " int c[(a+b)];\n" + " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " unsigned int a = { 2 };\n" + " unsigned int b[] = { 0 };\n" + " int c[a[b[0]]];\n" + " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + + check("void f() {\n" + " unsigned int a[] = { 1 };\n" + " unsigned int b = 2;\n" + " int c[(a[0]+b)];\n" + " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int a[] = { 1, 2, 3 };\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int a[3] = { 1, 2, 3 };\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f( int a[]) {\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " + "function argument returns size of a pointer.\n", errout.str()); + + check("void f( int a[]) {\n" + " std::cout << sizeof a / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " + "function argument returns size of a pointer.\n", errout.str()); + + check("void f( int a[3] ) {\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " + "function argument returns size of a pointer.\n", errout.str()); + + check("void f(int *p) {\n" + " p[0] = 0;\n" + " int unused = sizeof(p);\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " char p[] = \"test\";\n" + " int unused = sizeof(p);\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + // ticket #2495 + check("void f() {\n" + " static float col[][3]={\n" + " {1,0,0},\n" + " {0,0,1},\n" + " {0,1,0},\n" + " {1,0,1},\n" + " {1,0,1},\n" + " {1,0,1},\n" + " };\n" + " const int COL_MAX=sizeof(col)/sizeof(col[0]);\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + // ticket #155 + check("void f() {\n" + " char buff1[1024*64],buff2[sizeof(buff1)*2];\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + // ticket #2510 + check("void f( int a[], int b) {\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " + "function argument returns size of a pointer.\n", errout.str()); + + // ticket #2510 + check("void f( int a[3] , int b[2] ) {\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as " + "function argument returns size of a pointer.\n", errout.str()); + + // ticket #2510 + check("void f() {\n" + " char buff1[1024*64],buff2[sizeof(buff1)*(2+1)];\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + } + + void sizeofForNumericParameter() { + check("void f() {\n" + " std::cout << sizeof(10) << std::endl;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); + + check("void f() {\n" + " std::cout << sizeof(-10) << std::endl;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); + + check("void f() {\n" + " std::cout << sizeof 10 << std::endl;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); + + check("void f() {\n" + " std::cout << sizeof -10 << std::endl;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); + } + + void suspiciousSizeofCalculation() { + check("int* p;\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n", errout.str()); + + check("unknown p;\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("", errout.str()); + + check("return sizeof(unknown)/5;"); + ASSERT_EQUALS("", errout.str()); + + check("int p;\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("", errout.str()); + + check("int* p[5];\n" + "return sizeof(p)/5;"); + ASSERT_EQUALS("", errout.str()); + + + check("return sizeof(foo)*sizeof(bar);"); + ASSERT_EQUALS("[test.cpp:1]: (warning, inconclusive) Multiplying sizeof() with sizeof() indicates a logic error.\n", errout.str()); + + check("return (foo)*sizeof(bar);"); + ASSERT_EQUALS("", errout.str()); + + check("return sizeof(foo)*bar;"); + ASSERT_EQUALS("", errout.str()); + } + + void checkPointerSizeof() { + check("void f() {\n" + " char *x = malloc(10);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(*x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(&x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(100 * sizeof(x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(x) * 100);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof *x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(100 * sizeof x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = calloc(1, sizeof(*x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = calloc(1, sizeof *x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = calloc(1, sizeof(x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = calloc(1, sizeof x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = calloc(1, sizeof(int));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " char x[10];\n" + " memset(x, 0, sizeof(x));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " char* x[10];\n" + " memset(x, 0, sizeof(x));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " char x[10];\n" + " memset(x, 0, sizeof x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " memset(x, 0, sizeof(int));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " memset(x, 0, sizeof(*x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " memset(x, 0, sizeof *x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " memset(x, 0, sizeof x);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " memset(x, 0, sizeof(x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int) * 10);\n" + " memset(x, 0, sizeof(x) * 10);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int) * 10);\n" + " memset(x, 0, sizeof x * 10);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int) * 10);\n" + " memset(x, 0, sizeof(*x) * 10);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int) * 10);\n" + " memset(x, 0, sizeof *x * 10);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int) * 10);\n" + " memset(x, 0, sizeof(int) * 10);\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "int fun(const char *buf1)\n" + "{\n" + " const char *buf1_ex = \"foobarbaz\";\n" + " return strncmp(buf1, buf1_ex, sizeof(buf1_ex)) == 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Size of pointer 'buf1_ex' used instead of size of its data.\n", errout.str()); + + check( + "int fun(const char *buf1) {\n" + " return strncmp(buf1, foo(buf2), sizeof(buf1)) == 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'buf1' used instead of size of its data.\n", errout.str()); + + // #ticket 3874 + check("void f()\n" + "{\n" + " int * pIntArray[10];\n" + " memset(pIntArray, 0, sizeof(pIntArray));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } +}; \ No newline at end of file