From 37befc75efeb78d84737759533b64f6592dcb1f9 Mon Sep 17 00:00:00 2001 From: Alexander Mai Date: Sat, 22 Mar 2014 08:49:28 +0100 Subject: [PATCH 1/2] In case of a loop within the class hierarchie Function::isImplicitlyVirtual_rec() was entering an endless loop. Tracking the previously analyzed types shall prevent this. --- lib/symboldatabase.cpp | 18 ++++++++++++++---- lib/symboldatabase.h | 3 ++- test/testsymboldatabase.cpp | 21 +++++++++++++++++++++ test/testunusedprivfunc.cpp | 20 ++++++++++++++++++++ 4 files changed, 57 insertions(+), 5 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 3c654f757..4a5b9256a 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2029,7 +2029,7 @@ bool Function::isImplicitlyVirtual(bool defaultVal) const return false; } -bool Function::isImplicitlyVirtual_rec(const ::Type* baseType, bool& safe) const +bool Function::isImplicitlyVirtual_rec(const ::Type* baseType, bool& safe, std::deque< const ::Type* > *anchestors) const { // check each base class for (std::size_t i = 0; i < baseType->derivedFrom.size(); ++i) { @@ -2065,9 +2065,19 @@ bool Function::isImplicitlyVirtual_rec(const ::Type* baseType, bool& safe) const } if (!baseType->derivedFrom[i].type->derivedFrom.empty()) { - // avoid endless recursion, see #5289 Crash: Stack overflow in isImplicitlyVirtual_rec when checking SVN - if ((baseType != baseType->derivedFrom[i].type) && isImplicitlyVirtual_rec(baseType->derivedFrom[i].type, safe)) - return true; + // avoid endless recursion, see #5289 Crash: Stack overflow in isImplicitlyVirtual_rec when checking SVN and + // #5590 with a loop within the class hierarchie. + // We do so by tracking all previously checked types in a deque. + std::deque< const ::Type* > local_anchestors; + if (!anchestors) { + anchestors=&local_anchestors; + } + anchestors->push_back(baseType); + if (std::find(anchestors->begin(), anchestors->end(), baseType->derivedFrom[i].type)==anchestors->end()) { + if (isImplicitlyVirtual_rec(baseType->derivedFrom[i].type, safe, anchestors)) { + return true; + } + } } } else { // unable to find base class so assume it has no virtual function diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 5056ed394..82d2a2b04 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -603,7 +604,7 @@ public: static bool argsMatch(const Scope *info, const Token *first, const Token *second, const std::string &path, unsigned int depth); private: - bool isImplicitlyVirtual_rec(const ::Type* type, bool& safe) const; + bool isImplicitlyVirtual_rec(const ::Type* type, bool& safe, std::deque *anchestors = nullptr) const; }; class CPPCHECKLIB Scope { diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index 37420412a..14f9f7502 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -1818,6 +1818,27 @@ private: " }\n" "};"); ASSERT(db && db->findScopeByName("Bar") && !db->findScopeByName("Bar")->functionList.front().isImplicitlyVirtual(false)); + ASSERT_EQUALS(1, db->findScopeByName("Bar")->functionList.size()); + } + + // #5590 + { + GET_SYMBOL_DB("class InfiniteB : InfiniteA {\n" + " class D {\n" + " };\n" + "};\n" + "namespace N {\n" + " class InfiniteA : InfiniteB {\n" + " };\n" + "}\n" + "class InfiniteA : InfiniteB {\n" + " void foo();\n" + "};\n" + "void InfiniteA::foo() {\n" + " C a;\n" + "}"); + //ASSERT(db && db->findScopeByName("InfiniteA") && !db->findScopeByName("InfiniteA")->functionList.front().isImplicitlyVirtual()); + TODO_ASSERT_EQUALS(1, 0, db->findScopeByName("InfiniteA")->functionList.size()); } } diff --git a/test/testunusedprivfunc.cpp b/test/testunusedprivfunc.cpp index e313828e6..29f335913 100644 --- a/test/testunusedprivfunc.cpp +++ b/test/testunusedprivfunc.cpp @@ -72,6 +72,7 @@ private: TEST_CASE(multiFile); TEST_CASE(unknownBaseTemplate); // ticket #2580 + TEST_CASE(hierarchie_loop); // ticket 5590 } @@ -692,6 +693,25 @@ private: ASSERT_EQUALS("", errout.str()); } + void hierarchie_loop() { + check("class InfiniteB : InfiniteA {\n" + " class D {\n" + " };\n" + "};\n" + "namespace N {\n" + " class InfiniteA : InfiniteB {\n" + " };\n" + "}\n" + "class InfiniteA : InfiniteB {\n" + " void foo();\n" + "};\n" + "void InfiniteA::foo() {\n" + " C a;\n" + "}"); + + ASSERT_EQUALS("[test.cpp:10]: (style) Unused private function: 'InfiniteA::foo'\n", errout.str()); + } + }; REGISTER_TEST(TestUnusedPrivateFunction) From cdd6d4df277ec8530905b26d9eaeeaef536495b5 Mon Sep 17 00:00:00 2001 From: Alexander Mai Date: Fri, 21 Mar 2014 22:11:10 +0100 Subject: [PATCH 2/2] Removing a useless variable to fix cppcheck warning in its own code --- lib/valueflow.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index e7e72ec7b..234b9f6b6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -788,7 +788,6 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int } else if (Token::Match(tok2, ") {") && Token::findmatch(tok2->link(), "%varid%", tok2, varid)) { - const Token *start = tok2->link(); if (settings->debugwarnings) bailout(tokenlist, errorLogger, tok2, "For loop variable stopping on {"); break;