From 70b407611124838b3c83e8e920100d4771c71f29 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Wed, 9 Mar 2011 21:29:30 +1300 Subject: [PATCH 1/3] refactor noMemset so it recursively checks parent classes for non-memset-compatible things --- lib/checkclass.cpp | 146 ++++++++++++++++++++++++++------------------- lib/checkclass.h | 1 + test/testclass.cpp | 12 ++++ 3 files changed, 98 insertions(+), 61 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 569f4ed9d..648c562d1 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -693,6 +693,90 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string // ClassCheck: Check that memset is not used on classes //--------------------------------------------------------------------------- +void CheckClass::checkMemsetType(const Token *tok, const std::string &type) +{ + // Warn if type is a class or struct that contains any std::* variables + const std::string pattern2(std::string("struct|class ") + type + " :|{"); + const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); + + if (!tstruct) + return; + + const std::string &typeName = tstruct->str(); + + if (tstruct->tokAt(2)->str() == ":") + { + tstruct = tstruct->tokAt(3); + for (; tstruct; tstruct = tstruct->next()) + { + while (Token::Match(tstruct, "public|private|protected|virtual")) + { + tstruct = tstruct->next(); + } + + // recursively check all parent classes + checkMemsetType(tok, tstruct->str()); + + tstruct = tstruct->next(); + if (tstruct->str() != ",") + break; + } + } + + for (; tstruct; tstruct = tstruct->next()) + { + if (tstruct->str() == "}") + break; + + // struct with function? skip function body.. + if (Token::simpleMatch(tstruct, ") {")) + { + tstruct = tstruct->next()->link(); + if (!tstruct) + break; + } + + // before a statement there must be either: + // * private:|protected:|public: + // * { } ; + if (Token::Match(tstruct, "[;{}]") || + tstruct->str().find(":") != std::string::npos) + { + if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) + memsetError(tok, tok->str(), tstruct->strAt(3), typeName); + + else if (Token::Match(tstruct->next(), "std :: %type% <")) + { + // backup the type + const std::string typestr(tstruct->strAt(3)); + + // check if it's a pointer variable.. + unsigned int level = 0; + while (0 != (tstruct = tstruct->next())) + { + if (tstruct->str() == "<") + ++level; + else if (tstruct->str() == ">") + { + if (level <= 1) + break; + --level; + } + else if (tstruct->str() == "(") + tstruct = tstruct->link(); + } + + if (!tstruct) + break; + + // found error => report + if (Token::Match(tstruct, "> %var% ;")) + memsetError(tok, tok->str(), typestr, typeName); + } + } + } +} + void CheckClass::noMemset() { createSymbolDatabase(); @@ -726,67 +810,7 @@ void CheckClass::noMemset() if (type.empty()) continue; - // Warn if type is a class or struct that contains any std::* variables - const std::string pattern2(std::string("struct|class ") + type + " {"); - const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); - - if (!tstruct) - continue; - - const std::string &typeName = tstruct->str(); - - for (; tstruct; tstruct = tstruct->next()) - { - if (tstruct->str() == "}") - break; - - // struct with function? skip function body.. - if (Token::simpleMatch(tstruct, ") {")) - { - tstruct = tstruct->next()->link(); - if (!tstruct) - break; - } - - // before a statement there must be either: - // * private:|protected:|public: - // * { } ; - if (Token::Match(tstruct, "[;{}]") || - tstruct->str().find(":") != std::string::npos) - { - if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) - memsetError(tok, tok->str(), tstruct->strAt(3), typeName); - - else if (Token::Match(tstruct->next(), "std :: %type% <")) - { - // backup the type - const std::string typestr(tstruct->strAt(3)); - - // check if it's a pointer variable.. - unsigned int level = 0; - while (0 != (tstruct = tstruct->next())) - { - if (tstruct->str() == "<") - ++level; - else if (tstruct->str() == ">") - { - if (level <= 1) - break; - --level; - } - else if (tstruct->str() == "(") - tstruct = tstruct->link(); - } - - if (!tstruct) - break; - - // found error => report - if (Token::Match(tstruct, "> %var% ;")) - memsetError(tok, tok->str(), typestr, typeName); - } - } - } + checkMemsetType(tok, type); } } diff --git a/lib/checkclass.h b/lib/checkclass.h index 2dd238f83..1b3b5fa43 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -84,6 +84,7 @@ public: * Important: The checking doesn't work on simplified tokens list. */ void noMemset(); + void checkMemsetType(const Token *tok, const std::string &type); /** @brief 'operator=' should return something and it should not be const. */ void operatorEq(); diff --git a/test/testclass.cpp b/test/testclass.cpp index d49d5abe3..a724285df 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2949,6 +2949,18 @@ private: " memset(&fred, 0, sizeof(fred));\n" "}\n"); ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + " std::string s;\n" + "};\n" + "class Pebbles: public Fred {};\n" + "void f()\n" + "{\n" + " Pebbles pebbles;\n" + " memset(&pebbles, 0, sizeof(pebbles));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); } void memsetOnStruct() From 3883afcbf48044aa3ef1c6ee4c6d3a7768bbeb9f Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Wed, 9 Mar 2011 22:10:39 +1300 Subject: [PATCH 2/3] Check for memset on objects with virtual functions (ticket #607) --- lib/checkclass.cpp | 8 +++++--- test/testclass.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 648c562d1..c01baed55 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -743,7 +743,7 @@ void CheckClass::checkMemsetType(const Token *tok, const std::string &type) tstruct->str().find(":") != std::string::npos) { if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) - memsetError(tok, tok->str(), tstruct->strAt(3), typeName); + memsetError(tok, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeName); else if (Token::Match(tstruct->next(), "std :: %type% <")) { @@ -771,8 +771,10 @@ void CheckClass::checkMemsetType(const Token *tok, const std::string &type) // found error => report if (Token::Match(tstruct, "> %var% ;")) - memsetError(tok, tok->str(), typestr, typeName); + memsetError(tok, tok->str(), "'std::" + typestr + "'", typeName); } + else if (Token::simpleMatch(tstruct->next(), "virtual")) + memsetError(tok, tok->str(), "virtual method", typeName); } } } @@ -816,7 +818,7 @@ void CheckClass::noMemset() 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 'std::" + classname + "'"); + reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname); } //--------------------------------------------------------------------------- diff --git a/test/testclass.cpp b/test/testclass.cpp index a724285df..ff0da1953 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2961,6 +2961,32 @@ private: " memset(&pebbles, 0, sizeof(pebbles));\n" "}\n"); ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + " virtual ~Fred();\n" + "};\n" + "void f()\n" + "{\n" + " Fred fred;\n" + " memset(&fred, 0, sizeof(fred));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a virtual method\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + "};\n" + "class Wilma\n" + "{\n" + " virtual ~Wilma();\n" + "};\n" + "class Pebbles: public Fred, Wilma {};\n" + "void f()\n" + "{\n" + " Pebbles pebbles;\n" + " memset(&pebbles, 0, sizeof(pebbles));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:12]: (error) Using 'memset' on class that contains a virtual method\n", errout.str()); } void memsetOnStruct() From a084697410bac0e08324236e78185a0c1a8a92ff Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Wed, 9 Mar 2011 22:49:13 +1300 Subject: [PATCH 3/3] Check for memset on nested structs (ticket #1288) --- lib/checkclass.cpp | 30 ++++++++++++++++++++++++------ lib/checkclass.h | 10 +++++++--- test/testclass.cpp | 12 ++++++++++++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index c01baed55..a43c39ea5 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -695,6 +695,14 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string void CheckClass::checkMemsetType(const Token *tok, const std::string &type) { + // check for cached message for this type + std::map::const_iterator msg = _memsetClassMessages.find(type); + if (msg != _memsetClassMessages.end()) + { + memsetError(tok, type, msg->second); + return; + } + // Warn if type is a class or struct that contains any std::* variables const std::string pattern2(std::string("struct|class ") + type + " :|{"); const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); @@ -702,7 +710,8 @@ void CheckClass::checkMemsetType(const Token *tok, const std::string &type) if (!tstruct) return; - const std::string &typeName = tstruct->str(); + // typeKind is either 'struct' or 'class' + const std::string &typeKind = tstruct->str(); if (tstruct->tokAt(2)->str() == ":") { @@ -743,7 +752,7 @@ void CheckClass::checkMemsetType(const Token *tok, const std::string &type) tstruct->str().find(":") != std::string::npos) { if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) - memsetError(tok, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeName); + memsetError(tok, type, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeKind); else if (Token::Match(tstruct->next(), "std :: %type% <")) { @@ -771,10 +780,12 @@ void CheckClass::checkMemsetType(const Token *tok, const std::string &type) // found error => report if (Token::Match(tstruct, "> %var% ;")) - memsetError(tok, tok->str(), "'std::" + typestr + "'", typeName); + memsetError(tok, type, tok->str(), "'std::" + typestr + "'", typeKind); } else if (Token::simpleMatch(tstruct->next(), "virtual")) - memsetError(tok, tok->str(), "virtual method", typeName); + memsetError(tok, type, tok->str(), "virtual method", typeKind); + else if (!Token::Match(tstruct->next(), "static|}")) + checkMemsetType(tok, tstruct->next()->str()); } } } @@ -816,9 +827,16 @@ void CheckClass::noMemset() } } -void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type) +void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &message) { - reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname); + reportError(tok, Severity::error, "memsetClass", message); + // cache the message for this type so we don't have to look it up again + _memsetClassMessages[type] = message; +} + +void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind) +{ + memsetError(tok, type, "Using '" + memfunc + "' on " + typekind + " that contains a " + classname); } //--------------------------------------------------------------------------- diff --git a/lib/checkclass.h b/lib/checkclass.h index 1b3b5fa43..9cd12f499 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -84,7 +84,6 @@ public: * Important: The checking doesn't work on simplified tokens list. */ void noMemset(); - void checkMemsetType(const Token *tok, const std::string &type); /** @brief 'operator=' should return something and it should not be const. */ void operatorEq(); @@ -118,7 +117,8 @@ private: void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname); void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname); 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 memsetError(const Token *tok, const std::string &type, const std::string &message); + void memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind); void operatorEqReturnError(const Token *tok); void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived); void thisSubtractionError(const Token *tok); @@ -134,7 +134,7 @@ private: c.uninitVarError(0, "classname", "varname"); c.operatorEqVarError(0, "classname", ""); c.unusedPrivateFunctionError(0, "classname", "funcname"); - c.memsetError(0, "memfunc", "classname", "class"); + c.memsetError(0, "type", "memfunc", "classname", "class"); c.operatorEqReturnError(0); //c.virtualDestructorError(0, "Base", "Derived"); c.thisSubtractionError(0); @@ -228,6 +228,10 @@ private: void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage); bool canNotCopy(const Scope *scope) const; + + // noMemset helpers + void checkMemsetType(const Token *tok, const std::string &type); + std::map _memsetClassMessages; }; /// @} //--------------------------------------------------------------------------- diff --git a/test/testclass.cpp b/test/testclass.cpp index ff0da1953..afa2748a2 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2962,6 +2962,18 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + checkNoMemset("struct Stringy {\n" + " std::string inner;\n" + "};\n" + "struct Foo {\n" + " Stringy s;\n" + "};\n" + "int main() {\n" + " Foo foo;\n" + " memset(&foo, 0, sizeof(Foo));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on struct that contains a 'std::string'\n", errout.str()); + checkNoMemset("class Fred\n" "{\n" " virtual ~Fred();\n"