From c24527dbdeefee58ed0b68bd7fe669b339b3b02a Mon Sep 17 00:00:00 2001 From: PKEuS Date: Mon, 4 Mar 2013 02:47:29 -0800 Subject: [PATCH 1/2] Improved handling of dereferences in CheckClass::noMemset(), fixing false negatives and false positives related to multidimensional arrays and arrays of pointers. --- lib/checkclass.cpp | 26 ++++++++++++++++++++++---- test/testclass.cpp | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index b695bead8..24474eb1f 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -838,10 +838,28 @@ void CheckClass::noMemset() typeTok = arg3->tokAt(3); else if (Token::simpleMatch(arg3, "sizeof ( * this ) )") || Token::simpleMatch(arg1, "this ,")) { type = findFunctionOf(arg3->scope()); - } else if (Token::Match(arg1, "&| %var% ,")) { - const Variable *var = arg1->str() == "&" ? arg1->next()->variable() : arg1->variable(); - if (var && (arg1->str() == "&" || var->isPointer() || var->isArray())) - type = var->type(); + } else if (Token::Match(arg1, "&|*|%var%")) { + int derefs = 1; + for (;; arg1 = arg1->next()) { + if (arg1->str() == "&") + derefs--; + else if (arg1->str() == "*") + derefs++; + else + break; + } + + const Variable *var = arg1->variable(); + if (var && arg1->strAt(1) == ",") { + if (var->isPointer()) + derefs--; + if (var->isArray()) + derefs -= var->dimensions().size(); + + if (derefs == 0) + type = var->type(); + } + } // No type defined => The tokens didn't match diff --git a/test/testclass.cpp b/test/testclass.cpp index c6635972f..583b0674e 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2305,6 +2305,24 @@ private: " memset(a, 0, 100);\n" "}"); ASSERT_EQUALS("", errout.str()); // #4460 + + checkNoMemset("struct C {\n" + " std::string s;\n" + "};\n" + "int foo() {\n" + " C* c1[10][10];\n" + " C* c2[10];\n" + " C c3[10][10];\n" + " memset(**c1, 0, 10);\n" + " memset(*c1, 0, 10);\n" + " memset(*c2, 0, 10);\n" + " memset(*c3, 0, 10);\n" + " memset(c2, 0, 10);\n" + " memset(c3, 0, 10);\n" + "}"); + ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on struct that contains a 'std::string'.\n" + "[test.cpp:10]: (error) Using 'memset' on struct that contains a 'std::string'.\n" + "[test.cpp:11]: (error) Using 'memset' on struct that contains a 'std::string'.\n", errout.str()); } void mallocOnClass() { From 34b1d75a10cd49cd09909f372617df3427c241f8 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Mon, 4 Mar 2013 02:57:09 -0800 Subject: [PATCH 2/2] Fixed handling of std::vector::insert() in CheckStl::pushback() --- lib/checkstl.cpp | 10 +++++++--- test/teststl.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index c07a7c0bc..ab2834471 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -638,7 +638,7 @@ void CheckStl::pushback() if (tok3->str() == "break" || tok3->str() == "return") { pushbackTok = 0; break; - } else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize|clear (", varId)) { + } else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize|clear (", varId) && !tok3->previous()->isAssignmentOp()) { pushbackTok = tok3->tokAt(2); } } @@ -649,9 +649,13 @@ void CheckStl::pushback() // Assigning iterator.. if (Token::Match(tok2, "%varid% =", iteratorid)) { - if (Token::Match(tok2->tokAt(2), "%var% . begin|end|rbegin|rend|cbegin|cend|crbegin|crend ( )")) { + if (Token::Match(tok2->tokAt(2), "%var% . begin|end|rbegin|rend|cbegin|cend|crbegin|crend|insert (")) { + if (!invalidIterator.empty() && Token::Match(tok2->tokAt(4), "insert ( %varid% ,", iteratorid)) { + invalidIteratorError(tok2, invalidIterator, tok2->strAt(6)); + break; + } vectorid = tok2->tokAt(2)->varId(); - tok2 = tok2->tokAt(6); + tok2 = tok2->linkAt(5); } else { vectorid = 0; } diff --git a/test/teststl.cpp b/test/teststl.cpp index bf9a908aa..712c6f810 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1161,6 +1161,31 @@ private: " return i->foo;\n" "}"); ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str()); + + check("void f(const std::vector& bars) {\n" + " for(std::vector::iterator i = bars.begin(); i != bars.end(); ++i) {\n" + " i = bars.insert(i, bar);\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f(const std::vector& bars) {\n" + " for(std::vector::iterator i = bars.begin(); i != bars.end(); ++i) {\n" + " bars.insert(i, bar);\n" + " i = bars.insert(i, bar);\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) After insert(), the iterator 'i' may be invalid.\n" + "[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str()); + + check("void* f(const std::vector& bars) {\n" + " std::vector::iterator i = bars.begin();\n" + " bars.insert(i, Bar());\n" + " i = bars.insert(i, Bar());\n" + " void* v = &i->foo;\n" + " return v;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str()); } void insert2() {