From 5afd6880c30bd38021175a159c805b5524193740 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Wed, 4 May 2022 23:54:36 -0500 Subject: [PATCH] Fix 11028: False positive: invalidContainer (#4083) * Fix 11028: False positive: invalidContainer * Format --- lib/checkstl.cpp | 2 + lib/valueflow.cpp | 101 +++++++++++++++++++++------------------------- test/teststl.cpp | 9 +++++ 3 files changed, 58 insertions(+), 54 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 1abd04b40..a5ad67d43 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1051,6 +1051,8 @@ static const ValueFlow::Value* getInnerLifetime(const Token* tok, ValueFlow::Value::LifetimeKind::SubObject, ValueFlow::Value::LifetimeKind::Lambda}, val.lifetimeKind)) { + if (val.isInconclusive()) + return nullptr; if (val.capturetok) return getInnerLifetime(val.capturetok, id, errorPath, depth - 1); if (errorPath) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 70a7e8b44..5ce932dc6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -3879,13 +3879,13 @@ private: } }; -static void valueFlowLifetimeConstructor(Token* tok, - const Function* constructor, - const std::string& name, - std::vector args, - TokenList* tokenlist, - ErrorLogger* errorLogger, - const Settings* settings) +static void valueFlowLifetimeUserConstructor(Token* tok, + const Function* constructor, + const std::string& name, + std::vector args, + TokenList* tokenlist, + ErrorLogger* errorLogger, + const Settings* settings) { if (!constructor) return; @@ -4026,7 +4026,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog } else if (tok->function()) { const Function *f = tok->function(); if (f->isConstructor()) { - valueFlowLifetimeConstructor(tok->next(), f, tok->str(), getArguments(tok), tokenlist, errorLogger, settings); + valueFlowLifetimeUserConstructor(tok->next(), f, tok->str(), getArguments(tok), tokenlist, errorLogger, settings); return; } if (Function::returnsReference(f)) @@ -4134,11 +4134,11 @@ static const Function* findConstructor(const Scope* scope, const Token* tok, con return f; } -static void valueFlowLifetimeConstructor(Token* tok, - const Type* t, - TokenList* tokenlist, - ErrorLogger* errorLogger, - const Settings* settings) +static void valueFlowLifetimeClassConstructor(Token* tok, + const Type* t, + TokenList* tokenlist, + ErrorLogger* errorLogger, + const Settings* settings) { if (!Token::Match(tok, "(|{")) return; @@ -4185,59 +4185,52 @@ static void valueFlowLifetimeConstructor(Token* tok, }); } else { const Function* constructor = findConstructor(scope, tok, args); - valueFlowLifetimeConstructor(tok, constructor, t->name(), args, tokenlist, errorLogger, settings); + valueFlowLifetimeUserConstructor(tok, constructor, t->name(), args, tokenlist, errorLogger, settings); } } } -static bool hasInitList(const Token* tok) -{ - if (astIsPointer(tok)) - return true; - if (astIsContainer(tok)) { - const Library::Container * library = getLibraryContainer(tok); - if (!library) - return false; - return library->hasInitializerListConstructor; - } - return false; -} - static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings) { if (!Token::Match(tok, "(|{")) return; if (isScope(tok)) return; - Token* parent = tok->astParent(); - while (Token::simpleMatch(parent, ",")) - parent = parent->astParent(); - if (Token::Match(tok, "{|(") && astIsContainerView(tok) && !tok->function()) { - std::vector args = getArguments(tok); - if (args.size() == 1 && astIsContainerOwned(args.front())) { - LifetimeStore{args.front(), "Passed to container view.", ValueFlow::Value::LifetimeKind::SubObject}.byRef( - tok, tokenlist, errorLogger, settings); - } - } else if (Token::simpleMatch(parent, "{") && hasInitList(parent->astParent())) { - valueFlowLifetimeConstructor(tok, Token::typeOf(parent->previous()), tokenlist, errorLogger, settings); - } else if (Token::simpleMatch(tok, "{") && hasInitList(parent)) { - std::vector args = getArguments(tok); - // Assume range constructor if passed a pair of iterators - if (astIsContainer(parent) && args.size() == 2 && astIsIterator(args[0]) && astIsIterator(args[1])) { - LifetimeStore::forEach( - args, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::SubObject, [&](const LifetimeStore& ls) { - ls.byDerefCopy(tok, tokenlist, errorLogger, settings); - }); + std::vector vts; + if (tok->valueType()) { + vts = {*tok->valueType()}; + } else if (Token::Match(tok->previous(), "%var% {|(") && isVariableDecl(tok->previous()) && + tok->previous()->valueType()) { + vts = {*tok->previous()->valueType()}; + } else if (Token::simpleMatch(tok, "{") && !Token::Match(tok->previous(), "%name%")) { + vts = getParentValueTypes(tok, settings); + } + + for (const ValueType& vt : vts) { + if (vt.container && vt.type == ValueType::CONTAINER) { + std::vector args = getArguments(tok); + if (args.size() == 1 && vt.container->view && astIsContainerOwned(args.front())) { + LifetimeStore{args.front(), "Passed to container view.", ValueFlow::Value::LifetimeKind::SubObject} + .byRef(tok, tokenlist, errorLogger, settings); + } else if (args.size() == 2 && astIsIterator(args[0]) && astIsIterator(args[1])) { + LifetimeStore::forEach( + args, + "Passed to initializer list.", + ValueFlow::Value::LifetimeKind::SubObject, + [&](const LifetimeStore& ls) { + ls.byDerefCopy(tok, tokenlist, errorLogger, settings); + }); + } else if (vt.container->hasInitializerListConstructor) { + LifetimeStore::forEach(args, + "Passed to initializer list.", + ValueFlow::Value::LifetimeKind::SubObject, + [&](const LifetimeStore& ls) { + ls.byVal(tok, tokenlist, errorLogger, settings); + }); + } } else { - LifetimeStore::forEach(args, - "Passed to initializer list.", - ValueFlow::Value::LifetimeKind::SubObject, - [&](const LifetimeStore& ls) { - ls.byVal(tok, tokenlist, errorLogger, settings); - }); + valueFlowLifetimeClassConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings); } - } else { - valueFlowLifetimeConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings); } } diff --git a/test/teststl.cpp b/test/teststl.cpp index c08bd2bdc..3280ad3d9 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -5336,6 +5336,15 @@ private: ASSERT_EQUALS( "[test.cpp:6] -> [test.cpp:7] -> [test.cpp:8] -> [test.cpp:5] -> [test.cpp:9]: (error) Using object that points to local variable 'v' that may be invalid.\n", errout.str()); + + // #11028 + check("void f(std::vector c) {\n" + " std::vector d(c.begin(), c.end());\n" + " c.erase(c.begin());\n" + " d.push_back(0);\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); } void invalidContainerLoop() {