diff --git a/cfg/boost.cfg b/cfg/boost.cfg index 6e30272f1..9021fb541 100644 --- a/cfg/boost.cfg +++ b/cfg/boost.cfg @@ -81,6 +81,7 @@ + diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index b4a3878bc..8a4d04f77 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -416,6 +416,9 @@ + + + diff --git a/cfg/std.cfg b/cfg/std.cfg index 8f05e1822..ea71c7ba4 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8305,6 +8305,18 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init + + + + + + + + + + + + diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 52e4839d5..4cc558c56 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -249,9 +249,18 @@ bool astIsIterator(const Token *tok) return tok && tok->valueType() && tok->valueType()->type == ValueType::Type::ITERATOR; } -bool astIsContainer(const Token *tok) +bool astIsContainer(const Token* tok) { + return getLibraryContainer(tok) != nullptr && !astIsIterator(tok); +} + +bool astIsContainerView(const Token* tok) { - return getLibraryContainer(tok) != nullptr && tok->valueType()->type != ValueType::Type::ITERATOR; + const Library::Container* container = getLibraryContainer(tok); + return container && !astIsIterator(tok) && container->view; +} + +bool astIsContainerOwned(const Token* tok) { + return astIsContainer(tok) && !astIsContainerView(tok); } std::string astCanonicalType(const Token *expr) diff --git a/lib/astutils.h b/lib/astutils.h index ee1b0d874..ea0921c84 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -85,6 +85,9 @@ bool astIsIterator(const Token *tok); bool astIsContainer(const Token *tok); +bool astIsContainerView(const Token* tok); +bool astIsContainerOwned(const Token* tok); + /** * Get canonical type of expression. const/static/etc are not included and neither *&. * For example: diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 075cc961f..3c42e7765 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -558,14 +558,13 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token break; } } - } + const bool escape = Token::Match(tok->astParent(), "return|throw"); for (const ValueFlow::Value& val:tok->values()) { if (!val.isLocalLifetimeValue() && !val.isSubFunctionLifetimeValue()) continue; if (!printInconclusive && val.isInconclusive()) continue; - const bool escape = Token::Match(tok->astParent(), "return|throw"); for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(val.tokvalue), escape || isAssignedToNonLocal(tok))) { const Token * tokvalue = lt.token; @@ -575,7 +574,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token continue; if (!isLifetimeBorrowed(tok, mSettings)) continue; - if (tokvalue->exprId() == tok->exprId() && !(tok->variable() && tok->variable()->isArray())) + if (tokvalue->exprId() == tok->exprId() && !(tok->variable() && tok->variable()->isArray()) && + !astIsContainerView(tok->astParent())) continue; if ((tokvalue->variable() && !isEscapedReference(tokvalue->variable()) && isInScope(tokvalue->variable()->nameToken(), scope)) || diff --git a/lib/library.cpp b/lib/library.cpp index 827a8c6a8..efbcad8d8 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -439,6 +439,9 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) const char* const hasInitializerListConstructor = node->Attribute("hasInitializerListConstructor"); if (hasInitializerListConstructor) container.hasInitializerListConstructor = std::string(hasInitializerListConstructor) == "true"; + const char* const view = node->Attribute("view"); + if (view) + container.view = std::string(view) == "true"; for (const tinyxml2::XMLElement *containerNode = node->FirstChildElement(); containerNode; containerNode = containerNode->NextSiblingElement()) { const std::string containerNodeName = containerNode->Name(); diff --git a/lib/library.h b/lib/library.h index e929f6e3e..1eddc8fdd 100644 --- a/lib/library.h +++ b/lib/library.h @@ -204,8 +204,8 @@ public: class Container { public: - Container() : - type_templateArgNo(-1), + Container() + : type_templateArgNo(-1), size_templateArgNo(-1), arrayLike_indexOp(false), stdStringLike(false), @@ -213,14 +213,33 @@ public: opLessAllowed(true), hasInitializerListConstructor(false), unstableErase(false), - unstableInsert(false) {} + unstableInsert(false), + view(false) + {} enum class Action { - RESIZE, CLEAR, PUSH, POP, FIND, INSERT, ERASE, CHANGE_CONTENT, CHANGE, CHANGE_INTERNAL, + RESIZE, + CLEAR, + PUSH, + POP, + FIND, + INSERT, + ERASE, + CHANGE_CONTENT, + CHANGE, + CHANGE_INTERNAL, NO_ACTION }; enum class Yield { - AT_INDEX, ITEM, BUFFER, BUFFER_NT, START_ITERATOR, END_ITERATOR, ITERATOR, SIZE, EMPTY, + AT_INDEX, + ITEM, + BUFFER, + BUFFER_NT, + START_ITERATOR, + END_ITERATOR, + ITERATOR, + SIZE, + EMPTY, NO_YIELD }; struct Function { @@ -238,6 +257,7 @@ public: bool hasInitializerListConstructor; bool unstableErase; bool unstableInsert; + bool view; Action getAction(const std::string& function) const { const std::map::const_iterator i = functions.find(function); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 10eee5abb..3e675fb2b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -3031,22 +3031,32 @@ static bool isNotLifetimeValue(const ValueFlow::Value& val) return !val.isLifetimeValue(); } +static bool isLifetimeOwned(const ValueType* vtParent) +{ + if (vtParent->container) + return !vtParent->container->view; + return vtParent->type == ValueType::CONTAINER; +} + static bool isLifetimeOwned(const ValueType *vt, const ValueType *vtParent) { if (!vtParent) return false; if (!vt) { - if (vtParent->type == ValueType::CONTAINER) + if (isLifetimeOwned(vtParent)) return true; return false; } + // If converted from iterator to pointer then the iterator is most likely a pointer + if (vtParent->pointer == 1 && vt->pointer == 0 && vt->type == ValueType::ITERATOR) + return false; if (vt->type != ValueType::UNKNOWN_TYPE && vtParent->type != ValueType::UNKNOWN_TYPE) { if (vt->pointer != vtParent->pointer) return true; if (vt->type != vtParent->type) { if (vtParent->type == ValueType::RECORD) return true; - if (vtParent->type == ValueType::CONTAINER) + if (isLifetimeOwned(vtParent)) return true; } } @@ -3062,6 +3072,8 @@ static bool isLifetimeBorrowed(const ValueType *vt, const ValueType *vtParent) return false; if (vt->pointer > 0 && vt->pointer == vtParent->pointer) return true; + if (vtParent->container && vtParent->container->view) + return true; if (vt->type != ValueType::UNKNOWN_TYPE && vtParent->type != ValueType::UNKNOWN_TYPE && vtParent->container == vt->container) { if (vtParent->pointer > vt->pointer) return true; @@ -3146,6 +3158,42 @@ static bool isDifferentType(const Token* src, const Token* dst) return false; } +static std::vector getParentValueTypes(const Token* tok, const Settings* settings = nullptr) +{ + if (!tok) + return {}; + if (!tok->astParent()) + return {}; + if (Token::Match(tok->astParent(), "(|{|,")) { + int argn = -1; + const Token* ftok = getTokenArgumentFunction(tok, argn); + if (ftok && ftok->function()) { + std::vector result; + std::vector argsVars = getArgumentVars(ftok, argn); + for (const Variable* var : getArgumentVars(ftok, argn)) { + if (!var) + continue; + if (!var->valueType()) + continue; + result.push_back(*var->valueType()); + } + return result; + } + } + if (settings && Token::Match(tok->astParent()->tokAt(-2), ". push_back|push_front|insert|push (") && + astIsContainer(tok->astParent()->tokAt(-2)->astOperand1())) { + const Token* contTok = tok->astParent()->tokAt(-2)->astOperand1(); + const ValueType* vtCont = contTok->valueType(); + if (!vtCont->containerTypeToken) + return {}; + ValueType vtParent = ValueType::parseDecl(vtCont->containerTypeToken, settings); + return {std::move(vtParent)}; + } + if (tok->astParent()->valueType()) + return {*tok->astParent()->valueType()}; + return {}; +} + bool isLifetimeBorrowed(const Token *tok, const Settings *settings) { if (!tok) @@ -3605,6 +3653,13 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog } if (update) valueFlowForwardLifetime(tok->next(), tokenlist, errorLogger, settings); + } else if (tok->valueType()) { + // TODO: Propagate lifetimes with library functions + if (settings->library.getFunction(tok->previous())) + return; + // Assume constructing the valueType + valueFlowLifetimeConstructor(tok, tokenlist, errorLogger, settings); + valueFlowForwardLifetime(tok->next(), tokenlist, errorLogger, settings); } } @@ -3673,7 +3728,13 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error Token* parent = tok->astParent(); while (Token::simpleMatch(parent, ",")) parent = parent->astParent(); - if (Token::simpleMatch(parent, "{") && hasInitList(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); @@ -3764,6 +3825,16 @@ static bool isDecayedPointer(const Token *tok) return astIsPointer(tok->astParent()); } +static bool isConvertedToView(const Token* tok, const Settings* settings) +{ + std::vector vtParents = getParentValueTypes(tok, settings); + return std::any_of(vtParents.begin(), vtParents.end(), [&](const ValueType& vt) { + if (!vt.container) + return false; + return vt.container->view; + }); +} + static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger *errorLogger, const Settings *settings) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { @@ -3861,6 +3932,13 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); } } + // Converting to container view + else if (astIsContainerOwned(tok) && isConvertedToView(tok, settings)) { + LifetimeStore ls = + LifetimeStore{tok, "Converted to container view", ValueFlow::Value::LifetimeKind::SubObject}; + ls.byRef(tok, tokenlist, errorLogger, settings); + valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + } // container lifetimes else if (astIsContainer(tok)) { Token * parent = astParentSkipParens(tok); @@ -3902,6 +3980,16 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger continue; toks.push_back(v.tokvalue); } + } else if (astIsContainerView(tok)) { + for (const ValueFlow::Value& v : tok->values()) { + if (!v.isLifetimeValue()) + continue; + if (!v.tokvalue) + continue; + if (!astIsContainerOwned(v.tokvalue)) + continue; + toks.push_back(v.tokvalue); + } } else { toks = {tok}; } diff --git a/man/reference-cfg-format.md b/man/reference-cfg-format.md index 81b6bf6f6..bb486ffcf 100644 --- a/man/reference-cfg-format.md +++ b/man/reference-cfg-format.md @@ -580,6 +580,8 @@ A lot of C++ libraries, among those the STL itself, provide containers with very The `hasInitializerListConstructor` attribute can be set when the container has a constructor taking an initializer list. +The `view` attribute can be set when the container is a view, which means it borrows the lifetime of another container. + Inside the `` tag, functions can be defined inside of the tags ``, `` and `` (on your choice). Each of them can specify an action like "resize" and/or the result it yields, for example "end-iterator". The following example provides a definition for std::vector, based on the definition of "stdContainer" (not shown): diff --git a/releasenotes.txt b/releasenotes.txt index ac2e29e94..e8ea63db4 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -1 +1,3 @@ release notes for cppcheck-2.7 + +Add support for container views. The `view` attribute has been added to the `` library tag to specify the class is a view. The lifetime analysis has been updated to use this new attribute to find dangling lifetime containers. diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index cad3a05ed..e1db3dab5 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -141,6 +141,7 @@ private: TEST_CASE(danglingLifetimeLambda); TEST_CASE(danglingLifetimeContainer); + TEST_CASE(danglingLifetimeContainerView); TEST_CASE(danglingLifetime); TEST_CASE(danglingLifetimeFunction); TEST_CASE(danglingLifetimeAggegrateConstructor); @@ -2444,6 +2445,102 @@ private: ASSERT_EQUALS("", errout.str()); } + void danglingLifetimeContainerView() + { + check("std::string_view f() {\n" + " std::string s = \"\";\n" + " return s;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n", + errout.str()); + + check("std::string_view f() {\n" + " std::string s = \"\";\n" + " std::string_view sv = s;\n" + " return sv;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n", + errout.str()); + + check("std::string_view f() {\n" + " std::string s = \"\";\n" + " return std::string_view{s};\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n", + errout.str()); + + check("std::string_view f(std::string_view s) {\n" + " return s;\n" + "}\n" + "std::string_view g() {\n" + " std::string s = \"\";\n" + " return f(s);\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:6] -> [test.cpp:6] -> [test.cpp:5] -> [test.cpp:6]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n", + errout.str()); + + check("const char * f() {\n" + " std::string s;\n" + " std::string_view sv = s;\n" + " return sv.begin();\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 's' that will be invalid when returning.\n", + errout.str()); + + check("const char * f() {\n" + " std::string s;\n" + " return std::string_view{s}.begin();\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning iterator to local container 's' that will be invalid when returning.\n", + errout.str()); + + check("const char * f() {\n" + " std::string s;\n" + " return std::string_view(s).begin();\n" + "}\n"); + TODO_ASSERT_EQUALS( + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning iterator to local container 's' that will be invalid when returning.\n", + "", + errout.str()); + + check("const char * f(std::string_view sv) {\n" + " return sv.begin();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("const char * f(std::string s) {\n" + " std::string_view sv = s;\n" + " return sv.begin();\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:3] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning iterator to local container 's' that will be invalid when returning.\n", + errout.str()); + + check("std::string_view f(std::string s) {\n" + " return s;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:2] -> [test.cpp:1] -> [test.cpp:2]: (error) Returning object that points to local variable 's' that will be invalid when returning.\n", + errout.str()); + + check("const char * f(const std::string& s) {\n" + " std::string_view sv = s;\n" + " return sv.begin();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("std::string_view f(const std::string_view& sv) {\n" + " return sv;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void danglingLifetime() { check("auto f() {\n" " std::vector a;\n"