Fixed #4818 (New check: Check memset() 2nd parameter)

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..
 //---------------------------------------------------------------------------

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"

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());