diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 22d9cfd0e..bb76132fe 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -517,6 +517,53 @@ static bool isAssignedToNonLocal(const Token* tok) return !var->isLocal() || var->isStatic(); } +static std::vector getParentMembers(const Token* tok) +{ + if (!tok) + return {}; + if (!Token::simpleMatch(tok->astParent(), ".")) + return {tok}; + const Token* parent = tok; + while (Token::simpleMatch(parent->astParent(), ".")) + parent = parent->astParent(); + std::vector result; + for (const Token* tok2 : astFlatten(parent, ".")) { + if (Token::simpleMatch(tok2, "(") && Token::simpleMatch(tok2->astOperand1(), ".")) { + std::vector sub = getParentMembers(tok2->astOperand1()); + result.insert(result.end(), sub.begin(), sub.end()); + } + result.push_back(tok2); + } + return result; +} + +static const Token* getParentLifetime(bool cpp, const Token* tok, const Library* library) +{ + std::vector members = getParentMembers(tok); + if (members.size() < 2) + return tok; + // Find the first local variable or temporary + auto it = std::find_if(members.rbegin(), members.rend(), [&](const Token* tok2) { + const Variable* var = tok2->variable(); + if (var) { + return var->isLocal() || var->isArgument(); + } else { + return isTemporary(cpp, tok2, library); + } + }); + if (it == members.rend()) + return tok; + // If any of the submembers are borrowed types then stop + if (std::any_of(it.base() - 1, members.end() - 1, [&](const Token* tok2) { + if (astIsPointer(tok2) || astIsContainerView(tok2) || astIsIterator(tok2)) + return true; + const Variable* var = tok2->variable(); + return var && var->isReference(); + })) + return nullptr; + return *it; +} + void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end) { const bool printInconclusive = mSettings->certainty.isEnabled(Certainty::inconclusive); @@ -569,13 +616,16 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token } } const bool escape = Token::Match(tok->astParent(), "return|throw"); + std::unordered_set exprs; for (const ValueFlow::Value& val:tok->values()) { if (!val.isLocalLifetimeValue() && !val.isSubFunctionLifetimeValue()) continue; if (!printInconclusive && val.isInconclusive()) continue; - for (const LifetimeToken& lt : - getLifetimeTokens(getParentLifetime(val.tokvalue), escape || isAssignedToNonLocal(tok))) { + const Token* parent = getParentLifetime(mTokenizer->isCPP(), val.tokvalue, &mSettings->library); + if (!exprs.insert(parent).second) + continue; + for (const LifetimeToken& lt : getLifetimeTokens(parent, escape || isAssignedToNonLocal(tok))) { const Token * tokvalue = lt.token; if (val.isLocalLifetimeValue()) { if (escape) { diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 49827d219..149ae0d1a 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -150,6 +150,7 @@ private: TEST_CASE(danglingLifetimeInitList); TEST_CASE(danglingLifetimeImplicitConversion); TEST_CASE(danglingTemporaryLifetime); + TEST_CASE(danglingLifetimeBorrowedMembers); TEST_CASE(invalidLifetime); TEST_CASE(deadPointer); TEST_CASE(splitNamespaceAuto); // crash #10473 @@ -3195,11 +3196,68 @@ private: " const std::map::iterator& m = func().a_.m_.begin();\n" " (void)m->first;\n" "}\n"); - ASSERT_EQUALS( - "[test.cpp:9] -> [test.cpp:9] -> [test.cpp:10]: (error) Using object that points to member variable 'm_' that is a temporary.\n", - errout.str()); + ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:9] -> [test.cpp:10]: (error) Using iterator that is a temporary.\n", + errout.str()); } + void danglingLifetimeBorrowedMembers() + { + // #10585 + check("struct Info { int k; };\n" + "struct MoreInfo {\n" + " int* k;\n" + " char dat;\n" + "};\n" + "struct Fields {\n" + " Info info;\n" + "};\n" + "template void func1(T val){}\n" + "template void func2(T val){}\n" + "Fields* get();\n" + "void doit() {\n" + " MoreInfo rech;\n" + " rech.k = &get()->info.k;\n" + " func1(&rech.dat);\n" + " func2(rech.k);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int x; };\n" + "A* g();\n" + "void f() {\n" + " A** ap = &g();\n" + " (*ap)->x;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:4] -> [test.cpp:5]: (error) Using pointer that is a temporary.\n", + errout.str()); + + check("struct A { int* x; };\n" + "A g();\n" + "void f() {\n" + " int* x = g().x;\n" + " (void)*x + 1;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int x; };\n" + "struct B { A* a; }\n" + "B g();\n" + "void f() {\n" + " int* x = &g()->a.x;\n" + " (void)*x + 1;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int x; };\n" + "struct B { A* g(); };\n" + "A* g();\n" + "void f(B b) {\n" + " A** ap = &b.g();\n" + " (*ap)->x;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:5] -> [test.cpp:6]: (error) Using pointer that is a temporary.\n", + errout.str()); + } void invalidLifetime() { check("void foo(int a) {\n" " std::function f;\n"