Exception safety: added check for unsafe reallocation of member pointer

lib/checkexceptionsafety.cpp

@@ -194,3 +194,68 @@ void CheckExceptionSafety::unsafeNew()
         }
     }
 }
+
+
+void CheckExceptionSafety::realloc()
+{
+    // Check that "--exception-safety" was given
+    if (!_settings->_exceptionSafety)
+        return;
+
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        if (Token::simpleMatch(tok, "try {"))
+        {
+            tok = tok->next()->link();
+            if (!tok)
+                break;
+        }
+
+        if (!Token::Match(tok, "[{};] delete"))
+            continue;
+
+        tok = tok->tokAt(2);
+        if (Token::simpleMatch(tok, "[ ]"))
+            tok = tok->tokAt(2);
+        if (!tok)
+            break;
+
+        // reallocating..
+        if (!Token::Match(tok, "%var% ; %var% = new"))
+            continue;
+
+        // variable id of deallocated pointer..
+        const unsigned int varid(tok->varId());
+        if (varid == 0)
+            continue;
+
+        // variable id of allocated pointer must match..
+        if (varid != tok->tokAt(2)->varId())
+            continue;
+
+        // is is a class member variable..
+        const Token *tok1 = Token::findmatch(_tokenizer->tokens(), "%varid%", varid);
+        while (0 != (tok1 = tok1 ? tok1->previous() : 0))
+        {
+            if (tok1->str() == "}")
+                tok1 = tok1->link();
+            else if (tok1->str() == "{")
+            {
+                if (tok1->previous() && tok1->previous()->isName())
+                {
+                    while (0 != (tok1 = tok1 ? tok1->previous() : 0))
+                    {
+                        if (!tok1->isName() && tok1->str() != ":" && tok1->str() != ",")
+                            break;
+                        if (tok1->str() == "class")
+                        {
+                            reallocError(tok->tokAt(2), tok->str());
+                            break;
+                        }
+                    }
+                }
+                break;
+            }
+        }
+    }
+}

lib/checkexceptionsafety.h

@@ -47,6 +47,7 @@ public:
         CheckExceptionSafety checkExceptionSafety(tokenizer, settings, errorLogger);
         checkExceptionSafety.destructors();
         checkExceptionSafety.unsafeNew();
+        checkExceptionSafety.realloc();
     }
 
 
@@ -56,23 +57,33 @@ public:
     /** unsafe use of "new" */
     void unsafeNew();
 
+    /** Unsafe realloc */
+    void realloc();
+
 private:
     /** Don't throw exceptions in destructors */
     void destructorsError(const Token * const tok)
     {
-        reportError(tok, Severity::style, "throwInDestructor", "Throwing exception in destructor is not recommended");
+        reportError(tok, Severity::style, "exceptThrowInDestructor", "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'");
+        reportError(tok, Severity::style, "exceptNew", "Upon exception there is memory leaks");
+    }
+
+    /** Unsafe reallocation */
+    void reallocError(const Token * const tok, const std::string &varname)
+    {
+        reportError(tok, Severity::style, "exceptRealloc", "Upon exception " + varname + " becomes a dead pointer");
     }
 
     void getErrorMessages()
     {
         destructorsError(0);
         unsafeNewError(0);
+        reallocError(0, "p");
     }
 
     std::string name() const
@@ -83,8 +94,9 @@ private:
     std::string classInfo() const
     {
         return "Checking exception safety\n"
-               " * Don't throw exceptions in destructors"
+               " * Don't throw exceptions in destructors\n"
-               " * Unsafe use of 'new'";
+               " * Unsafe use of 'new'\n"
+               " * Unsafe reallocation\n";
     }
 };
 /// @}

test/testexceptionsafety.cpp

@@ -36,6 +36,7 @@ private:
     {
         TEST_CASE(destructors);
         TEST_CASE(newnew);
+        TEST_CASE(realloc);
     }
 
     void check(const std::string &code)
@@ -56,6 +57,7 @@ private:
         CheckExceptionSafety checkExceptionSafety(&tokenizer, &settings, this);
         checkExceptionSafety.destructors();
         checkExceptionSafety.unsafeNew();
+        checkExceptionSafety.realloc();
     }
 
     void destructors()
@@ -72,7 +74,7 @@ private:
         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());
+        ASSERT_EQUALS("[test.cpp:1]: (style) Upon exception there is memory leaks\n", errout.str());
 
         check("C::C() : a(new A), b(new B) { }");
         ASSERT_EQUALS("", errout.str());
@@ -82,14 +84,41 @@ private:
               "    a = new A;\n"
               "    b = new B;\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (style) Exception safety: unsafe use of 'new'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (style) Upon exception there is memory leaks\n", errout.str());
 
         check("void a()\n"
               "{\n"
               "    A *a1 = new A;\n"
               "    A *a2 = new A;\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (style) Exception safety: unsafe use of 'new'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (style) Upon exception there is memory leaks\n", errout.str());
+    }
+
+    void realloc()
+    {
+        check("class A\n"
+              "{\n"
+              "    int *p;\n"
+              "    void a()\n"
+              "    {\n"
+              "        delete p;\n"
+              "        p = new[123];\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (style) Upon exception p becomes a dead pointer\n", errout.str());
+
+        check("class A\n"
+              "{\n"
+              "    int *p;\n"
+              "    void a()\n"
+              "    {\n"
+              "        try {\n"
+              "            delete p;\n"
+              "            p = new[123];\n"
+              "        } catch (...) { p = 0; }\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 };