Fixed #9271 (Safe classes: Class that store references)

lib/checkclass.cpp

@@ -2663,3 +2663,35 @@ void CheckClass::overrideError(const Function *funcInBase, const Function *funcI
                 CWE(0U) /* Unknown CWE! */,
                 false);
 }
+
+void CheckClass::checkSafeClassRefMember()
+{
+    if (!mSettings->safeChecks.classes || !mSettings->isEnabled(Settings::WARNING))
+        return;
+    for (const Scope * classScope : mSymbolDatabase->classAndStructScopes) {
+        for (const Function &func : classScope->functionList) {
+            if (!func.hasBody() || !func.isConstructor())
+                continue;
+
+            const Token *initList = func.constructorMemberInitialization();
+            while (Token::Match(initList, "[:,] %name% (")) {
+                if (Token::Match(initList->tokAt(2), "( %var% )")) {
+                    const Variable * const memberVar = initList->next()->variable();
+                    const Variable * const argVar = initList->tokAt(3)->variable();
+                    if (memberVar && argVar && memberVar->isConst() && memberVar->isReference() && argVar->isArgument() && argVar->isConst() && argVar->isReference())
+                        safeClassRefMemberError(initList->next(), classScope->className + "::" + memberVar->name());
+                }
+                initList = initList->linkAt(2)->next();
+            }
+        }
+    }
+}
+
+void CheckClass::safeClassRefMemberError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::warning, "safeClassRefMember",
+                "$symbol:" + varname + "\n"
+                "Unsafe class: The const reference member '$symbol' is initialized by a const reference constructor argument. You need to be careful about lifetime issues.\n"
+                "Safe class checking: The const reference member '$symbol' is initialized by a const reference constructor argument. You need to be careful about lifetime issues. If you pass a local variable or temporary value in this constructor argument, be extra careful. If the argument is always some global object that is never destroyed then this is safe usage. However it would be defensive to make the member '$symbol' a non-reference variable.",
+                CWE(0), false);
+}

lib/checkclass.h

@@ -79,6 +79,7 @@ public:
         checkClass.checkExplicitConstructors();
         checkClass.checkCopyCtorAndEqOperator();
         checkClass.checkOverride();
+        checkClass.checkSafeClassRefMember();
     }
 
     /** @brief %Check that all class constructors are ok */
@@ -146,6 +147,9 @@ public:
     /** @brief Check that the override keyword is used when overriding virtual functions */
     void checkOverride();
 
+    /** @brief Safe class check - const reference member */
+    void checkSafeClassRefMember();
+
 private:
     const SymbolDatabase *mSymbolDatabase;
 
@@ -183,6 +187,7 @@ private:
     void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
     void unsafeClassDivZeroError(const Token *tok, const std::string &className, const std::string &methodName, const std::string &varName);
     void overrideError(const Function *funcInBase, const Function *funcInDerived);
+    void safeClassRefMemberError(const Token *tok, const std::string &varname);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
         CheckClass c(nullptr, settings, errorLogger);
@@ -220,6 +225,7 @@ private:
         c.pureVirtualFunctionCallInConstructorError(nullptr, std::list<const Token *>(), "f");
         c.virtualFunctionCallInConstructorError(nullptr, std::list<const Token *>(), "f");
         c.overrideError(nullptr, nullptr);
+        c.safeClassRefMemberError(nullptr, "UnsafeClass::var");
     }
 
     static std::string myName() {

test/testclass.cpp

@@ -221,6 +221,8 @@ private:
 
         TEST_CASE(override1);
         TEST_CASE(overrideCVRefQualifiers);
+
+        TEST_CASE(safeClassRefMember);
     }
 
     void checkCopyCtorAndEqOperator(const char code[]) {
@@ -7160,6 +7162,28 @@ private:
                       "class Derived : Base { void f() &&; }");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void checkSafeClassRefMember(const char code[]) {
+        // Clear the error log
+        errout.str("");
+        Settings settings;
+        settings.safeChecks.classes = true;
+        settings.addEnabled("warning");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        // Check..
+        CheckClass checkClass(&tokenizer, &settings, this);
+        checkClass.checkSafeClassRefMember();
+    }
+
+    void safeClassRefMember() {
+        checkSafeClassRefMember("class C { C(const std::string &s) : s(s) {} const std::string &s; };");
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Unsafe class: The const reference member 'C::s' is initialized by a const reference constructor argument. You need to be careful about lifetime issues.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestClass)