Fixed #733 (New check: locking & unlocking mutex and semaphore variables)

src/checkmemoryleak.cpp

@@ -326,7 +326,10 @@ void CheckMemoryLeak::memleakError(const Token *tok, const std::string &varname,
 
 void CheckMemoryLeak::resourceLeakError(const Token *tok, const std::string &varname, bool all)
 {
-    reportErr(tok, all ? Severity::possibleError : Severity::error, "resourceLeak", "Resource leak: " + varname);
+    std::string errmsg("Resource leak");
+    if (!varname.empty())
+        errmsg += ": " + varname;
+    reportErr(tok, all ? Severity::possibleError : Severity::error, "resourceLeak", errmsg);
 }
 
 void CheckMemoryLeak::deallocDeallocError(const Token *tok, const std::string &varname)
@@ -472,7 +475,7 @@ const char * CheckMemoryLeakInFunction::call_func(const Token *tok, std::list<co
                      sizeof(call_func_white_list[0]), call_func_white_list_compare))
         return 0;
 
-    if (getAllocationType(tok, varid) != No || getReallocationType(tok, varid) != No || getDeallocationType(tok, varid) != No)
+    if (varid > 0 && (getAllocationType(tok, varid) != No || getReallocationType(tok, varid) != No || getDeallocationType(tok, varid) != No))
         return 0;
 
     if (callstack.size() > 2)
@@ -481,11 +484,31 @@ const char * CheckMemoryLeakInFunction::call_func(const Token *tok, std::list<co
     const std::string funcname(tok->str());
     for (std::list<const Token *>::const_iterator it = callstack.begin(); it != callstack.end(); ++it)
     {
-        if ((*it)->str() == funcname)
+        if ((*it) && (*it)->str() == funcname)
             return "recursive";
     }
     callstack.push_back(tok);
 
+    // lock/unlock..
+    if (varid == 0)
+    {
+        const Token *ftok = _tokenizer->getFunctionTokenByName(funcname.c_str());
+        while (ftok && (ftok->str() != "{"))
+            ftok = ftok->next();
+        if (!ftok)
+            return 0;
+
+        Token *func = getcode(ftok->tokAt(1), callstack, 0, alloctype, dealloctype, false, all, 1);
+        simplifycode(func, all);
+        const char *ret = 0;
+        if (Token::simpleMatch(func, "; alloc ; }"))
+            ret = "alloc";
+        else if (Token::simpleMatch(func, "; dealloc ; }"))
+            ret = "dealloc";
+        Tokenizer::deleteTokens(func);
+        return ret;
+    }
+
     // Check if this is a function that allocates memory..
     {
         const Token *ftok = _tokenizer->getFunctionTokenByName(funcname.c_str());
@@ -519,7 +542,7 @@ const char * CheckMemoryLeakInFunction::call_func(const Token *tok, std::list<co
         {
             if (tok->str() == ",")
                 ++par;
-            if (Token::Match(tok, "[,()] %varid% [,()]", varid))
+            if (varid > 0 && Token::Match(tok, "[,()] %varid% [,()]", varid))
             {
                 const Token *ftok = _tokenizer->getFunctionTokenByName(funcname.c_str());
                 if (!ftok)
@@ -568,7 +591,7 @@ const char * CheckMemoryLeakInFunction::call_func(const Token *tok, std::list<co
 
 
 
-Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Token *> callstack, unsigned int varid, AllocType &alloctype, AllocType &dealloctype, bool classmember, bool &all, unsigned int sz)
+Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Token *> callstack, unsigned int varid, CheckMemoryLeak::AllocType &alloctype, CheckMemoryLeak::AllocType &dealloctype, bool classmember, bool &all, unsigned int sz)
 {
     Token *rethead = 0, *rettail = 0;
 #define addtoken(_str)                  \
@@ -620,225 +643,255 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Toke
         if (parlevel == 0 && tok->str() == ";")
             addtoken(";");
 
-        // var = strcpy|.. ( var ,
+        if (varid == 0)
-        if (Token::Match(tok, "[;{}] %varid% = memcpy|memmove|memset|strcpy|strncpy|strcat|strncat ( %varid% ,", varid))
         {
-            tok = tok->tokAt(4)->link();
+            if (!callstack.empty() && Token::Match(tok, "[;{}] __cppcheck_lock|__cppcheck_unlock ( ) ;"))
-            continue;
-        }
-
-        if (Token::Match(tok->previous(), "[(;{}] %varid% =", varid) ||
-            Token::Match(tok, "asprintf ( & %varid% ,", varid))
-        {
-            AllocType alloc;
-
-            if (Token::simpleMatch(tok, "asprintf ("))
             {
-                // todo: check how the return value is used.
+                // Type of leak = Resource leak
-                if (!Token::Match(tok->previous(), "[;{}]"))
+                alloctype = dealloctype = CheckMemoryLeak::File;
+
+                if (tok->next()->str() == "__cppcheck_lock")
                 {
-                    Tokenizer::deleteTokens(rethead);
-                    return 0;
-                }
-                alloc = Malloc;
-                tok = tok->next()->link();
-            }
-            else
-            {
-                alloc = getAllocationType(tok->tokAt(2), varid);
-            }
-            bool realloc = false;
-
-            if (sz > 1 &&
-                Token::Match(tok->tokAt(2), "malloc ( %num% )") &&
-                (MathLib::toLongNumber(tok->strAt(4)) % sz) != 0)
-            {
-                mismatchSizeError(tok->tokAt(4), tok->strAt(4));
-            }
-
-            if (alloc == No)
-            {
-                alloc = getReallocationType(tok->tokAt(2), varid);
-                if (alloc != No)
-                {
-                    addtoken("realloc");
-                    addtoken(";");
-                    realloc = true;
-                    tok = tok->tokAt(2);
-                    continue;
-                }
-            }
-
-            // If "--all" hasn't been given, don't check classes..
-            if (alloc == New)
-            {
-                if (Token::Match(tok->tokAt(2), "new %type% [(;]"))
-                {
-                    if (isclass(_tokenizer, tok->tokAt(3)))
-                    {
-                        if (_settings->_showAll)
-                        {
-
-                            if (_settings->isAutoDealloc(tok->strAt(3)))
-                            {
-                                // This class has automatic deallocation
-                                alloc = No;
-                            }
-                            else
-                            {
-                                // The checking will proceed.. but any error messages that are shown are shown thanks to "--all"
-                                all = true;
-                            }
-                        }
-                        else
-                            alloc = No;
-                    }
-                }
-            }
-
-            if (alloc != No)
-            {
-                if (! realloc)
                     addtoken("alloc");
-
+                }
-                if (alloctype != No && alloctype != alloc)
+                else
-                    alloc = Many;
-
-                if (alloc != Many && dealloctype != No && dealloctype != Many && dealloctype != alloc)
                 {
-                    callstack.push_back(tok);
+                    addtoken("dealloc");
-                    mismatchAllocDealloc(callstack, Token::findmatch(_tokenizer->tokens(), "%varid%", varid)->str());
-                    callstack.pop_back();
                 }
 
-                alloctype = alloc;
+                tok = tok->tokAt(3);
+                continue;
             }
 
-            else if (matchFunctionsThatReturnArg(tok->previous(), varid))
+            if (tok->str() == "if")
             {
-                addtoken("use");
+                addtoken("if");
-            }
+                tok = tok->next()->link();
-
-            // assignment..
-            else
-            {
-                // is the pointer in rhs?
-                bool rhs = false;
-                for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
-                {
-                    if (tok2->str() == ";")
-                        break;
-
-                    if (Token::Match(tok2, "[=+] %varid%", varid))
-                    {
-                        rhs = true;
-                        break;
-                    }
-                }
-
-                addtoken((rhs ? "use" : "assign"));
-            }
-        }
-
-        if (Token::Match(tok->previous(), "[;{})=] %var%"))
-        {
-            AllocType dealloc = getDeallocationType(tok, varid);
-            if (dealloc != No)
-            {
-                addtoken("dealloc");
-
-                if (dealloctype != No && dealloctype != dealloc)
-                    dealloc = Many;
-
-                if (dealloc != Many && alloctype != No && alloctype != Many && alloctype != dealloc)
-                {
-                    callstack.push_back(tok);
-                    mismatchAllocDealloc(callstack, Token::findmatch(_tokenizer->tokens(), "%varid%", varid)->str());
-                    callstack.pop_back();
-                }
-                dealloctype = dealloc;
                 continue;
             }
         }
-
+        else
-        // if else switch
-        if (tok->str() == "if")
         {
-            if (alloctype == Fd)
+            // var = strcpy|.. ( var ,
+            if (Token::Match(tok, "[;{}] %varid% = memcpy|memmove|memset|strcpy|strncpy|strcat|strncat ( %varid% ,", varid))
             {
-                if (Token::Match(tok, "if ( %varid% >= 0 )", varid) ||
+                tok = tok->tokAt(4)->link();
-                    Token::Match(tok, "if ( %varid% != -1 )", varid))
+                continue;
+            }
+
+            if (Token::Match(tok->previous(), "[(;{}] %varid% =", varid) ||
+                Token::Match(tok, "asprintf ( & %varid% ,", varid))
+            {
+                CheckMemoryLeak::AllocType alloc;
+
+                if (Token::simpleMatch(tok, "asprintf ("))
+                {
+                    // todo: check how the return value is used.
+                    if (!Token::Match(tok->previous(), "[;{}]"))
+                    {
+                        Tokenizer::deleteTokens(rethead);
+                        return 0;
+                    }
+                    alloc = Malloc;
+                    tok = tok->next()->link();
+                }
+                else
+                {
+                    alloc = getAllocationType(tok->tokAt(2), varid);
+                }
+                bool realloc = false;
+
+                if (sz > 1 &&
+                    Token::Match(tok->tokAt(2), "malloc ( %num% )") &&
+                    (MathLib::toLongNumber(tok->strAt(4)) % sz) != 0)
+                {
+                    mismatchSizeError(tok->tokAt(4), tok->strAt(4));
+                }
+
+                if (alloc == CheckMemoryLeak::No)
+                {
+                    alloc = getReallocationType(tok->tokAt(2), varid);
+                    if (alloc != CheckMemoryLeak::No)
+                    {
+                        addtoken("realloc");
+                        addtoken(";");
+                        realloc = true;
+                        tok = tok->tokAt(2);
+                        continue;
+                    }
+                }
+
+                // If "--all" hasn't been given, don't check classes..
+                if (alloc == CheckMemoryLeak::New)
+                {
+                    if (Token::Match(tok->tokAt(2), "new %type% [(;]"))
+                    {
+                        if (isclass(_tokenizer, tok->tokAt(3)))
+                        {
+                            if (_settings->_showAll)
+                            {
+
+                                if (_settings->isAutoDealloc(tok->strAt(3)))
+                                {
+                                    // This class has automatic deallocation
+                                    alloc = No;
+                                }
+                                else
+                                {
+                                    // The checking will proceed.. but any error messages that are shown are shown thanks to "--all"
+                                    all = true;
+                                }
+                            }
+                            else
+                                alloc = No;
+                        }
+                    }
+                }
+
+                if (alloc != No)
+                {
+                    if (! realloc)
+                        addtoken("alloc");
+
+                    if (alloctype != No && alloctype != alloc)
+                        alloc = Many;
+
+                    if (alloc != Many && dealloctype != No && dealloctype != Many && dealloctype != alloc)
+                    {
+                        callstack.push_back(tok);
+                        mismatchAllocDealloc(callstack, Token::findmatch(_tokenizer->tokens(), "%varid%", varid)->str());
+                        callstack.pop_back();
+                    }
+
+                    alloctype = alloc;
+                }
+
+                else if (matchFunctionsThatReturnArg(tok->previous(), varid))
+                {
+                    addtoken("use");
+                }
+
+                // assignment..
+                else
+                {
+                    // is the pointer in rhs?
+                    bool rhs = false;
+                    for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
+                    {
+                        if (tok2->str() == ";")
+                            break;
+
+                        if (Token::Match(tok2, "[=+] %varid%", varid))
+                        {
+                            rhs = true;
+                            break;
+                        }
+                    }
+
+                    addtoken((rhs ? "use" : "assign"));
+                }
+            }
+
+            if (Token::Match(tok->previous(), "[;{})=] %var%"))
+            {
+                AllocType dealloc = getDeallocationType(tok, varid);
+                if (dealloc != No)
+                {
+                    addtoken("dealloc");
+
+                    if (dealloctype != No && dealloctype != dealloc)
+                        dealloc = Many;
+
+                    if (dealloc != Many && alloctype != No && alloctype != Many && alloctype != dealloc)
+                    {
+                        callstack.push_back(tok);
+                        mismatchAllocDealloc(callstack, Token::findmatch(_tokenizer->tokens(), "%varid%", varid)->str());
+                        callstack.pop_back();
+                    }
+                    dealloctype = dealloc;
+                    continue;
+                }
+            }
+
+            // if else switch
+            if (tok->str() == "if")
+            {
+                if (alloctype == Fd)
+                {
+                    if (Token::Match(tok, "if ( %varid% >= 0 )", varid) ||
+                        Token::Match(tok, "if ( %varid% != -1 )", varid))
+                    {
+                        addtoken("if(var)");
+                        tok = tok->next()->link();
+                        continue;
+                    }
+                    else if (Token::Match(tok, "if ( %varid% == -1 )", varid) ||
+                             Token::Match(tok, "if ( %varid% < 0 )", varid))
+                    {
+                        addtoken("if(!var)");
+                        tok = tok->next()->link();
+                        continue;
+                    }
+                }
+
+                if (Token::Match(tok, "if ( %varid% )", varid))
                 {
                     addtoken("if(var)");
+
+                    // Make sure the "use" will not be added
                     tok = tok->next()->link();
-                    continue;
                 }
-                else if (Token::Match(tok, "if ( %varid% == -1 )", varid) ||
+                else if (Token::simpleMatch(tok, "if (") && notvar(tok->tokAt(2), varid, true))
-                         Token::Match(tok, "if ( %varid% < 0 )", varid))
-                {
-                    addtoken("if(!var)");
-                    tok = tok->next()->link();
-                    continue;
-                }
-            }
-
-            if (Token::Match(tok, "if ( %varid% )", varid))
-            {
-                addtoken("if(var)");
-
-                // Make sure the "use" will not be added
-                tok = tok->next()->link();
-            }
-            else if (Token::simpleMatch(tok, "if (") && notvar(tok->tokAt(2), varid, true))
-            {
-                addtoken("if(!var)");
-            }
-            else
-            {
-                // Check if the condition depends on var somehow..
-                bool dep = false;
-                int parlevel = 0;
-                for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
-                {
-                    if (tok2->str() == "(")
-                        ++parlevel;
-                    if (tok2->str() == ")")
-                    {
-                        --parlevel;
-                        if (parlevel <= 0)
-                            break;
-                    }
-                    if (Token::Match(tok2, "close|fclose|closedir ( %varid% )", varid))
-                    {
-                        addtoken("dealloc");
-                        addtoken(";");
-                        dep = true;
-                        break;
-                    }
-                    if (Token::Match(tok2, ". %varid% !!.", varid))
-                    {
-                        dep = true;
-                        break;
-                    }
-                    if (alloctype == Fd && Token::Match(tok2, "%varid% !=|>=", varid))
-                    {
-                        dep = true;
-                    }
-                }
-
-                if (Token::Match(tok, "if ( ! %varid% &&", varid))
-                {
-                    addtoken("if(!var)");
-                }
-                else if (tok->next() &&
-                         tok->next()->link() &&
-                         Token::Match(tok->next()->link()->tokAt(-3), "&& ! %varid%", varid))
                 {
                     addtoken("if(!var)");
                 }
                 else
                 {
-                    addtoken((dep ? "ifv" : "if"));
+                    // Check if the condition depends on var somehow..
+                    bool dep = false;
+                    int parlevel = 0;
+                    for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
+                    {
+                        if (tok2->str() == "(")
+                            ++parlevel;
+                        if (tok2->str() == ")")
+                        {
+                            --parlevel;
+                            if (parlevel <= 0)
+                                break;
+                        }
+                        if (Token::Match(tok2, "close|fclose|closedir ( %varid% )", varid))
+                        {
+                            addtoken("dealloc");
+                            addtoken(";");
+                            dep = true;
+                            break;
+                        }
+                        if (Token::Match(tok2, ". %varid% !!.", varid))
+                        {
+                            dep = true;
+                            break;
+                        }
+                        if (alloctype == Fd && Token::Match(tok2, "%varid% !=|>=", varid))
+                        {
+                            dep = true;
+                        }
+                    }
+
+                    if (Token::Match(tok, "if ( ! %varid% &&", varid))
+                    {
+                        addtoken("if(!var)");
+                    }
+                    else if (tok->next() &&
+                             tok->next()->link() &&
+                             Token::Match(tok->next()->link()->tokAt(-3), "&& ! %varid%", varid))
+                    {
+                        addtoken("if(!var)");
+                    }
+                    else
+                    {
+                        addtoken((dep ? "ifv" : "if"));
+                    }
                 }
             }
         }
@@ -876,17 +929,21 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Toke
         {
             addtoken("do");
         }
-        if (isloop && notvar(tok, varid))
+        if (varid > 0 && isloop && notvar(tok, varid))
+        {
             addtoken("!var");
+        }
 
         // continue / break..
         if (tok->str() == "continue")
+        {
             addtoken("continue");
-        if (tok->str() == "break")
+        }
+        else if (tok->str() == "break")
+        {
             addtoken("break");
-
+        }
-        // goto..
+        else if (tok->str() == "goto")
-        if (tok->str() == "goto")
         {
             addtoken("goto");
         }
@@ -895,6 +952,15 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Toke
         else if (tok->str() == "return")
         {
             addtoken("return");
+            if (varid == 0)
+            {
+                addtoken(";");
+                while (tok && tok->str() != ";")
+                    tok = tok->next();
+                if (!tok)
+                    break;
+                continue;
+            }
 
             // Returning a auto_ptr of this allocated variable..
             if (Token::simpleMatch(tok->next(), "std :: auto_ptr <"))
@@ -1010,22 +1076,27 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Toke
                 const char *str = call_func(tok, callstack, varid, alloctype, dealloctype, all, sz);
                 if (str)
                 {
-                    if (Token::Match(tok->tokAt(-2), "%varid% =", varid) || std::string(str) != "alloc")
+                    if (varid == 0)
-                        addtoken(str);
-                }
-                else
-                {
-                    if (getReallocationType(tok, varid) != No &&
-                        tok->tokAt(2)->varId() == varid)
                     {
-                        addtoken("if");
+                        addtoken(str);
-                        addtoken("{");
-                        addtoken("dealloc");
-                        addtoken(";");
-                        addtoken("}");
-                        tok = tok->next()->link();
-                        continue;
                     }
+                    else if (str != std::string("alloc") ||
+                             Token::Match(tok->tokAt(-2), "%varid% =", varid))
+                    {
+                        addtoken(str);
+                    }
+                }
+                else if (varid > 0 &&
+                         getReallocationType(tok, varid) != No &&
+                         tok->tokAt(2)->varId() == varid)
+                {
+                    addtoken("if");
+                    addtoken("{");
+                    addtoken("dealloc");
+                    addtoken(";");
+                    addtoken("}");
+                    tok = tok->next()->link();
+                    continue;
                 }
             }
         }
@@ -1055,7 +1126,7 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Toke
         }
 
         // Linux lists..
-        if (Token::Match(tok, "[=(,] & %varid% [.[,)]", varid))
+        if (varid > 0 && Token::Match(tok, "[=(,] & %varid% [.[,)]", varid))
         {
             addtoken("&use");
         }
@@ -1841,7 +1912,10 @@ void CheckMemoryLeakInFunction::check()
         if (indentlevel == 0)
         {
             if (Token::simpleMatch(tok, ") {"))
+            {
                 infunc = true;
+                checkScope(tok->tokAt(2), "", 0, classmember, 1);
+            }
 
             else if (tok->str() == "(")
                 beforeParameters = false;

test/testmemleak.cpp

@@ -287,6 +287,7 @@ private:
         // getcode..
         CheckMemoryLeakInFunction checkMemoryLeak(&tokenizer, 0, 0);
         std::list<const Token *> callstack;
+        callstack.push_back(0);
         CheckMemoryLeak::AllocType allocType, deallocType;
         allocType = deallocType = CheckMemoryLeak::No;
         bool all = false;
@@ -322,6 +323,12 @@ private:
         // alloc; return;
         ASSERT_EQUALS(";;alloc;return;", getcode("char *s = new char[100]; return 0;", "s"));
 
+        // lock/unlock..
+        ASSERT_EQUALS(";;alloc;", getcode("int a; __cppcheck_lock();", ""));
+        ASSERT_EQUALS(";;callfunc;", getcode("int a; __cppcheck_lock();", "a"));
+        ASSERT_EQUALS(";;dealloc;", getcode("int a; __cppcheck_unlock();", ""));
+        ASSERT_EQUALS(";;callfunc;", getcode("int a; __cppcheck_unlock();", "a"));
+
         // dealloc;
         ASSERT_EQUALS(";;dealloc;", getcode("char *s; free(s);", "s"));
         ASSERT_EQUALS(";;dealloc;", getcode("char *s; free((void *)s);", "s"));