Merge pull request #2774 from pfultz2/lifetime-subfunction

Extend lifetimes to subfunctions

lib/checkautovariables.cpp

@@ -473,6 +473,25 @@ static bool isEscapedReference(const Variable* var)
     return !isTemporary(true, vartok, nullptr, false);
 }
 
+static bool isDanglingSubFunction(const Token* tokvalue, const Token* tok)
+{
+    if (!tokvalue)
+        return false;
+    const Variable* var = tokvalue->variable();
+    if (!var->isLocal())
+        return false;
+    Function* f = Scope::nestedInFunction(tok->scope());
+    if (!f)
+        return false;
+    const Token* parent = tokvalue->astParent();
+    while (parent && !Token::Match(parent->previous(), "%name% (")) {
+        parent = parent->astParent();
+    }
+    if (!Token::simpleMatch(parent, "("))
+        return false;
+    return exprDependsOnThis(parent);
+}
+
 void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end)
 {
     if (!start)
@@ -521,11 +540,12 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
 
         }
         for (const ValueFlow::Value& val:tok->values()) {
-            if (!val.isLocalLifetimeValue())
+            if (!val.isLocalLifetimeValue() && !val.isSubFunctionLifetimeValue())
                 continue;
             const bool escape = Token::Match(tok->astParent(), "return|throw");
             for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(val.tokvalue), escape)) {
                 const Token * tokvalue = lt.token;
+                if (val.isLocalLifetimeValue()) {
                     if (escape) {
                         if (getPointerDepth(tok) < getPointerDepth(tokvalue))
                             continue;
@@ -544,7 +564,10 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
                                isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) {
                         errorDanglingTemporaryLifetime(tok, &val);
                         break;
-                } else if (tokvalue->variable() && isInScope(tokvalue->variable()->nameToken(), tok->scope())) {
+                    }
+                }
+                if (tokvalue->variable() && (isInScope(tokvalue->variable()->nameToken(), tok->scope()) ||
+                                             (val.isSubFunctionLifetimeValue() && isDanglingSubFunction(tokvalue, tok)))) {
                     const Variable * var = nullptr;
                     const Token * tok2 = tok;
                     if (Token::simpleMatch(tok->astParent(), "=")) {

lib/symboldatabase.h

@@ -1049,6 +1049,18 @@ public:
         return false;
     }
 
+    static Function* nestedInFunction(const Scope* scope)
+    {
+        while (scope) {
+            if (scope->type == Scope::eFunction)
+                break;
+            scope = scope->nestedIn;
+        }
+        if (!scope)
+            return nullptr;
+        return scope->function;
+    }
+
     bool isClassOrStruct() const {
         return (type == eClass || type == eStruct);
     }

lib/valueflow.cpp

@@ -3928,6 +3928,10 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
                 valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
             }
         }
+        // Forward any lifetimes
+        else if (std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) {
+            valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
+        }
     }
 }
 
@@ -5491,8 +5495,6 @@ static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldat
                 // passing value(s) to function
                 std::list<ValueFlow::Value> argvalues(getFunctionArgumentValues(argtok));
 
-                // Don't forward lifetime values
-                argvalues.remove_if(std::mem_fn(&ValueFlow::Value::isLifetimeValue));
                 // Don't forward container sizes for now since programmemory can't evaluate conditions
                 argvalues.remove_if(std::mem_fn(&ValueFlow::Value::isContainerSizeValue));
 
@@ -5513,6 +5515,9 @@ static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldat
                                              "' value is " +
                                              v.infoString());
                     v.path = 256 * v.path + id;
+                    // Change scope of lifetime values
+                    if (v.isLifetimeValue())
+                        v.lifetimeScope = ValueFlow::Value::LifetimeScope::SubFunction;
                 }
 
                 // passed values are not "known"..
@@ -6709,6 +6714,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
         valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowSubFunction(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowFunctionReturn(tokenlist, errorLogger);
+        valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings);
         if (tokenlist->isCPP()) {

lib/valueflow.h

@@ -217,6 +217,11 @@ namespace ValueFlow {
             return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Argument;
         }
 
+        bool isSubFunctionLifetimeValue() const
+        {
+            return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::SubFunction;
+        }
+
         bool isNonValue() const {
             return isMovedValue() || isUninitValue() || isLifetimeValue();
         }
@@ -263,7 +268,7 @@ namespace ValueFlow {
 
         enum class LifetimeKind {Object, SubObject, Lambda, Iterator, Address} lifetimeKind;
 
-        enum class LifetimeScope { Local, Argument } lifetimeScope;
+        enum class LifetimeScope { Local, Argument, SubFunction } lifetimeScope;
 
         static const char* toString(MoveKind moveKind);
 

test/testautovariables.cpp

@@ -2222,6 +2222,46 @@ private:
               "        std::sort(x.begin(), x.end());\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("struct A {\n"
+              "    std::vector<int*> v;\n"
+              "    void add(int* i) {\n"
+              "        v.push_back(i);\n"
+              "    }\n"
+              "    void f() {\n"
+              "        int i = 0;\n"
+              "        add(&i);\n"
+              "    }\n"
+              "};\n");
+        ASSERT_EQUALS(
+            "[test.cpp:8] -> [test.cpp:8] -> [test.cpp:4] -> [test.cpp:7] -> [test.cpp:4]: (error) Non-local variable 'v' will use object that points to local variable 'i'.\n",
+            errout.str());
+
+        check("struct A {\n"
+              "    std::vector<int*> v;\n"
+              "    void add(int* i) {\n"
+              "        v.push_back(i);\n"
+              "    }\n"
+              "};\n"
+              "void f() {\n"
+              "    A a;\n"
+              "    int i = 0;\n"
+              "    a.add(&i);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("struct A {\n"
+              "    std::vector<int*> v;\n"
+              "    void add(int* i) {\n"
+              "        v.push_back(i);\n"
+              "    }\n"
+              "    void f() {\n"
+              "        A a;\n"
+              "        int i = 0;\n"
+              "        a.add(&i);\n"
+              "    }\n"
+              "};\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void danglingLifetime() {