Skip to content

Commit 096aae4

Browse files
PKEuSPKEuS
committed
New check: Warn about using malloc() for classes containing virtual methods, std::-objects or constructors
1 parent 7579944 commit 096aae4

3 files changed

Lines changed: 133 additions & 37 deletions

File tree

‎lib/checkclass.cpp‎

Lines changed: 68 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -817,61 +817,68 @@ void CheckClass::noMemset()
817817
for (std::size_t i = 0; i < functions; ++i) {
818818
const Scope * scope = symbolDatabase->functionScopes[i];
819819
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
820-
if (!Token::Match(tok, "memset|memcpy|memmove ( %any%"))
821-
continue;
822-
823-
const Token* arg1 = tok->tokAt(2);
824-
const Token* arg3 = arg1;
825-
arg3 = arg3->nextArgument();
826-
if (arg3)
820+
if (Token::Match(tok, "memset|memcpy|memmove ( %any%")) {
821+
const Token* arg1 = tok->tokAt(2);
822+
const Token* arg3 = arg1;
827823
arg3 = arg3->nextArgument();
824+
if (arg3)
825+
arg3 = arg3->nextArgument();
826+
827+
const Token *typeTok = 0;
828+
const Scope *type = 0;
829+
if (Token::Match(arg3, "sizeof ( %type% ) )"))
830+
typeTok = arg3->tokAt(2);
831+
else if (Token::Match(arg3, "sizeof ( %type% :: %type% ) )"))
832+
typeTok = arg3->tokAt(4);
833+
else if (Token::Match(arg3, "sizeof ( struct %type% ) )"))
834+
typeTok = arg3->tokAt(3);
835+
else if (Token::simpleMatch(arg3, "sizeof ( * this ) )") || Token::simpleMatch(arg1, "this ,")) {
836+
type = findFunctionOf(arg3->scope());
837+
} else if (Token::Match(arg1, "&| %var% ,")) {
838+
const Variable *var = arg1->str() == "&" ? arg1->next()->variable() : arg1->variable();
839+
if (var && (arg1->str() == "&" || var->isPointer() || var->isArray()))
840+
type = var->type();
841+
}
828842

829-
const Token *typeTok = 0;
830-
const Scope *type = 0;
831-
if (Token::Match(arg3, "sizeof ( %type% ) )"))
832-
typeTok = arg3->tokAt(2);
833-
else if (Token::Match(arg3, "sizeof ( %type% :: %type% ) )"))
834-
typeTok = arg3->tokAt(4);
835-
else if (Token::Match(arg3, "sizeof ( struct %type% ) )"))
836-
typeTok = arg3->tokAt(3);
837-
else if (Token::simpleMatch(arg3, "sizeof ( * this ) )") || Token::simpleMatch(arg1, "this ,")) {
838-
type = findFunctionOf(arg3->scope());
839-
} else if (Token::Match(arg1, "&| %var% ,")) {
840-
const Variable *var = arg1->str() == "&" ? arg1->next()->variable() : arg1->variable();
841-
if (var && (arg1->str() == "&" || var->isPointer() || var->isArray()))
842-
type = var->type();
843-
}
843+
// No type defined => The tokens didn't match
844+
if (!typeTok && !type)
845+
continue;
844846

845-
// No type defined => The tokens didn't match
846-
if (!typeTok && !type)
847-
continue;
847+
if (typeTok && typeTok->str() == "(")
848+
typeTok = typeTok->next();
848849

849-
if (typeTok && typeTok->str() == "(")
850-
typeTok = typeTok->next();
850+
if (!type)
851+
type = symbolDatabase->findVariableType(&(*scope), typeTok);
851852

852-
if (!type)
853-
type = symbolDatabase->findVariableType(&(*scope), typeTok);
853+
if (type)
854+
checkMemsetType(&(*scope), tok, type, false);
855+
} else if (tok->variable() && tok->variable()->type() && Token::Match(tok, "%var% = calloc|malloc|realloc|g_malloc|g_try_malloc|g_realloc|g_try_realloc (")) {
856+
checkMemsetType(&(*scope), tok->tokAt(2), tok->variable()->type(), true);
854857

855-
if (type)
856-
checkMemsetType(&(*scope), tok, type);
858+
if (tok->variable()->type()->numConstructors > 0 && _settings->isEnabled("warning"))
859+
mallocOnClassWarning(tok, tok->strAt(2), tok->variable()->type()->classDef);
860+
}
857861
}
858862
}
859863
}
860864

861-
void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type)
865+
void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation)
862866
{
863867
// recursively check all parent classes
864868
for (std::size_t i = 0; i < type->derivedFrom.size(); i++) {
865869
if (type->derivedFrom[i].scope)
866-
checkMemsetType(start, tok, type->derivedFrom[i].scope);
870+
checkMemsetType(start, tok, type->derivedFrom[i].scope, allocation);
867871
}
868872

869873
// Warn if type is a class that contains any virtual functions
870874
std::list<Function>::const_iterator func;
871875

872876
for (func = type->functionList.begin(); func != type->functionList.end(); ++func) {
873877
if (func->isVirtual)
874-
memsetError(tok, tok->str(), "virtual method", type->classDef->str());
878+
if (allocation)
879+
mallocOnClassError(tok, tok->str(), type->classDef, "virtual method");
880+
else
881+
memsetError(tok, tok->str(), "virtual method", type->classDef->str());
875882
}
876883

877884
// Warn if type is a class or struct that contains any std::* variables
@@ -884,15 +891,40 @@ void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Sco
884891

885892
// check for std:: type
886893
if (Token::simpleMatch(tok1, "std ::"))
887-
memsetError(tok, tok->str(), "'std::" + tok1->strAt(2) + "'", type->classDef->str());
894+
if (allocation)
895+
mallocOnClassError(tok, tok->str(), type->classDef, "'std::" + tok1->strAt(2) + "'");
896+
else
897+
memsetError(tok, tok->str(), "'std::" + tok1->strAt(2) + "'", type->classDef->str());
888898

889899
// check for known type
890900
else if (var->type())
891-
checkMemsetType(start, tok, var->type());
901+
checkMemsetType(start, tok, var->type(), allocation);
892902
}
893903
}
894904
}
895905

906+
void CheckClass::mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok)
907+
{
908+
std::list<const Token *> toks;
909+
toks.push_back(tok);
910+
toks.push_back(classTok);
911+
reportError(toks, Severity::warning, "mallocOnClassWarning",
912+
"Memory for class instance allocated with " + memfunc + "(), but class provides constructors.\n"
913+
"Memory for class instance allocated with " + memfunc + "(), but class provides constructors. This is unsafe, "
914+
"since no constructor is called and class members remain uninitialized. Consider using 'new' instead.");
915+
}
916+
917+
void CheckClass::mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname)
918+
{
919+
std::list<const Token *> toks;
920+
toks.push_back(tok);
921+
toks.push_back(classTok);
922+
reportError(toks, Severity::error, "mallocOnClassError",
923+
"Memory for class instance allocated with " + memfunc + "(), but class contains a " + classname + ".\n"
924+
"Memory for class instance allocated with " + memfunc + "(), but class a " + classname + ". This is unsafe, "
925+
"since no constructor is called and class members remain uninitialized. Consider using 'new' instead.");
926+
}
927+
896928
void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type)
897929
{
898930
reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname + ".");

‎lib/checkclass.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class CPPCHECKLIB CheckClass : public Check {
9191
* Important: The checking doesn't work on simplified tokens list.
9292
*/
9393
void noMemset();
94-
void checkMemsetType(const Scope *start, const Token *tok, const Scope *type);
94+
void checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation);
9595

9696
/** @brief 'operator=' should return something and it should not be const. */
9797
void operatorEq();
@@ -130,6 +130,8 @@ class CPPCHECKLIB CheckClass : public Check {
130130
void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive);
131131
void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname);
132132
void memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type);
133+
void mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname);
134+
void mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok);
133135
void operatorEqReturnError(const Token *tok, const std::string &className);
134136
void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived);
135137
void thisSubtractionError(const Token *tok);
@@ -150,6 +152,8 @@ class CPPCHECKLIB CheckClass : public Check {
150152
c.operatorEqVarError(0, "classname", "", false);
151153
c.unusedPrivateFunctionError(0, "classname", "funcname");
152154
c.memsetError(0, "memfunc", "classname", "class");
155+
c.mallocOnClassWarning(0, "malloc", 0);
156+
c.mallocOnClassError(0, "malloc", 0, "std::string");
153157
c.operatorEqReturnError(0, "class");
154158
c.virtualDestructorError(0, "Base", "Derived");
155159
c.thisSubtractionError(0);
@@ -172,6 +176,7 @@ class CPPCHECKLIB CheckClass : public Check {
172176
"* Are all variables initialized by the constructors?\n"
173177
"* Are all variables assigned by 'operator='?\n"
174178
"* Warn if memset, memcpy etc are used on a class\n"
179+
"* Warn if memory for classes is allocated with malloc()\n"
175180
"* If it's a base class, check that the destructor is virtual\n"
176181
"* Are there unused private functions?\n"
177182
"* 'operator=' should return reference to self\n"

‎test/testclass.cpp‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,11 @@ class TestClass : public TestFixture {
7676
TEST_CASE(operatorEqToSelf7);
7777
TEST_CASE(operatorEqToSelf8); // ticket #2179
7878
TEST_CASE(operatorEqToSelf9); // ticket #2592
79+
7980
TEST_CASE(memsetOnStruct);
8081
TEST_CASE(memsetVector);
8182
TEST_CASE(memsetOnClass);
83+
TEST_CASE(mallocOnClass);
8284

8385
TEST_CASE(this_subtraction); // warn about "this-x"
8486

@@ -1948,6 +1950,7 @@ class TestClass : public TestFixture {
19481950
errout.str("");
19491951

19501952
Settings settings;
1953+
settings.addEnabled("warning");
19511954

19521955
// Tokenize..
19531956
Tokenizer tokenizer(&settings, this);
@@ -2304,6 +2307,62 @@ class TestClass : public TestFixture {
23042307
ASSERT_EQUALS("", errout.str()); // #4460
23052308
}
23062309

2310+
void mallocOnClass() {
2311+
checkNoMemset("class C { C() {} };\n"
2312+
"void foo(C*& p) {\n"
2313+
" p = malloc(sizeof(C));\n"
2314+
"}");
2315+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with malloc(), but class provides constructors.\n", errout.str());
2316+
2317+
checkNoMemset("class C { C(int z, Foo bar) { bar(); } };\n"
2318+
"void foo(C*& p) {\n"
2319+
" p = malloc(sizeof(C));\n"
2320+
"}");
2321+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with malloc(), but class provides constructors.\n", errout.str());
2322+
2323+
checkNoMemset("struct C { C() {} };\n"
2324+
"void foo(C*& p) {\n"
2325+
" p = realloc(p, sizeof(C));\n"
2326+
"}");
2327+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with realloc(), but class provides constructors.\n", errout.str());
2328+
2329+
checkNoMemset("struct C { C() {} };\n"
2330+
"void foo(C*& p) {\n"
2331+
" p = realloc(p, sizeof(C));\n"
2332+
"}");
2333+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (warning) Memory for class instance allocated with realloc(), but class provides constructors.\n", errout.str());
2334+
2335+
checkNoMemset("struct C { virtual void bar(); };\n"
2336+
"void foo(C*& p) {\n"
2337+
" p = malloc(sizeof(C));\n"
2338+
"}");
2339+
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());
2340+
2341+
checkNoMemset("struct C { std::string s; };\n"
2342+
"void foo(C*& p) {\n"
2343+
" p = malloc(sizeof(C));\n"
2344+
"}");
2345+
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());
2346+
2347+
checkNoMemset("class C { };\n" // C-Style class/struct
2348+
"void foo(C*& p) {\n"
2349+
" p = malloc(sizeof(C));\n"
2350+
"}");
2351+
ASSERT_EQUALS("", errout.str());
2352+
2353+
checkNoMemset("struct C { C() {} };\n"
2354+
"void foo(C*& p) {\n"
2355+
" p = new C();\n"
2356+
"}");
2357+
ASSERT_EQUALS("", errout.str());
2358+
2359+
checkNoMemset("class C { C() {} };\n"
2360+
"void foo(D*& p) {\n" // Unknown type
2361+
" p = malloc(sizeof(C));\n"
2362+
"}");
2363+
ASSERT_EQUALS("", errout.str());
2364+
}
2365+
23072366

23082367
void checkThisSubtraction(const char code[]) {
23092368
// Clear the error log

0 commit comments

Comments
 (0)