From 2d5cabed4b10679ee449a59867ac449815490258 Mon Sep 17 00:00:00 2001 From: Mateusz Michalak Date: Thu, 9 Mar 2023 17:06:53 +0100 Subject: [PATCH] Add std::*begin and std::*end cfg (#4796) --- cfg/std.cfg | 16 ++++++++++++++ lib/astutils.cpp | 15 +++++++++++++ lib/astutils.h | 2 ++ lib/symboldatabase.cpp | 24 ++++++++++++++++++++- lib/tokenize.cpp | 4 +++- lib/valueflow.cpp | 35 ++++++++++++++++++++++++------- test/cfg/std.cpp | 43 ++++++++++++++++++++++++++++++++++++++ test/testautovariables.cpp | 7 +++++++ test/teststl.cpp | 32 +++++++++++++++++++++++----- 9 files changed, 163 insertions(+), 15 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index 9b7b03b9d..762c76b30 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8539,6 +8539,22 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init 0: + + + + false + + + + + + + + false + + + + malloc calloc diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 327ea5932..244eeb002 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -282,6 +282,7 @@ Library::Container::Action astContainerAction(const Token* tok, const Token** ft return Library::Container::Action::NO_ACTION; return tok->valueType()->container->getAction(ftok2->str()); } + Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok) { const Token* ftok2 = getContainerFunction(tok); @@ -292,6 +293,20 @@ Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok return tok->valueType()->container->getYield(ftok2->str()); } +Library::Container::Yield astFunctionYield(const Token* tok, const Settings* settings, const Token** ftok) +{ + if (!tok) + return Library::Container::Yield::NO_YIELD; + + const auto* function = settings->library.getFunction(tok); + if (!function) + return Library::Container::Yield::NO_YIELD; + + if (ftok) + *ftok = tok; + return function->containerYield; +} + bool astIsRangeBasedForDecl(const Token* tok) { return Token::simpleMatch(tok->astParent(), ":") && Token::simpleMatch(tok->astParent()->astParent(), "("); diff --git a/lib/astutils.h b/lib/astutils.h index e91599ec7..a82c2a7ec 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -153,6 +153,8 @@ bool astIsContainerOwned(const Token* tok); Library::Container::Action astContainerAction(const Token* tok, const Token** ftok = nullptr); Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok = nullptr); +Library::Container::Yield astFunctionYield(const Token* tok, const Settings* settings, const Token** ftok = nullptr); + /** Is given token a range-declaration in a range-based for loop */ bool astIsRangeBasedForDecl(const Token* tok); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 50c6b7521..5bcb0c6c5 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7029,6 +7029,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to } } + //Is iterator fetching function invoked on container? const bool isReturnIter = typestr == "iterator"; if (typestr.empty() || isReturnIter) { if (Token::simpleMatch(tok->astOperand1(), ".") && @@ -7037,7 +7038,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to tok->astOperand1()->astOperand1()->valueType() && tok->astOperand1()->astOperand1()->valueType()->container) { const Library::Container *cont = tok->astOperand1()->astOperand1()->valueType()->container; - const std::map::const_iterator it = cont->functions.find(tok->astOperand1()->astOperand2()->str()); + const auto it = cont->functions.find(tok->astOperand1()->astOperand2()->str()); if (it != cont->functions.end()) { if (it->second.yield == Library::Container::Yield::START_ITERATOR || it->second.yield == Library::Container::Yield::END_ITERATOR || @@ -7051,6 +7052,27 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to continue; } } + //Is iterator fetching function called? + } else if (Token::simpleMatch(tok->astOperand1(), "::") && + tok->astOperand2() && + tok->astOperand2()->isVariable()) { + const auto* const paramVariable = tok->astOperand2()->variable(); + if (!paramVariable || + !paramVariable->valueType() || + !paramVariable->valueType()->container) { + continue; + } + + const auto yield = astFunctionYield(tok->previous(), &mSettings); + if (yield == Library::Container::Yield::START_ITERATOR || + yield == Library::Container::Yield::END_ITERATOR || + yield == Library::Container::Yield::ITERATOR) { + ValueType vt; + vt.type = ValueType::Type::ITERATOR; + vt.container = paramVariable->valueType()->container; + vt.containerTypeToken = paramVariable->valueType()->containerTypeToken; + setValueType(tok, vt); + } } if (isReturnIter) { const std::vector args = getArguments(tok); diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 639cd6ebd..c69beb680 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -8898,7 +8898,9 @@ static const std::set stdFunctions = { "set_symmetric_difference", "push_heap", "pop_heap", "make_heap", "sort_heap", "min", "max", "min_element", "max_element", "lexicographical_compare", "next_permutation", "prev_permutation", "advance", "back_inserter", "distance", "front_inserter", "inserter", - "make_pair", "make_shared", "make_tuple" + "make_pair", "make_shared", "make_tuple", + "begin", "cbegin", "rbegin", "crbegin", + "end", "cend", "rend", "crend" }; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d3ef05007..09bc36ef8 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -663,7 +663,6 @@ static void setTokenValue(Token* tok, } } } - else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) && parent->astOperand1() && parent->astOperand1()->valueType()) { const Library::Container* c = getLibraryContainer(parent->astOperand1()); @@ -695,7 +694,6 @@ static void setTokenValue(Token* tok, } } } - return; } @@ -2203,7 +2201,7 @@ class SelectValueFromVarIdMapRange { using pointer = value_type *; using reference = value_type &; - explicit Iterator(const M::const_iterator &it) + explicit Iterator(const M::const_iterator & it) : mIt(it) {} reference operator*() const { @@ -4805,7 +4803,8 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro // container lifetimes else if (astIsContainer(tok)) { Token * parent = astParentSkipParens(tok); - if (!Token::Match(parent, ". %name% (")) + if (!Token::Match(parent, ". %name% (") && + !Token::simpleMatch(parent, "(")) continue; ValueFlow::Value master; @@ -4815,8 +4814,12 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro if (astIsIterator(parent->tokAt(2))) { master.errorPath.emplace_back(parent->tokAt(2), "Iterator to container is created here."); master.lifetimeKind = ValueFlow::Value::LifetimeKind::Iterator; - } else if ((astIsPointer(parent->tokAt(2)) && !isContainerOfPointers(tok->valueType()->containerTypeToken, settings)) || - Token::Match(parent->next(), "data|c_str")) { + } else if (astIsIterator(parent)) { + master.errorPath.emplace_back(parent, "Iterator to container is created here."); + master.lifetimeKind = ValueFlow::Value::LifetimeKind::Iterator; + } + else if ((astIsPointer(parent->tokAt(2)) && !isContainerOfPointers(tok->valueType()->containerTypeToken, settings)) || + Token::Match(parent->next(), "data|c_str")) { master.errorPath.emplace_back(parent->tokAt(2), "Pointer to container is created here."); master.lifetimeKind = ValueFlow::Value::LifetimeKind::Object; } else { @@ -4853,7 +4856,10 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro ValueFlow::Value value = master; value.tokvalue = rt.token; value.errorPath.insert(value.errorPath.begin(), rt.errors.cbegin(), rt.errors.cend()); - setTokenValue(parent->tokAt(2), std::move(value), settings); + if (Token::simpleMatch(parent, "(")) + setTokenValue(parent, value, settings); + else + setTokenValue(parent->tokAt(2), value, settings); if (!rt.token->variable()) { LifetimeStore ls = LifetimeStore{ @@ -8100,6 +8106,19 @@ static void valueFlowSmartPointer(TokenList *tokenlist, ErrorLogger * errorLogge } } +static Library::Container::Yield findIteratorYield(Token* tok, const Token** ftok, const Settings *settings) +{ + auto yield = astContainerYield(tok, ftok); + if (*ftok) + return yield; + + if (!tok->astParent()) + return yield; + + //begin/end free functions + return astFunctionYield(tok->astParent()->previous(), settings, ftok); +} + static void valueFlowIterators(TokenList *tokenlist, const Settings *settings) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { @@ -8110,7 +8129,7 @@ static void valueFlowIterators(TokenList *tokenlist, const Settings *settings) if (!astIsContainer(tok)) continue; const Token* ftok = nullptr; - const Library::Container::Yield yield = astContainerYield(tok, &ftok); + const Library::Container::Yield yield = findIteratorYield(tok, &ftok, settings); if (ftok) { ValueFlow::Value v(0); v.setKnown(); diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index bc7c3aee5..a8e8bd842 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -4661,4 +4661,47 @@ void stdspan() spn3.last<1>(); spn3.subspan<1, 1>(); #endif +} + +void beginEnd() +{ + std::vector v; + + //cppcheck-suppress ignoredReturnValue + std::begin(v); + //cppcheck-suppress ignoredReturnValue + std::rbegin(v); + //cppcheck-suppress ignoredReturnValue + std::cbegin(v); + //cppcheck-suppress ignoredReturnValue + std::crbegin(v); + + //cppcheck-suppress ignoredReturnValue + std::end(v); + //cppcheck-suppress ignoredReturnValue + std::rend(v); + //cppcheck-suppress ignoredReturnValue + std::cend(v); + //cppcheck-suppress ignoredReturnValue + std::crend(v); + + int arr[4]; + + //cppcheck-suppress ignoredReturnValue + std::begin(arr); + //cppcheck-suppress ignoredReturnValue + std::rbegin(arr); + //cppcheck-suppress ignoredReturnValue + std::cbegin(arr); + //cppcheck-suppress ignoredReturnValue + std::crbegin(arr); + + //cppcheck-suppress ignoredReturnValue + std::end(arr); + //cppcheck-suppress ignoredReturnValue + std::rend(arr); + //cppcheck-suppress ignoredReturnValue + std::cend(arr); + //cppcheck-suppress ignoredReturnValue + std::crend(arr); } \ No newline at end of file diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 60dd5bbb9..3c0de6d21 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -2281,6 +2281,13 @@ private: "}"); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n", errout.str()); + check("auto f() {\n" + " std::vector x;\n" + " auto it = std::begin(x);\n" + " return it;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n", errout.str()); + check("auto f() {\n" " std::vector x;\n" " auto p = x.data();\n" diff --git a/test/teststl.cpp b/test/teststl.cpp index d4a9cce16..7808ed7fd 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1885,7 +1885,6 @@ private: ASSERT_EQUALS("", errout.str()); check("std::vector& f();\n" - "std::vector& g();\n" "void foo() {\n" " auto it = f().end() - 1;\n" " f().begin() - it;\n" @@ -1907,18 +1906,28 @@ private: " (void)std::find(begin(f()), end(f()) - 1, 0);\n" " (void)std::find(begin(f()) + 1, end(f()) - 1, 0);\n" "}"); - ASSERT_EQUALS("[test.cpp:10]: (error) Dereference of an invalid iterator: f().end()+1\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (error) Dereference of an invalid iterator: f().end()+1\n", errout.str()); check("std::vector& f();\n" - "std::vector& g();\n" "void foo() {\n" " if(f().begin() == f().end()) {}\n" " if(f().begin() == f().end()+1) {}\n" " if(f().begin()+1 == f().end()) {}\n" " if(f().begin()+1 == f().end()+1) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Dereference of an invalid iterator: f().end()+1\n" - "[test.cpp:7]: (error) Dereference of an invalid iterator: f().end()+1\n", + ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: f().end()+1\n" + "[test.cpp:6]: (error) Dereference of an invalid iterator: f().end()+1\n", + errout.str()); + + check("std::vector& f();\n" + "void foo() {\n" + " if(std::begin(f()) == std::end(f())) {}\n" + " if(std::begin(f()) == std::end(f())+1) {}\n" + " if(std::begin(f())+1 == std::end(f())) {}\n" + " if(std::begin(f())+1 == std::end(f())+1) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: std::end(f())+1\n" + "[test.cpp:6]: (error) Dereference of an invalid iterator: std::end(f())+1\n", errout.str()); check("template\n" @@ -4496,6 +4505,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: i\n", errout.str()); + check("void f() {\n" + " std::vector v;\n" + " std::vector ::iterator i = std::end(v);\n" + " *i=0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: i\n", errout.str()); + check("void f(std::vector v) {\n" " std::vector ::iterator i = v.end();\n" " *i=0;\n" @@ -4520,6 +4536,12 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:3]: (error) Dereference of an invalid iterator: i-1\n", errout.str()); + check("void f(std::vector v) {\n" + " std::vector ::iterator i = std::begin(v);\n" + " *(i-1)=0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Dereference of an invalid iterator: i-1\n", errout.str()); + check("void f(std::vector v, bool b) {\n" " std::vector ::iterator i = v.begin();\n" " if (b)\n"