Fix #5606 - Endless recursion in CheckClass::isMemberVar(). New function Type::hasCircularDependencies() is supposed to detect loops within the class hierarchy which was causing the problem

lib/checkclass.cpp

@@ -1139,8 +1139,8 @@ void CheckClass::operatorEq()
 void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className)
 {
     reportError(tok, Severity::style, "operatorEq", "'" + className + "::operator=' should return '" + className + " &'.\n"
-      "The "+className+"::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '"+className+" &"+className+"::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers."
+                "The "+className+"::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '"+className+" &"+className+"::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers."
-    );
+               );
 }
 
 //---------------------------------------------------------------------------
@@ -1624,7 +1624,7 @@ bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) const
     }
 
     // not found in this class
-    if (!scope->definedType->derivedFrom.empty()) {
+    if (!scope->definedType->derivedFrom.empty() && !scope->definedType->hasCircularDependencies()) {
         // check each base class
         for (unsigned int i = 0; i < scope->definedType->derivedFrom.size(); ++i) {
             // find the base class

lib/symboldatabase.cpp

@@ -1593,6 +1593,28 @@ const Function* Type::getFunction(const std::string& funcName) const
     return 0;
 }
 
+bool Type::hasCircularDependencies(std::set<BaseInfo>* anchestors) const
+{
+    std::set<BaseInfo> knownAnchestors;
+    if (!anchestors) {
+        anchestors=&knownAnchestors;
+    }
+    for (std::vector<BaseInfo>::const_iterator parent=derivedFrom.begin(); parent!=derivedFrom.end(); ++parent) {
+        if (!parent->type)
+            continue;
+        else if (this==parent->type)
+            return true;
+        else if (anchestors->find(*parent)!=anchestors->end())
+            return true;
+        else {
+            anchestors->insert(*parent);
+            if (parent->type->hasCircularDependencies(anchestors))
+                return true;
+        }
+    }
+    return false;
+}
+
 bool Variable::arrayDimensions(std::vector<Dimension> &dimensions, const Token *tok)
 {
     bool isArray = false;
@@ -2027,7 +2049,7 @@ bool Function::isImplicitlyVirtual(bool defaultVal) const
         return false;
 }
 
-bool Function::isImplicitlyVirtual_rec(const ::Type* baseType, bool& safe, std::deque< const ::Type* > *anchestors) const
+bool Function::isImplicitlyVirtual_rec(const ::Type* baseType, bool& safe) const
 {
     // check each base class
     for (std::size_t i = 0; i < baseType->derivedFrom.size(); ++i) {
@@ -2062,19 +2084,11 @@ bool Function::isImplicitlyVirtual_rec(const ::Type* baseType, bool& safe, std::
                 }
             }
 
-            if (!baseType->derivedFrom[i].type->derivedFrom.empty()) {
+            if (!baseType->derivedFrom[i].type->derivedFrom.empty() && !baseType->derivedFrom[i].type->hasCircularDependencies()) {
                 // 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.
+                if (isImplicitlyVirtual_rec(baseType->derivedFrom[i].type, safe))  {
-                std::deque< const ::Type* > local_anchestors;
+                    return true;
-                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 {

lib/symboldatabase.h

@@ -66,7 +66,8 @@ public:
         Unknown, True, False
     } needInitialization;
 
-    struct BaseInfo {
+    class BaseInfo {
+    public:
         BaseInfo() :
             type(NULL), nameTok(NULL), access(Public), isVirtual(false) {
         }
@@ -76,6 +77,10 @@ public:
         const Token* nameTok;
         AccessControl access;  // public/protected/private
         bool isVirtual;
+        // allow ordering within containers
+        bool operator<(const BaseInfo& rhs) const {
+            return this->type < rhs.type;
+        }
     };
 
     struct FriendInfo {
@@ -107,6 +112,13 @@ public:
     const Token *initBaseInfo(const Token *tok, const Token *tok1);
 
     const Function* getFunction(const std::string& funcName) const;
+
+    /**
+    * Check for circulare dependencies, i.e. loops within the class hierarchie
+    * @param anchestors list of anchestors. For internal usage only, clients should not supply this argument.
+    * @return true if there is a circular dependency
+    */
+    bool hasCircularDependencies(std::set<BaseInfo>* anchestors = 0) const;
 };
 
 /** @brief Information about a member variable. */
@@ -604,7 +616,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, std::deque<const ::Type* > *anchestors = nullptr) const;
+    bool isImplicitlyVirtual_rec(const ::Type* type, bool& safe) const;
 };
 
 class CPPCHECKLIB Scope {

test/testclass.cpp

@@ -146,6 +146,7 @@ private:
         TEST_CASE(const58); // ticket #2698
         TEST_CASE(const59); // ticket #4646
         TEST_CASE(const60); // ticket #3322
+        TEST_CASE(const61); // ticket #5606
         TEST_CASE(const_handleDefaultParameters);
         TEST_CASE(const_passThisToMemberOfOtherClass);
         TEST_CASE(assigningPointerToPointerIsNotAConstOperation);
@@ -4792,6 +4793,16 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void const61() { // ticket #5606 - don't crash
+        checkConst("class MixerParticipant : public MixerParticipant {\n"
+                   "    int GetAudioFrame();\n"
+                   "};\n"
+                   "int MixerParticipant::GetAudioFrame() {\n"
+                   "    return 0;\n"
+                   "}");
+        ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (performance, inconclusive) Technically the member function 'MixerParticipant::GetAudioFrame' can be static.\n", errout.str());
+    }
+
 
     void const_handleDefaultParameters() {
         checkConst("struct Foo {\n"