Fixed #831 (Exception safety: multiple new in a simple execution path)

lib/checkexceptionsafety.cpp

@@ -74,3 +74,96 @@ void CheckExceptionSafety::destructors()
     }
 }
 
+
+static bool autodealloc(const Token * const C, const Token * const tokens)
+{
+    if (C->isStandardType())
+        return false;
+    return !Token::findmatch(tokens, ("class " + C->str() + " {").c_str());
+}
+
+void CheckExceptionSafety::unsafeNew()
+{
+    // Check that "--exception-safety" was given
+    if (!_settings->_exceptionSafety)
+        return;
+
+    // Inspect initializer lists..
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        if (tok->str() != ")")
+            continue;
+        tok = tok->next();
+        if (tok->str() != ":")
+            continue;
+
+        // count "new" and check that it's an initializer list..
+        unsigned int countNew = 0;
+        for (tok = tok->next(); tok; tok = tok->next())
+        {
+            if (!Token::Match(tok, "%var% ("))
+                break;
+            tok = tok->next();
+            if (Token::Match(tok->next(), "new %type%"))
+            {
+                if (countNew > 0 || !autodealloc(tok->tokAt(2), _tokenizer->tokens()))
+                {
+                    ++countNew;
+                }
+            }
+            tok = tok->link();
+            tok = tok ? tok->next() : 0;
+            if (!tok)
+                break;
+            if (tok->str() == "{")
+            {
+                if (countNew > 1)
+                    unsafeNewError(tok);
+                break;
+            }
+            else if (tok->str() != ",")
+                break;
+        }
+        if (!tok)
+            break;
+    }
+
+
+    // Inspect constructors..
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        // match this pattern.. "C :: C ( .. ) {"
+        if (!tok->next() || tok->next()->str() != "::")
+            continue;
+        if (!Token::Match(tok, "%var% :: %var% ("))
+            continue;
+        if (tok->str() != tok->strAt(2))
+            continue;
+        if (!Token::simpleMatch(tok->tokAt(3)->link(), ") {"))
+            continue;
+
+        // inspect the constructor..
+        unsigned int countNew = 0;
+        for (tok = tok->tokAt(3)->link()->tokAt(2); tok; tok = tok->next())
+        {
+            if (tok->str() == "{" || tok->str() == "}")
+                break;
+            // some variable declaration..
+            if (Token::Match(tok->previous(), "[{;] %type% * %var% ;"))
+                break;
+            // allocating with new..
+            if (Token::Match(tok, "%var% = new %type%"))
+            {
+                if (countNew > 0 || !autodealloc(tok->tokAt(3), _tokenizer->tokens()))
+                {
+                    ++countNew;
+                    if (countNew > 1)
+                    {
+                        unsafeNewError(tok);
+                        break;
+                    }
+                }
+            }
+        }
+    }
+}

lib/checkexceptionsafety.h

@@ -52,6 +52,9 @@ public:
     /** Don't throw exceptions in destructors */
     void destructors();
 
+    /** unsafe use of "new" */
+    void unsafeNew();
+
 private:
     /** Don't throw exceptions in destructors */
     void destructorsError(const Token * const tok)
@@ -59,9 +62,16 @@ private:
         reportError(tok, Severity::style, "throwInDestructor", "Throwing exception in destructor is not recommended");
     }
 
+    /** Unsafe use of new */
+    void unsafeNewError(const Token * const tok)
+    {
+        reportError(tok, Severity::style, "unsafeNew", "Exception safety: unsafe use of 'new'");
+    }
+
     void getErrorMessages()
     {
         destructorsError(0);
+        unsafeNewError(0);
     }
 
     std::string name() const
@@ -72,7 +82,8 @@ private:
     std::string classInfo() const
     {
         return "Checking exception safety\n"
-               " * Don't throw exceptions in destructors";
+               " * Don't throw exceptions in destructors"
+               " * Unsafe use of 'new'";
     }
 };
 /// @}

lib/cppcheck.cpp

@@ -109,6 +109,10 @@ void CppCheck::parseFromArgs(int argc, const char* const argv[])
         else if (strcmp(argv[i], "-s") == 0 || strcmp(argv[i], "--style") == 0)
             _settings._checkCodingStyle = true;
 
+        // Checking exception safety
+        else if (strcmp(argv[i], "--exception-safety") == 0)
+            _settings._exceptionSafety = true;
+
         // Filter errors
         else if (strcmp(argv[i], "--suppressions") == 0)
         {
@@ -348,6 +352,7 @@ void CppCheck::parseFromArgs(int argc, const char* const argv[])
         "                         if arguments are not valid or if no input files are\n"
         "                         provided. Note that your operating system can\n"
         "                         modify this value, e.g. 256 can become 0.\n"
+        "    --exception-safety   Extended checking for exception safety\n"
         "    -f, --force          Force checking on files that have \"too many\"\n"
         "                         configurations\n"
         "    -h, --help           Print this help\n"

lib/settings.cpp

@@ -36,6 +36,7 @@ Settings::Settings()
     _exitCode = 0;
     _showtime = false;
     _append = "";
+    _exceptionSafety = false;
 }
 
 Settings::~Settings()

lib/settings.h

@@ -56,6 +56,9 @@ public:
     bool _errorsOnly;
     bool _verbose;
 
+    /** extra checks for exception safety */
+    bool _exceptionSafety;
+
     /** Force checking t he files with "too many" configurations. */
     bool _force;
 

test/testexceptionsafety.cpp

@@ -35,13 +35,14 @@ private:
     void run()
     {
         TEST_CASE(destructors);
+        TEST_CASE(newnew);
     }
 
-    void check(const char code[])
+    void check(const std::string &code)
     {
         // Tokenize..
         Tokenizer tokenizer;
-        std::istringstream istr(code);
+        std::istringstream istr(code.c_str());
         tokenizer.tokenize(istr, "test.cpp");
         tokenizer.simplifyTokenList();
 
@@ -51,8 +52,10 @@ private:
         // Check char variable usage..
         Settings settings;
         settings._checkCodingStyle = true;
+        settings._exceptionSafety = true;
         CheckExceptionSafety checkExceptionSafety(&tokenizer, &settings, this);
         checkExceptionSafety.destructors();
+        checkExceptionSafety.unsafeNew();
     }
 
     void destructors()
@@ -64,6 +67,23 @@ private:
         ASSERT_EQUALS("[test.cpp:3]: (style) Throwing exception in destructor is not recommended\n", errout.str());
     }
 
+    void newnew()
+    {
+        const std::string AB("class A { }; class B { }; ");
+
+        check(AB + "C::C() : a(new A), b(new B) { }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Exception safety: unsafe use of 'new'\n", errout.str());
+
+        check("C::C() : a(new A), b(new B) { }");
+        ASSERT_EQUALS("", errout.str());
+
+        check(AB + "C::C()\n"
+              "{\n"
+              "    a = new A;\n"
+              "    b = new B;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (style) Exception safety: unsafe use of 'new'\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestExceptionSafety)