diff --git a/lib/checkother.cpp b/lib/checkother.cpp index aa4b57bd3..f7229fa48 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1746,6 +1746,60 @@ void CheckOther::memsetZeroBytesError(const Token *tok, const std::string &varna reportError(tok, Severity::warning, "memsetZeroBytes", summary + "\n" + verbose); } +void CheckOther::checkMemsetInvalid2ndParam() +{ + const bool portability = _settings->isEnabled("portability"); + const bool warning = _settings->isEnabled("warning"); + if (!warning && !portability) + 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 (")) { + const Token* firstParamTok = tok->tokAt(2); + if (!firstParamTok) + continue; + const Token* secondParamTok = firstParamTok->nextArgument(); + if (!secondParamTok) + continue; + const Variable* secondParamVar = secondParamTok->variable(); + + // Check if second parameter is a float variable or a float literal != 0.0f + // TODO: Use AST with astIsFloat() for catch expressions like 'a + 1.0f' + if (portability && + ((secondParamVar && Token::Match(secondParamVar->typeStartToken(), "float|double")) + || (secondParamTok->isNumber() && MathLib::isFloat(secondParamTok->str()) && + MathLib::toDoubleNumber(secondParamTok->str()) != 0.0 && secondParamTok->next()->str() == ","))) { + memsetFloatError(secondParamTok, secondParamTok->str()); + } else if (warning && 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) + 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); +} + +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); +} + //--------------------------------------------------------------------------- // Check scope of variables.. //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index a5c99632c..1580243f1 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -96,6 +96,7 @@ public: checkOther.checkIncorrectLogicOperator(); checkOther.checkMisusedScopedObject(); checkOther.checkMemsetZeroBytes(); + checkOther.checkMemsetInvalid2ndParam(); checkOther.checkIncorrectStringCompare(); checkOther.checkSwitchCaseFallThrough(); checkOther.checkAlwaysTrueOrFalseStringCompare(); @@ -202,6 +203,9 @@ public: /** @brief %Check for filling zero bytes with memset() */ void checkMemsetZeroBytes(); + /** @brief %Check for invalid 2nd parameter of memset() */ + void checkMemsetInvalid2ndParam(); + /** @brief %Check for using bad usage of strncmp and substr */ void checkIncorrectStringCompare(); @@ -303,6 +307,8 @@ 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 memsetFloatError(const Token *tok, const std::string &var_value); + void memsetValueOutOfRangeError(const Token *tok, const std::string &value); 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); @@ -368,6 +374,8 @@ private: c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true); c.redundantConditionError(0, "If x > 10 the condition x > 11 is always true."); c.memsetZeroBytesError(0, "varname"); + c.memsetFloatError(0, "varname"); + c.memsetValueOutOfRangeError(0, "varname"); c.clarifyCalculationError(0, "+"); c.clarifyConditionError(0, true, false); c.clarifyStatementError(0); @@ -415,11 +423,15 @@ private: // warning "* either division by zero or useless condition\n" + "* memset() with a value out of range as the 2nd parameter\n" - //performance + // performance "* redundant data copying for const variable\n" "* subsequent assignment or copying to a variable or buffer\n" + // portability + "* memset() with a float as the 2nd parameter\n" + // style "* Find dead code which is inaccessible due to the counter-conditions check in nested if statements\n" "* C-style pointer cast in cpp file\n" diff --git a/test/testother.cpp b/test/testother.cpp index a3d9a6157..6a17d779a 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -135,6 +135,7 @@ private: TEST_CASE(sameExpression); TEST_CASE(memsetZeroBytes); + TEST_CASE(memsetInvalid2ndParam); TEST_CASE(redundantGetAndSetUserId); @@ -4050,23 +4051,74 @@ private: void memsetZeroBytes() { check("void f() {\n" " memset(p, 10, 0x0);\n" - "}\n" - ); + "}\n"); ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes of 'p'.\n", errout.str()); check("void f() {\n" " memset(p, sizeof(p), 0);\n" - "}\n" - ); + "}\n"); ASSERT_EQUALS("[test.cpp:2]: (warning) memset() called to fill 0 bytes of 'p'.\n", errout.str()); check("void f() {\n" " memset(p, sizeof(p), i);\n" - "}\n" - ); + "}\n"); ASSERT_EQUALS("", 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" + " 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"); + TODO_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 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());