New check: Warn about using malloc() for classes containing virtual methods, std::-objects or constructors

lib/checkclass.cpp

@@ -817,9 +817,7 @@ void CheckClass::noMemset()
     for (std::size_t i = 0; i < functions; ++i) {
         const Scope * scope = symbolDatabase->functionScopes[i];
         for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
-            if (!Token::Match(tok, "memset|memcpy|memmove ( %any%"))
+            if (Token::Match(tok, "memset|memcpy|memmove ( %any%")) {
-                continue;
-
                 const Token* arg1 = tok->tokAt(2);
                 const Token* arg3 = arg1;
                 arg3 = arg3->nextArgument();
@@ -853,17 +851,23 @@ void CheckClass::noMemset()
                     type = symbolDatabase->findVariableType(&(*scope), typeTok);
 
                 if (type)
-                checkMemsetType(&(*scope), tok, type);
+                    checkMemsetType(&(*scope), tok, type, false);
+            } else if (tok->variable() && tok->variable()->type() && Token::Match(tok, "%var% = calloc|malloc|realloc|g_malloc|g_try_malloc|g_realloc|g_try_realloc (")) {
+                checkMemsetType(&(*scope), tok->tokAt(2), tok->variable()->type(), true);
+
+                if (tok->variable()->type()->numConstructors > 0 && _settings->isEnabled("warning"))
+                    mallocOnClassWarning(tok, tok->strAt(2), tok->variable()->type()->classDef);
+            }
         }
     }
 }
 
-void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type)
+void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation)
 {
     // recursively check all parent classes
     for (std::size_t i = 0; i < type->derivedFrom.size(); i++) {
         if (type->derivedFrom[i].scope)
-            checkMemsetType(start, tok, type->derivedFrom[i].scope);
+            checkMemsetType(start, tok, type->derivedFrom[i].scope, allocation);
     }
 
     // Warn if type is a class that contains any virtual functions
@@ -871,6 +875,9 @@ void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Sco
 
     for (func = type->functionList.begin(); func != type->functionList.end(); ++func) {
         if (func->isVirtual)
+            if (allocation)
+                mallocOnClassError(tok, tok->str(), type->classDef, "virtual method");
+            else
                 memsetError(tok, tok->str(), "virtual method", type->classDef->str());
     }
 
@@ -884,15 +891,40 @@ void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Sco
 
             // check for std:: type
             if (Token::simpleMatch(tok1, "std ::"))
+                if (allocation)
+                    mallocOnClassError(tok, tok->str(), type->classDef, "'std::" + tok1->strAt(2) + "'");
+                else
                     memsetError(tok, tok->str(), "'std::" + tok1->strAt(2) + "'", type->classDef->str());
 
             // check for known type
             else if (var->type())
-                checkMemsetType(start, tok, var->type());
+                checkMemsetType(start, tok, var->type(), allocation);
         }
     }
 }
 
+void CheckClass::mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok)
+{
+    std::list<const Token *> toks;
+    toks.push_back(tok);
+    toks.push_back(classTok);
+    reportError(toks, Severity::warning, "mallocOnClassWarning",
+                "Memory for class instance allocated with " + memfunc + "(), but class provides constructors.\n"
+                "Memory for class instance allocated with " + memfunc + "(), but class provides constructors. This is unsafe, "
+                "since no constructor is called and class members remain uninitialized. Consider using 'new' instead.");
+}
+
+void CheckClass::mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname)
+{
+    std::list<const Token *> toks;
+    toks.push_back(tok);
+    toks.push_back(classTok);
+    reportError(toks, Severity::error, "mallocOnClassError",
+                "Memory for class instance allocated with " + memfunc + "(), but class contains a " + classname + ".\n"
+                "Memory for class instance allocated with " + memfunc + "(), but class a " + classname + ". This is unsafe, "
+                "since no constructor is called and class members remain uninitialized. Consider using 'new' instead.");
+}
+
 void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type)
 {
     reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname + ".");

lib/checkclass.h

@@ -91,7 +91,7 @@ public:
      * Important: The checking doesn't work on simplified tokens list.
      */
     void noMemset();
-    void checkMemsetType(const Scope *start, const Token *tok, const Scope *type);
+    void checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation);
 
     /** @brief 'operator=' should return something and it should not be const. */
     void operatorEq();
@@ -130,6 +130,8 @@ private:
     void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive);
     void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname);
     void memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type);
+    void mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname);
+    void mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok);
     void operatorEqReturnError(const Token *tok, const std::string &className);
     void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived);
     void thisSubtractionError(const Token *tok);
@@ -150,6 +152,8 @@ private:
         c.operatorEqVarError(0, "classname", "", false);
         c.unusedPrivateFunctionError(0, "classname", "funcname");
         c.memsetError(0, "memfunc", "classname", "class");
+        c.mallocOnClassWarning(0, "malloc", 0);
+        c.mallocOnClassError(0, "malloc", 0, "std::string");
         c.operatorEqReturnError(0, "class");
         c.virtualDestructorError(0, "Base", "Derived");
         c.thisSubtractionError(0);
@@ -172,6 +176,7 @@ private:
                "* Are all variables initialized by the constructors?\n"
                "* Are all variables assigned by 'operator='?\n"
                "* Warn if memset, memcpy etc are used on a class\n"
+               "* Warn if memory for classes is allocated with malloc()\n"
                "* If it's a base class, check that the destructor is virtual\n"
                "* Are there unused private functions?\n"
                "* 'operator=' should return reference to self\n"

test/testclass.cpp

@@ -76,9 +76,11 @@ private:
         TEST_CASE(operatorEqToSelf7);
         TEST_CASE(operatorEqToSelf8);   // ticket #2179
         TEST_CASE(operatorEqToSelf9);   // ticket #2592
+
         TEST_CASE(memsetOnStruct);
         TEST_CASE(memsetVector);
         TEST_CASE(memsetOnClass);
+        TEST_CASE(mallocOnClass);
 
         TEST_CASE(this_subtraction);    // warn about "this-x"
 
@@ -1948,6 +1950,7 @@ private:
         errout.str("");
 
         Settings settings;
+        settings.addEnabled("warning");
 
         // Tokenize..
         Tokenizer tokenizer(&settings, this);
@@ -2304,6 +2307,62 @@ private:
         ASSERT_EQUALS("", errout.str()); // #4460
     }
 
+    void mallocOnClass() {
+        checkNoMemset("class C { C() {} };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = malloc(sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with malloc(), but class provides constructors.\n", errout.str());
+
+        checkNoMemset("class C { C(int z, Foo bar) { bar(); } };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = malloc(sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with malloc(), but class provides constructors.\n", errout.str());
+
+        checkNoMemset("struct C { C() {} };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = realloc(p, sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with realloc(), but class provides constructors.\n", errout.str());
+
+        checkNoMemset("struct C { C() {} };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = realloc(p, sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with realloc(), but class provides constructors.\n", errout.str());
+
+        checkNoMemset("struct C { virtual void bar(); };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = malloc(sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (error) Memory for class instance allocated with malloc(), but class contains a virtual method.\n", errout.str());
+
+        checkNoMemset("struct C { std::string s; };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = malloc(sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (error) Memory for class instance allocated with malloc(), but class contains a 'std::string'.\n", errout.str());
+
+        checkNoMemset("class C { };\n" // C-Style class/struct
+                      "void foo(C*& p) {\n"
+                      "    p = malloc(sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("", errout.str());
+
+        checkNoMemset("struct C { C() {} };\n"
+                      "void foo(C*& p) {\n"
+                      "    p = new C();\n"
+                      "}");
+        ASSERT_EQUALS("", errout.str());
+
+        checkNoMemset("class C { C() {} };\n"
+                      "void foo(D*& p) {\n" // Unknown type
+                      "    p = malloc(sizeof(C));\n"
+                      "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
 
     void checkThisSubtraction(const char code[]) {
         // Clear the error log