checkautovariables: refactor to use the symbol database

lib/checkautovariables.cpp

@@ -134,52 +134,49 @@ void CheckAutoVariables::addVDA(unsigned int varId)
 
 void CheckAutoVariables::autoVariables()
 {
-    bool begin_function = false;
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
-    bool begin_function_decl = false;
-    int bindent = 0;
 
-    // Which variables have an unknown type?
+    std::list<Scope *>::const_iterator i;
-    std::set<unsigned int> unknown_type;
 
-    const SymbolDatabase * const symbolDatabase = _tokenizer->getSymbolDatabase();
+    for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i)
-
-    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
+        const Scope *scope = *i;
+
+        // only check functions
+        if (scope->type != Scope::eFunction)
+            continue;
 
-        if (Token::Match(tok, "%type% *|::| %var% ("))
-        {
-            begin_function = true;
         fp_list.clear();
         vd_list.clear();
         vda_list.clear();
-        }
+
-        else if (begin_function && begin_function_decl && Token::Match(tok, "%type% * * %var%"))
+        const Token *tok = scope->classDef->next();
+
+        for (; tok && tok != scope->classDef->next()->link(); tok = tok->next())
+        {
+            if (Token::Match(tok, "%type% * * %var%"))
             {
                 fp_list.insert(tok->tokAt(3)->str());
             }
-        else if (begin_function && begin_function_decl && Token::Match(tok, "%type% * %var% ["))
+            else if (Token::Match(tok, "%type% * %var% ["))
             {
                 fp_list.insert(tok->tokAt(2)->str());
             }
-        else if (begin_function && tok->str() == "(")
-        {
-            begin_function_decl = true;
         }
-        else if (begin_function && tok->str() == ")")
+
+        unsigned int indentlevel = 0;
+        // Which variables have an unknown type?
+        std::set<unsigned int> unknown_type;
+        for (tok = scope->classDef->next()->link(); tok; tok = tok->next())
         {
-            begin_function_decl = false;
+            // indentlevel..
-        }
+            if (tok->str() == "{")
-        else if (begin_function && tok->str() == "{")
+                ++indentlevel;
+            else if (tok->str() == "}")
             {
-            bindent++;
+                if (indentlevel <= 1)
-        }
+                    break;
-        else if (begin_function && tok->str() == "}")
+                --indentlevel;
-        {
-            bindent--;
-        }
-        else if (bindent <= 0)
-        {
-            continue;
             }
 
             // Inside a function body
@@ -235,57 +232,57 @@ void CheckAutoVariables::autoVariables()
         vda_list.clear();
         fp_list.clear();
     }
+}
+
 //---------------------------------------------------------------------------
 
-
-
 void CheckAutoVariables::returnPointerToLocalArray()
 {
-    bool infunc = false;
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
-    int indentlevel = 0;
-    std::set<unsigned int> arrayVar;
-    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
-    {
-        // Is there a function declaration for a function that returns a pointer?
-        if (!infunc && (Token::Match(tok, "%type% * %var% (") || Token::Match(tok, "%type% * %var% :: %var% (")))
-        {
-            for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
-            {
-                if (tok2->str() == ")")
-                {
-                    tok = tok2;
-                    break;
-                }
-            }
-            if (Token::simpleMatch(tok, ") {"))
-            {
-                infunc = true;
-                indentlevel = 0;
-                arrayVar.clear();
-            }
-        }
 
-        // Parsing a function that returns a pointer..
+    std::list<Scope *>::const_iterator i;
-        if (infunc)
+
+    for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i)
     {
-            if (tok->str() == "{")
+        const Scope *scope = *i;
-            {
+
-                ++indentlevel;
+        // only check functions
-            }
+        if (scope->type != Scope::eFunction)
-            else if (tok->str() == "}")
-            {
-                --indentlevel;
-                if (indentlevel <= 0)
-                {
-                    infunc = false;
-                }
             continue;
+
+        const Token *tok = scope->classDef;
+
+        // skip any qualification
+        while (Token::Match(tok->tokAt(-2), "%type% ::"))
+            tok = tok->tokAt(-2);
+
+        // have we reached a function that returns a pointer
+        if (Token::Match(tok->tokAt(-2), "%type% *"))
+        {
+            // go to the '('
+            const Token *tok2 = scope->classDef->next();
+
+            // go to the ')'
+            tok2 = tok2->next()->link();
+
+            unsigned int indentlevel = 0;
+            std::set<unsigned int> arrayVar;
+            for (; tok2; tok2 = tok2->next())
+            {
+                // indentlevel..
+                if (tok2->str() == "{")
+                    ++indentlevel;
+                else if (tok2->str() == "}")
+                {
+                    if (indentlevel <= 1)
+                        break;
+                    --indentlevel;
                 }
 
                 // Declaring a local array..
-            if (Token::Match(tok, "[;{}] %type% %var% ["))
+                if (Token::Match(tok2, "[;{}] %type% %var% ["))
                 {
-                const unsigned int varid = tok->tokAt(2)->varId();
+                    const unsigned int varid = tok2->tokAt(2)->varId();
                     if (varid > 0)
                     {
                         arrayVar.insert(varid);
@@ -293,19 +290,16 @@ void CheckAutoVariables::returnPointerToLocalArray()
                 }
 
                 // Return pointer to local array variable..
-            if (Token::Match(tok, "return %var% ;"))
+                if (Token::Match(tok2, "return %var% ;"))
                 {
-                const unsigned int varid = tok->next()->varId();
+                    const unsigned int varid = tok2->next()->varId();
                     if (varid > 0 && arrayVar.find(varid) != arrayVar.end())
                     {
-                    errorReturnPointerToLocalArray(tok);
+                        errorReturnPointerToLocalArray(tok2);
+                    }
                 }
             }
         }
-
-        // Declaring array variable..
-
-
     }
 }
 
@@ -324,6 +318,8 @@ void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok)
                 "is invalid after the function ends.");
 }
 
+//---------------------------------------------------------------------------
+
 // return temporary?
 bool CheckAutoVariables::returnTemporary(const Token *tok) const
 {
@@ -332,38 +328,41 @@ bool CheckAutoVariables::returnTemporary(const Token *tok) const
     return bool(0 != Token::findmatch(_tokenizer->tokens(), ("std :: string " + tok->next()->str() + " (").c_str()));
 }
 
+//---------------------------------------------------------------------------
 
 void CheckAutoVariables::returnReference()
 {
-    // locate function that returns a reference..
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
-    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+
+    std::list<Scope *>::const_iterator i;
+
+    for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i)
     {
-        // Skip executable scopes..
+        const Scope *scope = *i;
-        if (Token::Match(tok, ") const| {"))
+
-        {
+        // only check functions
-            tok = tok->next();
+        if (scope->type != Scope::eFunction)
-            if (tok->str() == "const")
-                tok = tok->next();
-            tok = tok->link();
             continue;
-        }
+
+        const Token *tok = scope->classDef;
+
+        // skip any qualification
+        while (Token::Match(tok->tokAt(-2), "%type% ::"))
+            tok = tok->tokAt(-2);
 
         // have we reached a function that returns a reference?
-        if (Token::Match(tok, "%type% & %var% (") ||
+        if (Token::Match(tok->tokAt(-2), "%type% &") ||
-            Token::Match(tok, "> & %var% ("))
+            Token::Match(tok->tokAt(-2), "> &"))
         {
             // go to the '('
-            const Token *tok2 = tok->tokAt(3);
+            const Token *tok2 = scope->classDef->next();
 
             // go to the ')'
             tok2 = tok2->link();
 
-            // is this a function implementation?
-            if (Token::Match(tok2, ") const| {"))
-            {
             unsigned int indentlevel = 0;
             std::set<unsigned int> localvar;    // local variables in function
-                while (0 != (tok2 = tok2->next()))
+            for (; tok2; tok2 = tok2->next())
             {
                 // indentlevel..
                 if (tok2->str() == "{")
@@ -418,7 +417,6 @@ void CheckAutoVariables::returnReference()
         }
     }
 }
-}
 
 void CheckAutoVariables::errorReturnReference(const Token *tok)
 {
@@ -430,42 +428,42 @@ void CheckAutoVariables::errorReturnTempReference(const Token *tok)
     reportError(tok, Severity::error, "returnTempReference", "Returning reference to temporary");
 }
 
+//---------------------------------------------------------------------------
 
 // Return c_str
 void CheckAutoVariables::returncstr()
 {
     // locate function that returns a const char *..
-    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
-    {
-        // Skip executable scopes..
-        if (Token::Match(tok, ") const| {"))
-        {
-            tok = tok->next();
-            if (tok->str() == "const")
-                tok = tok->next();
-            tok = tok->link();
-            continue;
-        }
 
-        // have we reached a function that returns a reference?
+    std::list<Scope *>::const_iterator i;
-        if (Token::simpleMatch(tok, "const char *"))
+
+    for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i)
+    {
+        const Scope *scope = *i;
+
+        // only check functions
+        if (scope->type != Scope::eFunction)
+            continue;
+
+        const Token *tok = scope->classDef;
+
+        // skip any qualification
+        while (Token::Match(tok->tokAt(-2), "%type% ::"))
+            tok = tok->tokAt(-2);
+
+        // have we reached a function that returns a const char *
+        if (Token::simpleMatch(tok->tokAt(-3), "const char *"))
         {
             // go to the '('
-            const Token *tok2 = tok->tokAt(3);
+            const Token *tok2 = scope->classDef->next();
-            while (Token::Match(tok2, "%var% ::"))
-                tok2 = tok2->tokAt(2);
-            if (!Token::Match(tok2, "%var% ("))
-                continue;
 
             // go to the ')'
             tok2 = tok2->next()->link();
 
-            // is this a function implementation?
-            if (Token::Match(tok2, ") const| {"))
-            {
             unsigned int indentlevel = 0;
             std::set<unsigned int> localvar;    // local variables in function
-                while (0 != (tok2 = tok2->next()))
+            for (; tok2; tok2 = tok2->next())
             {
                 // indentlevel..
                 if (tok2->str() == "{")
@@ -518,7 +516,6 @@ void CheckAutoVariables::returncstr()
         }
     }
 }
-}
 
 void CheckAutoVariables::errorReturnAutocstr(const Token *tok)
 {
@@ -529,6 +526,3 @@ void CheckAutoVariables::errorReturnTempPointer(const Token *tok)
 {
     reportError(tok, Severity::error, "returnTempPointer", "Returning pointer to temporary");
 }
-
-
-

test/testautovariables.cpp

@@ -65,9 +65,12 @@ private:
 
     void run()
     {
-        TEST_CASE(testautovar);
+        TEST_CASE(testautovar1);
-        TEST_CASE(testautovar_array);
+        TEST_CASE(testautovar2);
-        TEST_CASE(testautovar_return);
+        TEST_CASE(testautovar_array1);
+        TEST_CASE(testautovar_array2);
+        TEST_CASE(testautovar_return1);
+        TEST_CASE(testautovar_return2);
         TEST_CASE(testautovar_extern);
         TEST_CASE(testinvaliddealloc);
         TEST_CASE(testassign);  // Ticket #1819
@@ -76,15 +79,17 @@ private:
         TEST_CASE(returnLocalVariable2);
 
         // return reference..
-        TEST_CASE(returnReference);
+        TEST_CASE(returnReference1);
+        TEST_CASE(returnReference2);
 
         // return c_str()..
-        TEST_CASE(returncstr);
+        TEST_CASE(returncstr1);
+        TEST_CASE(returncstr2);
     }
 
 
 
-    void testautovar()
+    void testautovar1()
     {
         check("void func1(int **res)\n"
               "{\n"
@@ -101,7 +106,30 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
-    void testautovar_array()
+    void testautovar2()
+    {
+        check("class Fred {\n"
+              "    void func1(int **res);\n"
+              "}\n"
+              "void Fred::func1(int **res)\n"
+              "{\n"
+              "    int num = 2;\n"
+              "    *res = #\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:6]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str());
+
+        check("class Fred {\n"
+              "    void func1(int **res);\n"
+              "}\n"
+              "void Fred::func1(int **res)\n"
+              "{\n"
+              "    int num = 2;\n"
+              "    foo.res = #\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void testautovar_array1()
     {
         check("void func1(int* arr[2])\n"
               "{\n"
@@ -111,15 +139,42 @@ private:
         ASSERT_EQUALS("[test.cpp:3]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str());
     }
 
-    void testautovar_return()
+    void testautovar_array2()
+    {
+        check("class Fred {\n"
+              "    void func1(int* arr[2]);\n"
+              "}\n"
+              "void Fred::func1(int* arr[2])\n"
+              "{\n"
+              "    int num=2;"
+              "    arr[0]=#\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:6]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str());
+    }
+
+    void testautovar_return1()
     {
         check("int* func1()\n"
               "{\n"
               "    int num=2;"
-              "return #}");
+              "    return #"
+              "}");
         ASSERT_EQUALS("[test.cpp:3]: (error) Return of the address of an auto-variable\n", errout.str());
     }
 
+    void testautovar_return2()
+    {
+        check("class Fred {\n"
+              "    int* func1()\n"
+              "}\n"
+              "int* Fred::func1()\n"
+              "{\n"
+              "    int num=2;"
+              "    return #"
+              "}");
+        ASSERT_EQUALS("[test.cpp:6]: (error) Return of the address of an auto-variable\n", errout.str());
+    }
+
     void testautovar_extern()
     {
         check("struct foo *f()\n"
@@ -169,6 +224,16 @@ private:
               "    return str;\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:4]: (error) Returning pointer to local array variable\n", errout.str());
+
+        check("class Fred {\n"
+              "    char *foo();\n"
+              "};\n"
+              "char *Fred::foo()\n"
+              "{\n"
+              "    char str[100] = {0};\n"
+              "    return str;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Returning pointer to local array variable\n", errout.str());
     }
 
     void returnLocalVariable2()
@@ -179,9 +244,19 @@ private:
               "    return str;\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("class Fred {\n"
+              "    std::string foo();\n"
+              "};\n"
+              "std::string Fred::foo()\n"
+              "{\n"
+              "    char str[100] = {0};\n"
+              "    return str;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
-    void returnReference()
+    void returnReference1()
     {
         check("std::string &foo()\n"
               "{\n"
@@ -216,7 +291,67 @@ private:
         ASSERT_EQUALS("[test.cpp:8]: (error) Returning reference to temporary\n", errout.str());
     }
 
-    void returncstr()
+    void returnReference2()
+    {
+        check("class Fred {\n"
+              "    std::string &foo();\n"
+              "}\n"
+              "std::string &Fred::foo()\n"
+              "{\n"
+              "    std::string s;\n"
+              "    return s;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Returning reference to auto variable\n", errout.str());
+
+        check("class Fred {\n"
+              "    std::vector<int> &foo();\n"
+              "};\n"
+              "std::vector<int> &Fred::foo()\n"
+              "{\n"
+              "    std::vector<int> v;\n"
+              "    return v;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Returning reference to auto variable\n", errout.str());
+
+        check("class Fred {\n"
+              "    std::vector<int> &foo();\n"
+              "};\n"
+              "std::vector<int> &Fred::foo()\n"
+              "{\n"
+              "    static std::vector<int> v;\n"
+              "    return v;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("class Fred {\n"
+              "    std::string &f();\n"
+              "};\n"
+              "std::string hello()\n"
+              "{\n"
+              "     return \"hello\";\n"
+              "}\n"
+              "std::string &Fred::f()\n"
+              "{\n"
+              "    return hello();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:10]: (error) Returning reference to temporary\n", errout.str());
+
+        check("class Fred {\n"
+              "    std::string hello();\n"
+              "    std::string &f();\n"
+              "};\n"
+              "std::string Fred::hello()\n"
+              "{\n"
+              "     return \"hello\";\n"
+              "}\n"
+              "std::string &Fred::f()\n"
+              "{\n"
+              "    return hello();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:11]: (error) Returning reference to temporary\n", errout.str());
+    }
+
+    void returncstr1()
     {
         check("const char *foo()\n"
               "{\n"
@@ -244,6 +379,43 @@ private:
         ASSERT_EQUALS("[test.cpp:8]: (error) Returning pointer to temporary\n", errout.str());
     }
 
+    void returncstr2()
+    {
+        check("class Fred {\n"
+              "    const char *foo();\n"
+              "};\n"
+              "const char *Fred::foo()\n"
+              "{\n"
+              "    std::string s;\n"
+              "    return s.c_str();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Returning pointer to auto variable\n", errout.str());
+
+        check("class Fred {\n"
+              "    const char *foo();\n"
+              "};\n"
+              "const char *Foo::f()\n"
+              "{\n"
+              "    std::string s;\n"
+              "    return s.c_str();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (error) Returning pointer to auto variable\n", errout.str());
+
+        check("class Fred {\n"
+              "    std::string hello();\n"
+              "    const char *f();\n"
+              "};\n"
+              "std::string Fred::hello()\n"
+              "{\n"
+              "     return \"hello\";\n"
+              "}\n"
+              "const char *Fred::f()\n"
+              "{\n"
+              "    return hello().c_str();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:11]: (error) Returning pointer to temporary\n", errout.str());
+    }
+
 };
 
 REGISTER_TEST(TestAutoVariables)