Fixed #8262: false positive memleak (`shared_ptr` in function argument) (#1249)

* Fixed #8262.
checkleakautovar.cpp:
* added `isFunctionCall` (supports template functions)
* smart pointer check in `functionCall`
* updated test case

* Renamed "bracket" to "parenthesis"

* (#8262) Fixed broken test

* (#8262) Code review comments

* (#8262) Renamed `tokOpeningBr` to `tokOpeningPar`

lib/checkleakautovar.cpp

@@ -206,6 +206,31 @@ static bool isPointerReleased(const Token *startToken, const Token *endToken, un
     return false;
 }
 
+/** checks if nameToken is a name of a function in a function call:
+*     func(arg)
+* or
+*     func<temp1_arg>(arg)
+* @param nameToken Function name token
+* @return opening parenthesis token or NULL if not a function call
+*/
+
+static const Token * isFunctionCall(const Token * nameToken) {
+    if (nameToken->isName()) {
+        nameToken = nameToken->next();
+        // check if function is a template
+        if (nameToken && nameToken->link() && nameToken->str() == "<") {
+            // skip template arguments
+            nameToken = nameToken->link()->next();
+        }
+        // check for '('
+        if (nameToken && nameToken->link() && nameToken->str() == "(") {
+            // returning opening parenthesis pointer
+            return nameToken;
+        }
+    }
+    return nullptr;
+}
+
 void CheckLeakAutoVar::checkScope(const Token * const startToken,
                                   VarInfo *varInfo,
                                   std::set<unsigned int> notzero)
@@ -223,29 +248,15 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
                 break;
         }
 
-        // Deallocation and then dereferencing pointer..
+        // check each token
-        if (tok->varId() > 0) {
+        {
-            const std::map<unsigned int, VarInfo::AllocInfo>::const_iterator var = alloctype.find(tok->varId());
+            const Token * nextTok = checkTokenInsideExpression(tok, varInfo);
-            if (var != alloctype.end()) {
+            if (nextTok) {
-                bool unknown = false;
+                tok = nextTok;
-                if (var->second.status == VarInfo::DEALLOC && CheckNullPointer::isPointerDeRef(tok,unknown) && !unknown) {
+                continue;
-                    deallocUseError(tok, tok->str());
-                } else if (Token::simpleMatch(tok->tokAt(-2), "= &")) {
-                    varInfo->erase(tok->varId());
-                } else if (tok->strAt(-1) == "=") {
-                    varInfo->erase(tok->varId());
-                }
-            } else if (Token::Match(tok->previous(), "& %name% = %var% ;")) {
-                varInfo->referenced.insert(tok->tokAt(2)->varId());
             }
         }
 
-        if (tok->str() == "(" && tok->previous()->isName()) {
-            const VarInfo::AllocInfo allocation(0, VarInfo::NOALLOC);
-            functionCall(tok->previous(), varInfo, allocation, nullptr);
-            tok = tok->link();
-            continue;
-        }
 
         // look for end of statement
         if (!Token::Match(tok, "[;{}]") || Token::Match(tok->next(), "[;{}]"))
@@ -268,8 +279,10 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
 
         // assignment..
         if (Token::Match(varTok, "%var% =")) {
+            const Token* const tokAssignOp = varTok->next();
+
             // taking address of another variable..
-            if (Token::Match(varTok->next(), "= %var% [+;]")) {
+            if (Token::Match(tokAssignOp, "= %var% [+;]")) {
                 if (varTok->tokAt(2)->varId() != varTok->varId()) {
                     // If variable points at allocated memory => error
                     leakIfAllocated(varTok, *varInfo);
@@ -286,8 +299,11 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
                 }
             }
 
+            // right ast part (after `=` operator)
+            const Token* const tokRightAstOperand = tokAssignOp->astOperand2();
+
             // is variable used in rhs?
-            if (isVarUsedInTree(varTok->next()->astOperand2(), varTok->varId()))
+            if (isVarUsedInTree(tokRightAstOperand, varTok->varId()))
                 continue;
 
             // Variable has already been allocated => error
@@ -316,8 +332,8 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
             }
 
             // allocation?
-            if (varTok->next()->astOperand2() && Token::Match(varTok->next()->astOperand2()->previous(), "%type% (")) {
+            if (tokRightAstOperand && Token::Match(tokRightAstOperand->previous(), "%type% (")) {
-                const Library::AllocFunc* f = _settings->library.alloc(varTok->next()->astOperand2()->previous());
+                const Library::AllocFunc* f = _settings->library.alloc(tokRightAstOperand->previous());
                 if (f && f->arg == -1) {
                     VarInfo::AllocInfo& varAlloc = alloctype[varTok->varId()];
                     varAlloc.type = f->groupId;
@@ -344,7 +360,11 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
         // if/else
         else if (Token::simpleMatch(tok, "if (")) {
             // Parse function calls inside the condition
-            for (const Token *innerTok = tok->tokAt(2); innerTok; innerTok = innerTok->next()) {
+
+            const Token * closingParenthesis = tok->linkAt(1);
+            for (const Token *innerTok = tok->tokAt(2); innerTok && innerTok != closingParenthesis; innerTok = innerTok->next()) {
+                // TODO: replace with checkTokenInsideExpression()
+                
                 if (Token::Match(innerTok, "%var% =")) {
                     // allocation?
                     if (Token::Match(innerTok->tokAt(2), "%type% (")) {
@@ -363,17 +383,17 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
                     }
                 }
 
-                if (innerTok->str() == ")")
+                // check for function call
-                    break;
+                const Token * const openingPar = isFunctionCall(innerTok);
-                if (innerTok->str() == "(" && innerTok->previous()->isName()) {
+                if (openingPar) {
+                    // innerTok is a function name
                     const VarInfo::AllocInfo allocation(0, VarInfo::NOALLOC);
-                    functionCall(innerTok->previous(), varInfo, allocation, nullptr);
+                    functionCall(innerTok, openingPar, varInfo, allocation, nullptr);
-                    innerTok = innerTok->link();
+                    innerTok = openingPar->link();
                 }
             }
 
-            const Token *tok2 = tok->linkAt(1);
+            if (Token::simpleMatch(closingParenthesis, ") {")) {
-            if (Token::simpleMatch(tok2, ") {")) {
                 VarInfo varInfo1(*varInfo);  // VarInfo for if code
                 VarInfo varInfo2(*varInfo);  // VarInfo for else code
 
@@ -429,13 +449,13 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
                     }
                 }
 
-                checkScope(tok2->next(), &varInfo1, notzero);
+                checkScope(closingParenthesis->next(), &varInfo1, notzero);
-                tok2 = tok2->linkAt(1);
+                closingParenthesis = closingParenthesis->linkAt(1);
-                if (Token::simpleMatch(tok2, "} else {")) {
+                if (Token::simpleMatch(closingParenthesis, "} else {")) {
-                    checkScope(tok2->tokAt(2), &varInfo2, notzero);
+                    checkScope(closingParenthesis->tokAt(2), &varInfo2, notzero);
-                    tok = tok2->linkAt(2)->previous();
+                    tok = closingParenthesis->linkAt(2)->previous();
                 } else {
-                    tok = tok2->previous();
+                    tok = closingParenthesis->previous();
                 }
 
                 VarInfo old;
@@ -520,12 +540,13 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
         }
 
         // Function call..
-        else if (Token::Match(ftok, "%type% (")) {
+        else if (isFunctionCall(ftok)) {
+            const Token * openingPar = isFunctionCall(ftok);
             const Library::AllocFunc* af = _settings->library.dealloc(ftok);
             VarInfo::AllocInfo allocation(af ? af->groupId : 0, VarInfo::DEALLOC);
             if (allocation.type == 0)
                 allocation.status = VarInfo::NOALLOC;
-            functionCall(ftok, varInfo, allocation, af);
+            functionCall(ftok, openingPar, varInfo, allocation, af);
 
             tok = ftok->next()->link();
 
@@ -644,6 +665,45 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken,
     }
 }
 
+
+const Token * CheckLeakAutoVar::checkTokenInsideExpression(const Token * const tok, VarInfo *varInfo){
+    std::map<unsigned int, VarInfo::AllocInfo> &alloctype = varInfo->alloctype;
+
+    // Deallocation and then dereferencing pointer..
+    if (tok->varId() > 0) {
+        const std::map<unsigned int, VarInfo::AllocInfo>::const_iterator var = alloctype.find(tok->varId());
+        if (var != alloctype.end()) {
+            bool unknown = false;
+            if (var->second.status == VarInfo::DEALLOC && CheckNullPointer::isPointerDeRef(tok, unknown) && !unknown) {
+                deallocUseError(tok, tok->str());
+            }
+            else if (Token::simpleMatch(tok->tokAt(-2), "= &")) {
+                varInfo->erase(tok->varId());
+            }
+            else if (tok->strAt(-1) == "=") {
+                varInfo->erase(tok->varId());
+            }
+        }
+        else if (Token::Match(tok->previous(), "& %name% = %var% ;")) {
+            varInfo->referenced.insert(tok->tokAt(2)->varId());
+        }
+    }
+
+    // check for function call
+    const Token * const openingPar = isFunctionCall(tok);
+    if (openingPar) {
+        const Library::AllocFunc* allocFunc = _settings->library.dealloc(tok);
+        VarInfo::AllocInfo alloc(allocFunc ? allocFunc->groupId : 0, VarInfo::DEALLOC);
+        if (alloc.type == 0)
+            alloc.status = VarInfo::NOALLOC;
+        functionCall(tok, openingPar, varInfo, alloc, nullptr);
+        return openingPar->link();
+    }
+
+    return nullptr;
+}
+
+
 void CheckLeakAutoVar::changeAllocStatus(VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Token* tok, const Token* arg)
 {
     std::map<unsigned int, VarInfo::AllocInfo> &alloctype = varInfo->alloctype;
@@ -671,21 +731,27 @@ void CheckLeakAutoVar::changeAllocStatus(VarInfo *varInfo, const VarInfo::AllocI
     }
 }
 
-void CheckLeakAutoVar::functionCall(const Token *tok, VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Library::AllocFunc* af)
+void CheckLeakAutoVar::functionCall(const Token *tokName, const Token *tokOpeningPar, VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Library::AllocFunc* af)
 {
     // Ignore function call?
-    if (_settings->library.isLeakIgnore(tok->str()))
+    if (_settings->library.isLeakIgnore(tokName->str()))
         return;
 
+    const Token * const tokFirstArg = tokOpeningPar->next();
+    if (!tokFirstArg || tokFirstArg->str() == ")") {
+        // no arguments
+        return;
+    }
+
     int argNr = 1;
-    for (const Token *arg = tok->tokAt(2); arg; arg = arg->nextArgument()) {
+    for (const Token *arg = tokFirstArg; arg; arg = arg->nextArgument()) {
         if (_tokenizer->isCPP() && arg->str() == "new") {
             arg = arg->next();
             if (Token::simpleMatch(arg, "( std :: nothrow )"))
                 arg = arg->tokAt(5);
         }
 
-        while (Token::Match(arg, "%var% . %var%"))
+        while (Token::Match(arg, "%name% .|:: %name%"))
             arg = arg->tokAt(2);
 
         if (Token::Match(arg, "%var% [-,)] !!.") || Token::Match(arg, "& %var%")) {
@@ -697,14 +763,57 @@ void CheckLeakAutoVar::functionCall(const Token *tok, VarInfo *varInfo, const Va
 
             // Is variable allocated?
             if (!isnull && (!af || af->arg == argNr))
-                changeAllocStatus(varInfo, allocation, tok, arg);
+                changeAllocStatus(varInfo, allocation, tokName, arg);
-        } else if (Token::Match(arg, "%name% (")) {
-            const Library::AllocFunc* allocFunc = _settings->library.dealloc(arg);
-            VarInfo::AllocInfo alloc(allocFunc ? allocFunc->groupId : 0, VarInfo::DEALLOC);
-            if (alloc.type == 0)
-                alloc.status = VarInfo::NOALLOC;
-            functionCall(arg, varInfo, alloc, allocFunc);
         }
+        // Check smart pointer
+        else if (Token::Match(arg, "auto_ptr|unique_ptr|shared_ptr < %type%")) {
+            const Token * typeEndTok = arg->linkAt(1);
+            if (!Token::Match(typeEndTok, "> {|( %var% ,|)|}"))
+                continue;
+
+            bool arrayDelete = false;
+            if (Token::findsimplematch(arg->next(), "[ ]", typeEndTok))
+                arrayDelete = true;
+
+            // Check deleter
+            const Token * deleterToken = nullptr;
+            const Token * endDeleterToken = nullptr;
+            const Library::AllocFunc* sp_af = nullptr;
+            if (Token::Match(arg, "unique_ptr < %type% ,")) {
+                deleterToken = arg->tokAt(4);
+                endDeleterToken = typeEndTok;
+            }
+            else if (Token::Match(typeEndTok, "> {|( %var% ,")) {
+                deleterToken = typeEndTok->tokAt(4);
+                endDeleterToken = typeEndTok->linkAt(1);
+            }
+            if (deleterToken) {
+                // Check if its a pointer to a function
+                const Token * dtok = Token::findmatch(deleterToken, "& %name%", endDeleterToken);
+                if (dtok) {
+                    sp_af = _settings->library.dealloc(dtok->tokAt(1));
+                }
+                else {
+                    // If the deleter is a class, check if class calls the dealloc function
+                    dtok = Token::findmatch(deleterToken, "%type%", endDeleterToken);
+                    if (dtok && dtok->type()) {
+                        const Scope * tscope = dtok->type()->classScope;
+                        for (const Token *tok2 = tscope->bodyStart; tok2 != tscope->bodyEnd; tok2 = tok2->next()) {
+                            sp_af = _settings->library.dealloc(tok2);
+                            if (sp_af)
+                                break;
+                        }
+                    }
+                }
+            }
+
+            const Token * vtok = typeEndTok->tokAt(2);
+            const VarInfo::AllocInfo sp_allocation(sp_af ? sp_af->groupId : (arrayDelete ? NEW_ARRAY : NEW), VarInfo::OWNED);
+            changeAllocStatus(varInfo, sp_allocation, vtok, vtok);
+        } else {
+            checkTokenInsideExpression(arg, varInfo);
+        }
+        // TODO: check each token in argument expression (could contain multiple variables)
         argNr++;
     }
 }

lib/checkleakautovar.h

@@ -119,8 +119,15 @@ private:
                     VarInfo *varInfo,
                     std::set<unsigned int> notzero);
 
+    /** Check token inside expression.
+    * @param tok token inside expression.
+    * @param varInfo Variable info
+    * @return next token to process (if no other checks needed for this token). NULL if other checks could be performed.
+    */
+    const Token * checkTokenInsideExpression(const Token * const tok, VarInfo *varInfo);
+
     /** parse function call */
-    void functionCall(const Token *tok, VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Library::AllocFunc* af);
+    void functionCall(const Token *tokName, const Token *tokOpeningPar, VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Library::AllocFunc* af);
 
     /** parse changes in allocation status */
     void changeAllocStatus(VarInfo *varInfo, const VarInfo::AllocInfo& allocation, const Token* tok, const Token* arg);

test/testleakautovar.cpp

@@ -1570,7 +1570,7 @@ private:
               "}\n",
               true
              );
-        TODO_ASSERT_EQUALS("", "[test.cpp:5]: (error) Memory leak: pt\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
     }
 
 };