Skip to content

Commit 101dc28

Browse files
danmardanmar
committed
Refactoring: Moved checkMemset.. from CheckOther to CheckFunctions
1 parent f6ab204 commit 101dc28

6 files changed

Lines changed: 208 additions & 206 deletions

File tree

‎lib/checkfunctions.cpp‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ static const CWE CWE252(252U); // Unchecked Return Value
3737
static const CWE CWE477(477U); // Use of Obsolete Functions
3838
static const CWE CWE758(758U); // Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
3939
static const CWE CWE628(628U); // Function Call with Incorrectly Specified Arguments
40+
static const CWE CWE686(686U); // Function Call With Incorrect Argument Type
41+
static const CWE CWE687(687U); // Function Call With Incorrectly Specified Argument Value
42+
static const CWE CWE688(688U); // Function Call With Incorrect Variable or Reference as Argument
4043

4144
void CheckFunctions::checkProhibitedFunctions()
4245
{
@@ -284,6 +287,95 @@ void CheckFunctions::mathfunctionCallWarning(const Token *tok, const std::string
284287
reportError(tok, Severity::style, "unpreciseMathCall", "Expression '" + oldexp + "' can be replaced by '" + newexp + "' to avoid loss of precision.", CWE758, false);
285288
}
286289

290+
//---------------------------------------------------------------------------
291+
// memset(p, y, 0 /* bytes to fill */) <- 2nd and 3rd arguments inverted
292+
//---------------------------------------------------------------------------
293+
void CheckFunctions::memsetZeroBytes()
294+
{
295+
if (!_settings->isEnabled(Settings::WARNING))
296+
return;
297+
298+
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
299+
const std::size_t functions = symbolDatabase->functionScopes.size();
300+
for (std::size_t i = 0; i < functions; ++i) {
301+
const Scope * scope = symbolDatabase->functionScopes[i];
302+
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
303+
if (Token::Match(tok, "memset|wmemset (") && (numberOfArguments(tok)==3)) {
304+
const Token* lastParamTok = getArguments(tok)[2];
305+
if (lastParamTok->str() == "0")
306+
memsetZeroBytesError(tok);
307+
}
308+
}
309+
}
310+
}
311+
312+
void CheckFunctions::memsetZeroBytesError(const Token *tok)
313+
{
314+
const std::string summary("memset() called to fill 0 bytes.");
315+
const std::string verbose(summary + " The second and third arguments might be inverted."
316+
" The function memset ( void * ptr, int value, size_t num ) sets the"
317+
" first num bytes of the block of memory pointed by ptr to the specified value.");
318+
reportError(tok, Severity::warning, "memsetZeroBytes", summary + "\n" + verbose, CWE687, false);
319+
}
320+
321+
void CheckFunctions::memsetInvalid2ndParam()
322+
{
323+
const bool printPortability = _settings->isEnabled(Settings::PORTABILITY);
324+
const bool printWarning = _settings->isEnabled(Settings::WARNING);
325+
if (!printWarning && !printPortability)
326+
return;
327+
328+
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
329+
const std::size_t functions = symbolDatabase->functionScopes.size();
330+
for (std::size_t i = 0; i < functions; ++i) {
331+
const Scope * scope = symbolDatabase->functionScopes[i];
332+
for (const Token* tok = scope->classStart->next(); tok && (tok != scope->classEnd); tok = tok->next()) {
333+
if (!Token::simpleMatch(tok, "memset ("))
334+
continue;
335+
336+
const std::vector<const Token *> args = getArguments(tok);
337+
if (args.size() != 3)
338+
continue;
339+
340+
// Second parameter is zero literal, i.e. 0.0f
341+
const Token * const secondParamTok = args[1];
342+
if (Token::Match(secondParamTok, "%num% ,") && MathLib::isNullValue(secondParamTok->str()))
343+
continue;
344+
345+
// Check if second parameter is a float variable or a float literal != 0.0f
346+
if (printPortability && astIsFloat(secondParamTok,false)) {
347+
memsetFloatError(secondParamTok, secondParamTok->expressionString());
348+
}
349+
350+
if (printWarning && secondParamTok->isNumber()) { // Check if the second parameter is a literal and is out of range
351+
const long long int value = MathLib::toLongNumber(secondParamTok->str());
352+
if (value < -128 || value > 255) // FIXME: Use platform char_bits
353+
memsetValueOutOfRangeError(secondParamTok, secondParamTok->str());
354+
}
355+
}
356+
}
357+
}
358+
359+
void CheckFunctions::memsetFloatError(const Token *tok, const std::string &var_value)
360+
{
361+
const std::string message("The 2nd memset() argument '" + var_value +
362+
"' is a float, its representation is implementation defined.");
363+
const std::string verbose(message + " memset() is used to set each byte of a block of memory to a specific value and"
364+
" the actual representation of a floating-point value is implementation defined.");
365+
reportError(tok, Severity::portability, "memsetFloat", message + "\n" + verbose, CWE688, false);
366+
}
367+
368+
void CheckFunctions::memsetValueOutOfRangeError(const Token *tok, const std::string &value)
369+
{
370+
const std::string message("The 2nd memset() argument '" + value + "' doesn't fit into an 'unsigned char'.");
371+
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.");
372+
reportError(tok, Severity::warning, "memsetValueOutOfRange", message + "\n" + verbose, CWE686, false);
373+
}
374+
375+
//---------------------------------------------------------------------------
376+
// --check-library => warn for unconfigured functions
377+
//---------------------------------------------------------------------------
378+
287379
void CheckFunctions::checkLibraryMatchFunctions()
288380
{
289381
if (!_settings->checkLibrary || !_settings->isEnabled(Settings::INFORMATION))

‎lib/checkfunctions.h‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ class CPPCHECKLIB CheckFunctions : public Check {
6363
checkFunctions.checkProhibitedFunctions();
6464
checkFunctions.invalidFunctionUsage();
6565
checkFunctions.checkMathFunctions();
66+
checkFunctions.memsetZeroBytes();
67+
checkFunctions.memsetInvalid2ndParam();
6668
}
6769

6870
/** Check for functions that should not be used */
@@ -84,6 +86,12 @@ class CPPCHECKLIB CheckFunctions : public Check {
8486
/** @brief %Check for parameters given to math function that do not make sense*/
8587
void checkMathFunctions();
8688

89+
/** @brief %Check for filling zero bytes with memset() */
90+
void memsetZeroBytes();
91+
92+
/** @brief %Check for invalid 2nd parameter of memset() */
93+
void memsetInvalid2ndParam();
94+
8795
/** @brief --check-library: warn for unconfigured function calls */
8896
void checkLibraryMatchFunctions();
8997

@@ -93,6 +101,9 @@ class CPPCHECKLIB CheckFunctions : public Check {
93101
void ignoredReturnValueError(const Token* tok, const std::string& function);
94102
void mathfunctionCallWarning(const Token *tok, const unsigned int numParam = 1);
95103
void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp);
104+
void memsetZeroBytesError(const Token *tok);
105+
void memsetFloatError(const Token *tok, const std::string &var_value);
106+
void memsetValueOutOfRangeError(const Token *tok, const std::string &value);
96107

97108
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
98109
CheckFunctions c(nullptr, settings, errorLogger);
@@ -106,6 +117,9 @@ class CPPCHECKLIB CheckFunctions : public Check {
106117
c.ignoredReturnValueError(nullptr, "malloc");
107118
c.mathfunctionCallWarning(nullptr);
108119
c.mathfunctionCallWarning(nullptr, "1 - erf(x)", "erfc(x)");
120+
c.memsetZeroBytesError(nullptr);
121+
c.memsetFloatError(nullptr, "varname");
122+
c.memsetValueOutOfRangeError(nullptr, "varname");
109123
}
110124

111125
static std::string myName() {
@@ -116,7 +130,10 @@ class CPPCHECKLIB CheckFunctions : public Check {
116130
return "Check function usage:\n"
117131
"- return value of certain functions not used\n"
118132
"- invalid input values for functions\n"
119-
"- Warn if a function is called whose usage is discouraged\n";
133+
"- Warn if a function is called whose usage is discouraged\n"
134+
"- memset() third argument is zero\n"
135+
"- memset() with a value out of range as the 2nd parameter\n"
136+
"- memset() with a float as the 2nd parameter\n";
120137
}
121138
};
122139
/// @}

‎lib/checkother.cpp‎

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ static const struct CWE CWE672(672U); // Operation on a Resource after Expirat
4949
static const struct CWE CWE628(628U); // Function Call with Incorrectly Specified Arguments
5050
static const struct CWE CWE683(683U); // Function Call With Incorrect Order of Arguments
5151
static const struct CWE CWE686(686U); // Function Call With Incorrect Argument Type
52-
static const struct CWE CWE687(687U); // Function Call With Incorrectly Specified Argument Value
53-
static const struct CWE CWE688(688U); // Function Call With Incorrect Variable or Reference as Argument
5452
static const struct CWE CWE704(704U); // Incorrect Type Conversion or Cast
5553
static const struct CWE CWE758(758U); // Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
5654
static const struct CWE CWE768(768U); // Incorrect Short Circuit Evaluation
@@ -1084,91 +1082,6 @@ void CheckOther::unreachableCodeError(const Token *tok, bool inconclusive)
10841082
"Statements following return, break, continue, goto or throw will never be executed.", CWE561, inconclusive);
10851083
}
10861084

1087-
//---------------------------------------------------------------------------
1088-
// memset(p, y, 0 /* bytes to fill */) <- 2nd and 3rd arguments inverted
1089-
//---------------------------------------------------------------------------
1090-
void CheckOther::checkMemsetZeroBytes()
1091-
{
1092-
if (!_settings->isEnabled(Settings::WARNING))
1093-
return;
1094-
1095-
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
1096-
const std::size_t functions = symbolDatabase->functionScopes.size();
1097-
for (std::size_t i = 0; i < functions; ++i) {
1098-
const Scope * scope = symbolDatabase->functionScopes[i];
1099-
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
1100-
if (Token::Match(tok, "memset|wmemset (") && (numberOfArguments(tok)==3)) {
1101-
const Token* lastParamTok = tok->next()->link()->previous();
1102-
if (lastParamTok->str() == "0")
1103-
memsetZeroBytesError(tok);
1104-
}
1105-
}
1106-
}
1107-
}
1108-
1109-
void CheckOther::memsetZeroBytesError(const Token *tok)
1110-
{
1111-
const std::string summary("memset() called to fill 0 bytes.");
1112-
const std::string verbose(summary + " The second and third arguments might be inverted."
1113-
" The function memset ( void * ptr, int value, size_t num ) sets the"
1114-
" first num bytes of the block of memory pointed by ptr to the specified value.");
1115-
reportError(tok, Severity::warning, "memsetZeroBytes", summary + "\n" + verbose, CWE687, false);
1116-
}
1117-
1118-
void CheckOther::checkMemsetInvalid2ndParam()
1119-
{
1120-
const bool printPortability = _settings->isEnabled(Settings::PORTABILITY);
1121-
const bool printWarning = _settings->isEnabled(Settings::WARNING);
1122-
if (!printWarning && !printPortability)
1123-
return;
1124-
1125-
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
1126-
const std::size_t functions = symbolDatabase->functionScopes.size();
1127-
for (std::size_t i = 0; i < functions; ++i) {
1128-
const Scope * scope = symbolDatabase->functionScopes[i];
1129-
for (const Token* tok = scope->classStart->next(); tok && (tok != scope->classEnd); tok = tok->next()) {
1130-
if (!Token::simpleMatch(tok, "memset ("))
1131-
continue;
1132-
1133-
const std::vector<const Token *> args = getArguments(tok);
1134-
if (args.size() != 3)
1135-
continue;
1136-
1137-
// Second parameter is zero literal, i.e. 0.0f
1138-
const Token * const secondParamTok = args[1];
1139-
if (Token::Match(secondParamTok, "%num% ,") && MathLib::isNullValue(secondParamTok->str()))
1140-
continue;
1141-
1142-
// Check if second parameter is a float variable or a float literal != 0.0f
1143-
if (printPortability && astIsFloat(secondParamTok,false)) {
1144-
memsetFloatError(secondParamTok, secondParamTok->expressionString());
1145-
}
1146-
1147-
if (printWarning && secondParamTok->isNumber()) { // Check if the second parameter is a literal and is out of range
1148-
const long long int value = MathLib::toLongNumber(secondParamTok->str());
1149-
if (value < -128 || value > 255) // FIXME: Use platform char_bits
1150-
memsetValueOutOfRangeError(secondParamTok, secondParamTok->str());
1151-
}
1152-
}
1153-
}
1154-
}
1155-
1156-
void CheckOther::memsetFloatError(const Token *tok, const std::string &var_value)
1157-
{
1158-
const std::string message("The 2nd memset() argument '" + var_value +
1159-
"' is a float, its representation is implementation defined.");
1160-
const std::string verbose(message + " memset() is used to set each byte of a block of memory to a specific value and"
1161-
" the actual representation of a floating-point value is implementation defined.");
1162-
reportError(tok, Severity::portability, "memsetFloat", message + "\n" + verbose, CWE688, false);
1163-
}
1164-
1165-
void CheckOther::memsetValueOutOfRangeError(const Token *tok, const std::string &value)
1166-
{
1167-
const std::string message("The 2nd memset() argument '" + value + "' doesn't fit into an 'unsigned char'.");
1168-
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.");
1169-
reportError(tok, Severity::warning, "memsetValueOutOfRange", message + "\n" + verbose, CWE686, false);
1170-
}
1171-
11721085
//---------------------------------------------------------------------------
11731086
// Check scope of variables..
11741087
//---------------------------------------------------------------------------

‎lib/checkother.h‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,6 @@ class CPPCHECKLIB CheckOther : public Check {
8787
checkOther.checkCastIntToCharAndBack();
8888

8989
checkOther.checkMisusedScopedObject();
90-
checkOther.checkMemsetZeroBytes();
91-
checkOther.checkMemsetInvalid2ndParam();
9290
checkOther.checkPipeParameterSize();
9391

9492
checkOther.checkInvalidFree();
@@ -147,12 +145,6 @@ class CPPCHECKLIB CheckOther : public Check {
147145
/** @brief %Check for objects that are destroyed immediately */
148146
void checkMisusedScopedObject();
149147

150-
/** @brief %Check for filling zero bytes with memset() */
151-
void checkMemsetZeroBytes();
152-
153-
/** @brief %Check for invalid 2nd parameter of memset() */
154-
void checkMemsetInvalid2ndParam();
155-
156148
/** @brief %Check for suspicious code where if and else branch are the same (e.g "if (a) b = true; else b = true;") */
157149
void checkDuplicateBranch();
158150

@@ -238,9 +230,6 @@ class CPPCHECKLIB CheckOther : public Check {
238230
void suspiciousEqualityComparisonError(const Token* tok);
239231
void selfAssignmentError(const Token *tok, const std::string &varname);
240232
void misusedScopeObjectError(const Token *tok, const std::string &varname);
241-
void memsetZeroBytesError(const Token *tok);
242-
void memsetFloatError(const Token *tok, const std::string &var_value);
243-
void memsetValueOutOfRangeError(const Token *tok, const std::string &value);
244233
void duplicateBranchError(const Token *tok1, const Token *tok2);
245234
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
246235
void duplicateExpressionTernaryError(const Token *tok);
@@ -298,9 +287,6 @@ class CPPCHECKLIB CheckOther : public Check {
298287
c.suspiciousCaseInSwitchError(nullptr, "||");
299288
c.suspiciousEqualityComparisonError(nullptr);
300289
c.selfAssignmentError(nullptr, "varname");
301-
c.memsetZeroBytesError(nullptr);
302-
c.memsetFloatError(nullptr, "varname");
303-
c.memsetValueOutOfRangeError(nullptr, "varname");
304290
c.clarifyCalculationError(nullptr, "+");
305291
c.clarifyStatementError(nullptr);
306292
c.duplicateBranchError(nullptr, 0);
@@ -349,7 +335,6 @@ class CPPCHECKLIB CheckOther : public Check {
349335

350336
// warning
351337
"- either division by zero or useless condition\n"
352-
"- memset() with a value out of range as the 2nd parameter\n"
353338
"- access of moved or forwarded variable.\n"
354339

355340
// performance
@@ -358,7 +343,6 @@ class CPPCHECKLIB CheckOther : public Check {
358343
"- passing parameter by value\n"
359344

360345
// portability
361-
"- memset() with a float as the 2nd parameter\n"
362346
"- Passing NULL pointer to function with variable number of arguments leads to UB.\n"
363347

364348
// style

0 commit comments

Comments
 (0)