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

Refactoring: Moved checkMemset.. from CheckOther to CheckFunctions

This commit is contained in:
Daniel Marjamäki 2017-04-23 07:53:41 +02:00
parent f6ab204dc6
commit 101dc28afa
6 changed files with 208 additions and 206 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
lib/checkfunctions.cpp
View File

@ -37,6 +37,9 @@ static const CWE CWE252(252U); // Unchecked Return Value
static const CWE CWE477(477U); // Use of Obsolete Functions
static const CWE CWE758(758U); // Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
static const CWE CWE628(628U); // Function Call with Incorrectly Specified Arguments
static const CWE CWE686(686U); // Function Call With Incorrect Argument Type
static const CWE CWE687(687U); // Function Call With Incorrectly Specified Argument Value
static const CWE CWE688(688U); // Function Call With Incorrect Variable or Reference as Argument
void CheckFunctions::checkProhibitedFunctions()
{
@ -284,6 +287,95 @@ void CheckFunctions::mathfunctionCallWarning(const Token *tok, const std::string
reportError(tok, Severity::style, "unpreciseMathCall", "Expression '" + oldexp + "' can be replaced by '" + newexp + "' to avoid loss of precision.", CWE758, false);
}
//---------------------------------------------------------------------------
// memset(p, y, 0 /* bytes to fill */) <- 2nd and 3rd arguments inverted
//---------------------------------------------------------------------------
void CheckFunctions::memsetZeroBytes()
{
if (!_settings->isEnabled(Settings::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, "memset|wmemset (") && (numberOfArguments(tok)==3)) {
const Token* lastParamTok = getArguments(tok)[2];
if (lastParamTok->str() == "0")
memsetZeroBytesError(tok);
}
}
}
}
void CheckFunctions::memsetZeroBytesError(const Token *tok)
{
const std::string summary("memset() called to fill 0 bytes.");
const std::string verbose(summary + " The second and third arguments might be inverted."
" The function memset ( void * ptr, int value, size_t num ) sets the"
" first num bytes of the block of memory pointed by ptr to the specified value.");
reportError(tok, Severity::warning, "memsetZeroBytes", summary + "\n" + verbose, CWE687, false);
}
void CheckFunctions::memsetInvalid2ndParam()
{
const bool printPortability = _settings->isEnabled(Settings::PORTABILITY);
const bool printWarning = _settings->isEnabled(Settings::WARNING);
if (!printWarning && !printPortability)
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 && (tok != scope->classEnd); tok = tok->next()) {
if (!Token::simpleMatch(tok, "memset ("))
continue;
const std::vector<const Token *> args = getArguments(tok);
if (args.size() != 3)
continue;
// Second parameter is zero literal, i.e. 0.0f
const Token * const secondParamTok = args[1];
if (Token::Match(secondParamTok, "%num% ,") && MathLib::isNullValue(secondParamTok->str()))
continue;
// Check if second parameter is a float variable or a float literal != 0.0f
if (printPortability && astIsFloat(secondParamTok,false)) {
memsetFloatError(secondParamTok, secondParamTok->expressionString());
}
if (printWarning && secondParamTok->isNumber()) { // Check if the second parameter is a literal and is out of range
const long long int value = MathLib::toLongNumber(secondParamTok->str());
if (value < -128 || value > 255) // FIXME: Use platform char_bits
memsetValueOutOfRangeError(secondParamTok, secondParamTok->str());
}
}
}
}
void CheckFunctions::memsetFloatError(const Token *tok, const std::string &var_value)
{
const std::string message("The 2nd memset() argument '" + var_value +
"' is a float, its representation is implementation defined.");
const std::string verbose(message + " memset() is used to set each byte of a block of memory to a specific value and"
" the actual representation of a floating-point value is implementation defined.");
reportError(tok, Severity::portability, "memsetFloat", message + "\n" + verbose, CWE688, false);
}
void CheckFunctions::memsetValueOutOfRangeError(const Token *tok, const std::string &value)
{
const std::string message("The 2nd memset() argument '" + value + "' doesn't fit into an 'unsigned char'.");
const std::string verbose(message + " The 2nd parameter is passed as an 'int', but the function fills the block of memory using the 'unsigned char' conversion of this value.");
reportError(tok, Severity::warning, "memsetValueOutOfRange", message + "\n" + verbose, CWE686, false);
}
//---------------------------------------------------------------------------
// --check-library => warn for unconfigured functions
//---------------------------------------------------------------------------
void CheckFunctions::checkLibraryMatchFunctions()
{
if (!_settings->checkLibrary || !_settings->isEnabled(Settings::INFORMATION))

19
lib/checkfunctions.h
View File

@ -63,6 +63,8 @@ public:
checkFunctions.checkProhibitedFunctions();
checkFunctions.invalidFunctionUsage();
checkFunctions.checkMathFunctions();
checkFunctions.memsetZeroBytes();
checkFunctions.memsetInvalid2ndParam();
}
/** Check for functions that should not be used */
@ -84,6 +86,12 @@ public:
/** @brief %Check for parameters given to math function that do not make sense*/
void checkMathFunctions();
/** @brief %Check for filling zero bytes with memset() */
void memsetZeroBytes();
/** @brief %Check for invalid 2nd parameter of memset() */
void memsetInvalid2ndParam();
/** @brief --check-library: warn for unconfigured function calls */
void checkLibraryMatchFunctions();
@ -93,6 +101,9 @@ private:
void ignoredReturnValueError(const Token* tok, const std::string& function);
void mathfunctionCallWarning(const Token *tok, const unsigned int numParam = 1);
void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp);
void memsetZeroBytesError(const Token *tok);
void memsetFloatError(const Token *tok, const std::string &var_value);
void memsetValueOutOfRangeError(const Token *tok, const std::string &value);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckFunctions c(nullptr, settings, errorLogger);
@ -106,6 +117,9 @@ private:
c.ignoredReturnValueError(nullptr, "malloc");
c.mathfunctionCallWarning(nullptr);
c.mathfunctionCallWarning(nullptr, "1 - erf(x)", "erfc(x)");
c.memsetZeroBytesError(nullptr);
c.memsetFloatError(nullptr, "varname");
c.memsetValueOutOfRangeError(nullptr, "varname");
}
static std::string myName() {
@ -116,7 +130,10 @@ private:
return "Check function usage:\n"
"- return value of certain functions not used\n"
"- invalid input values for functions\n"
"- Warn if a function is called whose usage is discouraged\n";
"- Warn if a function is called whose usage is discouraged\n"
"- memset() third argument is zero\n"
"- memset() with a value out of range as the 2nd parameter\n"
"- memset() with a float as the 2nd parameter\n";
}
};
/// @}

87
lib/checkother.cpp
View File

@ -49,8 +49,6 @@ static const struct CWE CWE672(672U); // Operation on a Resource after Expirat
static const struct CWE CWE628(628U); // Function Call with Incorrectly Specified Arguments
static const struct CWE CWE683(683U); // Function Call With Incorrect Order of Arguments
static const struct CWE CWE686(686U); // Function Call With Incorrect Argument Type
static const struct CWE CWE687(687U); // Function Call With Incorrectly Specified Argument Value
static const struct CWE CWE688(688U); // Function Call With Incorrect Variable or Reference as Argument
static const struct CWE CWE704(704U); // Incorrect Type Conversion or Cast
static const struct CWE CWE758(758U); // Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
static const struct CWE CWE768(768U); // Incorrect Short Circuit Evaluation
@ -1084,91 +1082,6 @@ void CheckOther::unreachableCodeError(const Token *tok, bool inconclusive)
"Statements following return, break, continue, goto or throw will never be executed.", CWE561, inconclusive);
}
//---------------------------------------------------------------------------
// memset(p, y, 0 /* bytes to fill */) <- 2nd and 3rd arguments inverted
//---------------------------------------------------------------------------
void CheckOther::checkMemsetZeroBytes()
{
if (!_settings->isEnabled(Settings::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, "memset|wmemset (") && (numberOfArguments(tok)==3)) {
const Token* lastParamTok = tok->next()->link()->previous();
if (lastParamTok->str() == "0")
memsetZeroBytesError(tok);
}
}
}
}
void CheckOther::memsetZeroBytesError(const Token *tok)
{
const std::string summary("memset() called to fill 0 bytes.");
const std::string verbose(summary + " The second and third arguments might be inverted."
" The function memset ( void * ptr, int value, size_t num ) sets the"
" first num bytes of the block of memory pointed by ptr to the specified value.");
reportError(tok, Severity::warning, "memsetZeroBytes", summary + "\n" + verbose, CWE687, false);
}
void CheckOther::checkMemsetInvalid2ndParam()
{
const bool printPortability = _settings->isEnabled(Settings::PORTABILITY);
const bool printWarning = _settings->isEnabled(Settings::WARNING);
if (!printWarning && !printPortability)
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 && (tok != scope->classEnd); tok = tok->next()) {
if (!Token::simpleMatch(tok, "memset ("))
continue;
const std::vector<const Token *> args = getArguments(tok);
if (args.size() != 3)
continue;
// Second parameter is zero literal, i.e. 0.0f
const Token * const secondParamTok = args[1];
if (Token::Match(secondParamTok, "%num% ,") && MathLib::isNullValue(secondParamTok->str()))
continue;
// Check if second parameter is a float variable or a float literal != 0.0f
if (printPortability && astIsFloat(secondParamTok,false)) {
memsetFloatError(secondParamTok, secondParamTok->expressionString());
}
if (printWarning && secondParamTok->isNumber()) { // Check if the second parameter is a literal and is out of range
const long long int value = MathLib::toLongNumber(secondParamTok->str());
if (value < -128 || value > 255) // FIXME: Use platform char_bits
memsetValueOutOfRangeError(secondParamTok, secondParamTok->str());
}
}
}
}
void CheckOther::memsetFloatError(const Token *tok, const std::string &var_value)
{
const std::string message("The 2nd memset() argument '" + var_value +
"' is a float, its representation is implementation defined.");
const std::string verbose(message + " memset() is used to set each byte of a block of memory to a specific value and"
" the actual representation of a floating-point value is implementation defined.");
reportError(tok, Severity::portability, "memsetFloat", message + "\n" + verbose, CWE688, false);
}
void CheckOther::memsetValueOutOfRangeError(const Token *tok, const std::string &value)
{
const std::string message("The 2nd memset() argument '" + value + "' doesn't fit into an 'unsigned char'.");
const std::string verbose(message + " The 2nd parameter is passed as an 'int', but the function fills the block of memory using the 'unsigned char' conversion of this value.");
reportError(tok, Severity::warning, "memsetValueOutOfRange", message + "\n" + verbose, CWE686, false);
}
//---------------------------------------------------------------------------
// Check scope of variables..
//---------------------------------------------------------------------------

16
lib/checkother.h
View File

@ -87,8 +87,6 @@ public:
checkOther.checkCastIntToCharAndBack();
checkOther.checkMisusedScopedObject();
checkOther.checkMemsetZeroBytes();
checkOther.checkMemsetInvalid2ndParam();
checkOther.checkPipeParameterSize();
checkOther.checkInvalidFree();
@ -147,12 +145,6 @@ public:
/** @brief %Check for objects that are destroyed immediately */
void checkMisusedScopedObject();
/** @brief %Check for filling zero bytes with memset() */
void checkMemsetZeroBytes();
/** @brief %Check for invalid 2nd parameter of memset() */
void checkMemsetInvalid2ndParam();
/** @brief %Check for suspicious code where if and else branch are the same (e.g "if (a) b = true; else b = true;") */
void checkDuplicateBranch();
@ -238,9 +230,6 @@ private:
void suspiciousEqualityComparisonError(const Token* tok);
void selfAssignmentError(const Token *tok, const std::string &varname);
void misusedScopeObjectError(const Token *tok, const std::string &varname);
void memsetZeroBytesError(const Token *tok);
void memsetFloatError(const Token *tok, const std::string &var_value);
void memsetValueOutOfRangeError(const Token *tok, const std::string &value);
void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void duplicateExpressionTernaryError(const Token *tok);
@ -298,9 +287,6 @@ private:
c.suspiciousCaseInSwitchError(nullptr, "||");
c.suspiciousEqualityComparisonError(nullptr);
c.selfAssignmentError(nullptr, "varname");
c.memsetZeroBytesError(nullptr);
c.memsetFloatError(nullptr, "varname");
c.memsetValueOutOfRangeError(nullptr, "varname");
c.clarifyCalculationError(nullptr, "+");
c.clarifyStatementError(nullptr);
c.duplicateBranchError(nullptr, 0);
@ -349,7 +335,6 @@ private:
// warning
"- either division by zero or useless condition\n"
"- memset() with a value out of range as the 2nd parameter\n"
"- access of moved or forwarded variable.\n"
// performance
@ -358,7 +343,6 @@ private:
"- passing parameter by value\n"
// portability
"- memset() with a float as the 2nd parameter\n"
"- Passing NULL pointer to function with variable number of arguments leads to UB.\n"
// style

104
test/testfunctions.cpp
View File

@ -71,6 +71,10 @@ private:
// Ignored return value
TEST_CASE(checkIgnoredReturnValue);
// memset..
TEST_CASE(memsetZeroBytes);
TEST_CASE(memsetInvalid2ndParam);
}
void check(const char code[], const char filename[]="test.cpp", const Settings* settings_=nullptr) {
@ -86,17 +90,13 @@ private:
tokenizer.tokenize(istr, filename);
CheckFunctions checkFunctions(&tokenizer, settings_, this);
// Check...
checkFunctions.checkIgnoredReturnValue();
checkFunctions.runChecks(&tokenizer, settings_, this);
// Simplify...
tokenizer.simplifyTokenList2();
// Check...
checkFunctions.checkProhibitedFunctions();
checkFunctions.checkMathFunctions();
checkFunctions.invalidFunctionUsage();
checkFunctions.runSimplifiedChecks(&tokenizer, settings_, this);
}
void prohibitedFunctions_posix() {
@ -922,7 +922,99 @@ private:
check("void foo() {\n" // don't crash
" DEBUG(123)(mystrcmp(a,b))(fd);\n"
"}", "test.c", &settings2);
}
void memsetZeroBytes() {
check("void f() {\n"
" memset(p, 10, 0x0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes.\n", errout.str());
check("void f() {\n"
" memset(p, sizeof(p), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes.\n", errout.str());
check("void f() {\n"
" memset(p, sizeof(p), i);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
// #6269 false positives in case of overloaded standard library functions
check("class c {\n"
" void memset( int i );\n"
" void f( void ) {\n"
" memset( 0 );\n"
" }\n"
"};");
ASSERT_EQUALS("", errout.str());
// #7285
check("void f() {\n"
" memset(&tm, sizeof(tm), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes.\n", errout.str());
}
void memsetInvalid2ndParam() {
check("void f() {\n"
" int* is = new int[10];\n"
" memset(is, 1.0f, 40);\n"
" int* is2 = new int[10];\n"
" memset(is2, 0.1f, 40);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (portability) The 2nd memset() argument '1.0f' is a float, its representation is implementation defined.\n"
"[test.cpp:5]: (portability) The 2nd memset() argument '0.1f' is a float, its representation is implementation defined.\n", errout.str());
check("void f() {\n"
" int* is = new int[10];\n"
" float g = computeG();\n"
" memset(is, g, 40);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument 'g' is a float, its representation is implementation defined.\n", errout.str());
check("void f() {\n"
" int* is = new int[10];\n"
" memset(is, 0.0f, 40);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n" // FP
" float x = 2.3f;\n"
" memset(a, (x?64:0), 40);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" short ss[] = {1, 2};\n"
" memset(ss, 256, 4);\n"
" short ss2[2];\n"
" memset(ss2, -129, 4);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (warning) The 2nd memset() argument '256' doesn't fit into an 'unsigned char'.\n"
"[test.cpp:5]: (warning) The 2nd memset() argument '-129' doesn't fit into an 'unsigned char'.\n", errout.str());
check("void f() {\n"
" int is[10];\n"
" memset(is, 0xEE, 40);\n"
" unsigned char* cs = malloc(256);\n"
" memset(cs, -1, 256);\n"
" short* ss[30];\n"
" memset(ss, -128, 60);\n"
" char cs2[30];\n"
" memset(cs2, 255, 30);\n"
" char cs3[30];\n"
" memset(cs3, 0, 30);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int is[10];\n"
" const int i = g();\n"
" memset(is, 1.0f + i, 40);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument '1.0f+i' is a float, its representation is implementation defined.\n", errout.str());
}
};

96
test/testother.cpp
View File

@ -111,9 +111,6 @@ private:
TEST_CASE(trac2084);
TEST_CASE(trac3693);
TEST_CASE(memsetZeroBytes);
TEST_CASE(memsetInvalid2ndParam);
TEST_CASE(clarifyCalculation);
TEST_CASE(clarifyStatement);
@ -3123,99 +3120,6 @@ private:
ASSERT_EQUALS("", errout.str());
}
void memsetZeroBytes() {
check("void f() {\n"
" memset(p, 10, 0x0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes.\n", errout.str());
check("void f() {\n"
" memset(p, sizeof(p), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes.\n", errout.str());
check("void f() {\n"
" memset(p, sizeof(p), i);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
// #6269 false positives in case of overloaded standard library functions
check("class c {\n"
" void memset( int i );\n"
" void f( void ) {\n"
" memset( 0 );\n"
" }\n"
"};");
ASSERT_EQUALS("", errout.str());
// #7285
check("void f() {\n"
" memset(&tm, sizeof(tm), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes.\n", errout.str());
}
void memsetInvalid2ndParam() {
check("void f() {\n"
" int* is = new int[10];\n"
" memset(is, 1.0f, 40);\n"
" int* is2 = new int[10];\n"
" memset(is2, 0.1f, 40);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (portability) The 2nd memset() argument '1.0f' is a float, its representation is implementation defined.\n"
"[test.cpp:5]: (portability) The 2nd memset() argument '0.1f' is a float, its representation is implementation defined.\n", errout.str());
check("void f() {\n"
" int* is = new int[10];\n"
" float g = computeG();\n"
" memset(is, g, 40);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument 'g' is a float, its representation is implementation defined.\n", errout.str());
check("void f() {\n"
" int* is = new int[10];\n"
" memset(is, 0.0f, 40);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n" // FP
" float x = 2.3f;\n"
" memset(a, (x?64:0), 40);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" short ss[] = {1, 2};\n"
" memset(ss, 256, 4);\n"
" short ss2[2];\n"
" memset(ss2, -129, 4);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (warning) The 2nd memset() argument '256' doesn't fit into an 'unsigned char'.\n"
"[test.cpp:5]: (warning) The 2nd memset() argument '-129' doesn't fit into an 'unsigned char'.\n", errout.str());
check("void f() {\n"
" int is[10];\n"
" memset(is, 0xEE, 40);\n"
" unsigned char* cs = malloc(256);\n"
" memset(cs, -1, 256);\n"
" short* ss[30];\n"
" memset(ss, -128, 60);\n"
" char cs2[30];\n"
" memset(cs2, 255, 30);\n"
" char cs3[30];\n"
" memset(cs3, 0, 30);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int is[10];\n"
" const int i = g();\n"
" memset(is, 1.0f + i, 40);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument '1.0f+i' is a float, its representation is implementation defined.\n", errout.str());
}
void clarifyCalculation() {
check("int f(char c) {\n"
" return 10 * (c == 0) ? 1 : 2;\n"