Fix 11057: FP danglingTemporaryLifetime with reference member (#4103)

* Fix 11057: FP danglingTemporaryLifetime with reference member * Add test * Format * Use ast for number of arguments * Get number of arguments using ast * Skip aggregate constructor when there are too many arguments * Format
2022-05-12 23:51:07 -05:00 · 2022-05-12 23:51:07 -05:00 · fbba72ab5a
parent 5f9bee9b91
commit fbba72ab5a
8 changed files with 129 additions and 32 deletions
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@ -147,6 +147,17 @@ std::vector<Token*> astFlatten(Token* tok, const char* op)
    return result;
 }

+nonneg int astCount(const Token* tok, const char* op, int depth)
+{
+    --depth;
+    if (!tok || depth < 0)
+        return 0;
+    if (tok->str() == op)
+        return astCount(tok->astOperand1(), op, depth) + astCount(tok->astOperand2(), op, depth);
+    else
+        return 1;
+}
+
 bool astHasToken(const Token* root, const Token * tok)
 {
    if (!root)
@ -2583,7 +2594,24 @@ bool isExpressionChanged(const Token* expr, const Token* start, const Token* end
    return result;
 }

-int numberOfArguments(const Token *start)
+const Token* getArgumentStart(const Token* ftok)
+{
+    const Token* tok = ftok;
+    if (Token::Match(tok, "%name% (|{"))
+        tok = ftok->next();
+    if (!Token::Match(tok, "(|{|["))
+        return nullptr;
+    const Token* startTok = tok->astOperand2();
+    if (!startTok && tok->next() != tok->link())
+        startTok = tok->astOperand1();
+    return startTok;
+}
+
+int numberOfArguments(const Token* ftok) {
+    return astCount(getArgumentStart(ftok), ",");
+}
+
+int numberOfArgumentsWithoutAst(const Token* start)
 {
    int arguments=0;
    const Token* const openBracket = start->next();
@ -2597,17 +2625,8 @@ int numberOfArguments(const Token *start)
    return arguments;
 }

-std::vector<const Token *> getArguments(const Token *ftok)
-{
-    const Token* tok = ftok;
-    if (Token::Match(tok, "%name% (|{"))
-        tok = ftok->next();
-    if (!Token::Match(tok, "(|{|["))
-        return std::vector<const Token *> {};
-    const Token *startTok = tok->astOperand2();
-    if (!startTok && tok->next() != tok->link())
-        startTok = tok->astOperand1();
-    return astFlatten(startTok, ",");
+std::vector<const Token*> getArguments(const Token* ftok) {
+    return astFlatten(getArgumentStart(ftok), ",");
 }

 int getArgumentPos(const Variable* var, const Function* f)
--- a/lib/astutils.h
+++ b/lib/astutils.h
@ -89,6 +89,8 @@ const Token* findExpression(const Token* start, const nonneg int exprid);
 std::vector<const Token*> astFlatten(const Token* tok, const char* op);
 std::vector<Token*> astFlatten(Token* tok, const char* op);

+nonneg int astCount(const Token* tok, const char* op, int depth = 100);
+
 bool astHasToken(const Token* root, const Token * tok);

 bool astHasVar(const Token * tok, nonneg int varid);
@ -310,11 +312,16 @@ bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr)

 bool isAliased(const Variable *var);

+const Token* getArgumentStart(const Token* ftok);
+
 /** Determines the number of arguments - if token is a function call or macro
 * @param start token which is supposed to be the function/macro name.
 * \return Number of arguments
 */
-int numberOfArguments(const Token *start);
+int numberOfArguments(const Token* ftok);
+
+/// Get number of arguments without using AST
+int numberOfArgumentsWithoutAst(const Token* start);

 /**
 * Get arguments (AST)
--- a/lib/library.cpp
+++ b/lib/library.cpp
@ -1197,7 +1197,7 @@ bool Library::isNotLibraryFunction(const Token *ftok) const

 bool Library::matchArguments(const Token *ftok, const std::string &functionName) const
 {
-    const int callargs = numberOfArguments(ftok);
+    const int callargs = numberOfArgumentsWithoutAst(ftok);
    const std::unordered_map<std::string, Function>::const_iterator it = functions.find(functionName);
    if (it == functions.cend())
        return (callargs == 0);
--- a/lib/symboldatabase.cpp
+++ b/lib/symboldatabase.cpp
@ -7172,6 +7172,14 @@ MathLib::bigint ValueType::typeSize(const cppcheck::Platform &platform, bool p)
    return 0;
 }

+bool ValueType::isTypeEqual(const ValueType* that) const
+{
+    auto tie = [](const ValueType* vt) {
+        return std::tie(vt->type, vt->container, vt->pointer, vt->typeScope, vt->smartPointer);
+    };
+    return tie(this) == tie(that);
+}
+
 std::string ValueType::str() const
 {
    std::string ret;
--- a/lib/symboldatabase.h
+++ b/lib/symboldatabase.h
@ -1334,6 +1334,9 @@ public:

    MathLib::bigint typeSize(const cppcheck::Platform &platform, bool p=false) const;

+    /// Check if type is the same ignoring const and references
+    bool isTypeEqual(const ValueType* that) const;
+
    std::string str() const;
    std::string dump() const;
 };
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@ -311,22 +311,37 @@ static std::vector<ValueType> getParentValueTypes(const Token* tok,
    } else if (Token::Match(tok->astParent(), "(|{|,")) {
        int argn = -1;
        const Token* ftok = getTokenArgumentFunction(tok, argn);
-        if (ftok && ftok->function()) {
-            std::vector<ValueType> result;
-            std::vector<const Variable*> argsVars = getArgumentVars(ftok, argn);
-            const Token* nameTok = nullptr;
-            for (const Variable* var : getArgumentVars(ftok, argn)) {
-                if (!var)
-                    continue;
-                if (!var->valueType())
-                    continue;
-                nameTok = var->nameToken();
-                result.push_back(*var->valueType());
+        const Token* typeTok = nullptr;
+        if (ftok && argn >= 0) {
+            if (ftok->function()) {
+                std::vector<ValueType> result;
+                std::vector<const Variable*> argsVars = getArgumentVars(ftok, argn);
+                const Token* nameTok = nullptr;
+                for (const Variable* var : getArgumentVars(ftok, argn)) {
+                    if (!var)
+                        continue;
+                    if (!var->valueType())
+                        continue;
+                    nameTok = var->nameToken();
+                    result.push_back(*var->valueType());
+                }
+                if (result.size() == 1 && nameTok && parent) {
+                    *parent = nameTok;
+                }
+                return result;
+            } else if (const Type* t = Token::typeOf(ftok, &typeTok)) {
+                if (astIsPointer(typeTok))
+                    return {*typeTok->valueType()};
+                const Scope* scope = t->classScope;
+                // Check for aggregate constructors
+                if (scope && scope->numConstructors == 0 && t->derivedFrom.empty() &&
+                    (t->isClassType() || t->isStructType()) && numberOfArguments(ftok) < scope->varlist.size()) {
+                    assert(argn < scope->varlist.size());
+                    auto it = std::next(scope->varlist.begin(), argn);
+                    if (it->valueType())
+                        return {*it->valueType()};
+                }
            }
-            if (result.size() == 1 && nameTok && parent) {
-                *parent = nameTok;
-            }
-            return result;
        }
    }
    if (settings && Token::Match(tok->astParent()->tokAt(-2), ". push_back|push_front|insert|push (") &&
@ -3293,6 +3308,14 @@ static std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
            return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1),
                                               !(astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "[")));
        }
+    } else if (Token::simpleMatch(tok, "{") && getArgumentStart(tok) &&
+               !Token::simpleMatch(getArgumentStart(tok), ",") && getArgumentStart(tok)->valueType()) {
+        const Token* vartok = getArgumentStart(tok);
+        auto vts = getParentValueTypes(tok);
+        for (const ValueType& vt : vts) {
+            if (vt.isTypeEqual(vartok->valueType()))
+                return getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1);
+        }
    }
    return {{tok, std::move(errorPath)}};
 }
@ -4216,7 +4239,15 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error
    }

    for (const ValueType& vt : vts) {
-        if (vt.container && vt.type == ValueType::CONTAINER) {
+        if (vt.pointer > 0) {
+            std::vector<const Token*> args = getArguments(tok);
+            LifetimeStore::forEach(args,
+                                   "Passed to initializer list.",
+                                   ValueFlow::Value::LifetimeKind::SubObject,
+                                   [&](const LifetimeStore& ls) {
+                ls.byVal(tok, tokenlist, errorLogger, settings);
+            });
+        } else if (vt.container && vt.type == ValueType::CONTAINER) {
            std::vector<const Token*> args = getArguments(tok);
            if (args.size() == 1 && vt.container->view && astIsContainerOwned(args.front())) {
                LifetimeStore{args.front(), "Passed to container view.", ValueFlow::Value::LifetimeKind::SubObject}
@ -4238,7 +4269,12 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error
                });
            }
        } else {
-            valueFlowLifetimeClassConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings);
+            const Type* t = nullptr;
+            if (vt.typeScope && vt.typeScope->definedType)
+                t = vt.typeScope->definedType;
+            else
+                t = Token::typeOf(tok->previous());
+            valueFlowLifetimeClassConstructor(tok, t, tokenlist, errorLogger, settings);
        }
    }
 }
@ -4544,7 +4580,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
            valueFlowForwardLifetime(parent->tokAt(2), tokenlist, errorLogger, settings);
        }
        // Check constructors
-        else if (Token::Match(tok, "=|return|%type%|%var% {") && !isScope(tok->next())) {
+        else if (Token::Match(tok, "=|return|%name%|{|,|> {") && !isScope(tok->next())) {
            valueFlowLifetimeConstructor(tok->next(), tokenlist, errorLogger, settings);
        }
        // Check function calls
--- a/test/testautovariables.cpp
+++ b/test/testautovariables.cpp
@ -3561,6 +3561,16 @@ private:
              "    }\n"
              "};\n");
        ASSERT_EQUALS("", errout.str());
+
+        // #11057
+        check("struct S {\n"
+              "    int& r;\n"
+              "};\n"
+              "void f(int i) {\n"
+              "    const S a[] = { { i } };\n"
+              "    for (const auto& s : a) {}\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
    }

    void danglingLifetimeBorrowedMembers()
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@ -6571,6 +6571,20 @@ private:
        code = "void f(const char * const x) { !!system(x); }\n";
        valueOfTok(code, "x");

+        code = "struct struct1 {\n"
+               "    int i1;\n"
+               "    int i2;\n"
+               "};\n"
+               "struct struct2 {\n"
+               "    char c1;\n"
+               "    struct1 is1;\n"
+               "    char c2[4];\n"
+               "};\n"
+               "void f() {\n"
+               "    struct2 a = { 1, 2, 3, {4,5,6,7} }; \n"
+               "}\n";
+        valueOfTok(code, "a");
+
        code = "void setDeltas(int life, int age, int multiplier) {\n"
               "    int dx = 0;\n"
               "    int dy = 0;\n"