From 0e3372df08f645ade2dd7398814105b72625d9af Mon Sep 17 00:00:00 2001 From: ashimaru Date: Wed, 15 Feb 2023 22:00:23 +0100 Subject: [PATCH 01/15] Add configuration and cfg test --- cfg/std.cfg | 10 ++++++++++ test/cfg/std.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/cfg/std.cfg b/cfg/std.cfg index 05a060377e8..efef193c6c7 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8517,6 +8517,16 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init 0: + + + false + + + + + false + + malloc calloc diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index 64788f85d97..5405f5ef68a 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -4605,4 +4605,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 From 42058fef0880f89a33e635d6ff55f6ae27a82846 Mon Sep 17 00:00:00 2001 From: ashimaru Date: Fri, 17 Feb 2023 17:53:42 +0100 Subject: [PATCH 02/15] Fix xml format and add begin/end to stdFunctions --- cfg/std.cfg | 9 +++++++-- lib/tokenize.cpp | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index ff122775521..d34797f93b4 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8525,15 +8525,20 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init 0: - + + false + + - + false + + malloc diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index db1cfc18ec1..784c40d56fa 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -8890,7 +8890,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" }; From a2ba480db68e8229bd3e53e0dc97db7cd816443d Mon Sep 17 00:00:00 2001 From: ashimaru Date: Fri, 17 Feb 2023 17:54:01 +0100 Subject: [PATCH 03/15] Add test to cover basic iterator leak --- test/testautovariables.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 143b5175fae..34d7eee225a 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -2271,6 +2271,13 @@ class TestAutoVariables : public TestFixture { "}"); 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" From efd4688d01ffe8a25e980ac39a46fc6eb3740f10 Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sat, 18 Feb 2023 15:02:36 +0100 Subject: [PATCH 04/15] Fix modernize-pass-by-value warning in valueflow.cpp --- lib/valueflow.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 312740f9f25..048299f5260 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2186,8 +2186,8 @@ class SelectValueFromVarIdMapRange { using pointer = value_type *; using reference = value_type &; - explicit Iterator(const M::const_iterator &it) - : mIt(it) {} + explicit Iterator(M::const_iterator it) + : mIt(std::move(it)) {} reference operator*() const { return mIt->second; From cbda7aadc7d78cf0e6b558b79253d03f335df0d4 Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sat, 18 Feb 2023 22:17:42 +0100 Subject: [PATCH 05/15] Add missing specifiers to function cfg --- cfg/std.cfg | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index d34797f93b4..fabc6a05c38 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8531,14 +8531,15 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init false - + - + + false - + malloc From 43b5f3a2a1620a0e6c6b38e77e2d5506448f9ebb Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sat, 18 Feb 2023 22:19:14 +0100 Subject: [PATCH 06/15] Add iterator value forwarding to result of std::begin/std::end functions --- lib/symboldatabase.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 352de2c66de..f4723320ae4 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7023,13 +7023,14 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to } if (typestr.empty() || typestr == "iterator") { + //Is iterator fetching function invoked on container? if (Token::simpleMatch(tok->astOperand1(), ".") && tok->astOperand1()->astOperand1() && tok->astOperand1()->astOperand2() && 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 || @@ -7042,6 +7043,26 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to setValueType(tok, vt); } } + //Is iterator fetching function called? + } else if (Token::simpleMatch(tok->astOperand1(), " :: ") && + tok->astOperand2() && + tok->astOperand2()->valueType() && + tok->astOperand2()->valueType()->container) { + const auto function = mSettings->library.getFunction(tok->previous()); + if (!function) + { + continue; + } + + if (function->containerYield == Library::Container::Yield::START_ITERATOR || + function->containerYield == Library::Container::Yield::END_ITERATOR || + function->containerYield == Library::Container::Yield::ITERATOR) + { + ValueType vt; + vt.type = ValueType::Type::ITERATOR; + vt.container = tok->astOperand2()->valueType()->container; + setValueType(tok, vt); + } } continue; } From ce54f5f74936770d17849e4c68fdbb33bda0e893 Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sun, 19 Feb 2023 11:49:56 +0100 Subject: [PATCH 07/15] Enable std::begin/end to be analyzed in valueFlow --- lib/symboldatabase.cpp | 14 +++++++++++--- lib/valueflow.cpp | 24 ++++++++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index f4723320ae4..524ca63b225 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7044,10 +7044,17 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to } } //Is iterator fetching function called? - } else if (Token::simpleMatch(tok->astOperand1(), " :: ") && + } else if (Token::simpleMatch(tok->astOperand1(), "::") && tok->astOperand2() && - tok->astOperand2()->valueType() && - tok->astOperand2()->valueType()->container) { + tok->astOperand2()->isVariable()) { + const auto paramVariable = tok->astOperand2()->variable(); + if (!paramVariable || + !paramVariable->valueType() || + !paramVariable->valueType()->container) + { + continue; + } + const auto function = mSettings->library.getFunction(tok->previous()); if (!function) { @@ -7061,6 +7068,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to ValueType vt; vt.type = ValueType::Type::ITERATOR; vt.container = tok->astOperand2()->valueType()->container; + vt.containerTypeToken = tok->astOperand2()->valueType()->containerTypeToken; setValueType(tok, vt); } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 048299f5260..bf63ded17d6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -680,6 +680,14 @@ static void setTokenValue(Token* tok, v.intvalue = !v.intvalue; v.valueType = ValueFlow::Value::ValueType::INT; setTokenValue(parent, v, settings); + } else if (f->containerYield == Library::Container::Yield::START_ITERATOR) { + ValueFlow::Value v(value); + v.valueType = ValueFlow::Value::ValueType::ITERATOR_START; + setTokenValue(parent, v, settings); + } else if (f->containerYield == Library::Container::Yield::END_ITERATOR) { + ValueFlow::Value v(value); + v.valueType = ValueFlow::Value::ValueType::ITERATOR_END; + setTokenValue(parent, v, settings); } } } @@ -4789,7 +4797,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; @@ -4799,8 +4808,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 { @@ -4837,7 +4850,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), value, tokenlist->getSettings()); + if (Token::Match(parent, "(")) + setTokenValue(parent, value, tokenlist->getSettings()); + else + setTokenValue(parent->tokAt(2), value, tokenlist->getSettings()); if (!rt.token->variable()) { LifetimeStore ls = LifetimeStore{ From eec0921871aaecb8018c1b7d6dae83a947470232 Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sun, 19 Feb 2023 16:28:31 +0100 Subject: [PATCH 08/15] Cleanup and more tests --- lib/symboldatabase.cpp | 13 +++++-------- lib/valueflow.cpp | 10 ---------- test/teststl.cpp | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 524ca63b225..455aabae40b 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7050,25 +7050,22 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to const auto paramVariable = tok->astOperand2()->variable(); if (!paramVariable || !paramVariable->valueType() || - !paramVariable->valueType()->container) - { + !paramVariable->valueType()->container) { continue; } const auto function = mSettings->library.getFunction(tok->previous()); - if (!function) - { + if (!function) { continue; } if (function->containerYield == Library::Container::Yield::START_ITERATOR || function->containerYield == Library::Container::Yield::END_ITERATOR || - function->containerYield == Library::Container::Yield::ITERATOR) - { + function->containerYield == Library::Container::Yield::ITERATOR) { ValueType vt; vt.type = ValueType::Type::ITERATOR; - vt.container = tok->astOperand2()->valueType()->container; - vt.containerTypeToken = tok->astOperand2()->valueType()->containerTypeToken; + vt.container = paramVariable->valueType()->container; + vt.containerTypeToken = paramVariable->valueType()->containerTypeToken; setValueType(tok, vt); } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index bf63ded17d6..008d8158526 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -654,7 +654,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()); @@ -680,18 +679,9 @@ static void setTokenValue(Token* tok, v.intvalue = !v.intvalue; v.valueType = ValueFlow::Value::ValueType::INT; setTokenValue(parent, v, settings); - } else if (f->containerYield == Library::Container::Yield::START_ITERATOR) { - ValueFlow::Value v(value); - v.valueType = ValueFlow::Value::ValueType::ITERATOR_START; - setTokenValue(parent, v, settings); - } else if (f->containerYield == Library::Container::Yield::END_ITERATOR) { - ValueFlow::Value v(value); - v.valueType = ValueFlow::Value::ValueType::ITERATOR_END; - setTokenValue(parent, v, settings); } } } - return; } diff --git a/test/teststl.cpp b/test/teststl.cpp index 43590966d03..a2ec96bfd21 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1918,6 +1918,18 @@ class TestStl : public TestFixture { "[test.cpp:7]: (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(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:5]: (error) Dereference of an invalid iterator: f().end()+1\n" + "[test.cpp:7]: (error) Dereference of an invalid iterator: f().end()+1\n", + errout.str()); + check("template\n" "std::vector& f();\n" "void foo() {\n" @@ -4468,6 +4480,13 @@ class TestStl : public TestFixture { "}\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" @@ -4492,6 +4511,12 @@ class TestStl : public TestFixture { "}\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" From fb0af84589f6b2a67075bdb0c9337401fa90b2eb Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sun, 19 Feb 2023 19:53:47 +0100 Subject: [PATCH 09/15] Lookup of function's container yield for iterators --- lib/astutils.cpp | 14 ++++++++++++++ lib/astutils.h | 2 ++ lib/valueflow.cpp | 12 +++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index be81142b280..08ac0954800 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,19 @@ 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; + + auto function = settings->library.getFunction(tok); + if (!function) + return Library::Container::Yield::NO_YIELD; + + *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 8481e4b88b6..908c3d4f739 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/valueflow.cpp b/lib/valueflow.cpp index 008d8158526..a503f2b4dad 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -8079,6 +8079,16 @@ 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; + + //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()) { @@ -8089,7 +8099,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(); From 0fe1fa841b98bd98add0119bcece8da9986a859b Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sun, 19 Feb 2023 20:09:48 +0100 Subject: [PATCH 10/15] Test adjustments --- lib/valueflow.cpp | 3 +++ test/teststl.cpp | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index a503f2b4dad..5470422d49d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -8085,6 +8085,9 @@ static Library::Container::Yield findIteratorYield(Token* tok, const Token** fto if (*ftok) return yield; + if (!tok->astParent()) + return yield; + //begin/end free functions return astFunctionYield(tok->astParent()->previous(), settings, ftok); } diff --git a/test/teststl.cpp b/test/teststl.cpp index a2ec96bfd21..00d5b7d86bf 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1926,8 +1926,8 @@ class TestStl : public TestFixture { " if(std::begin(f())+1 == std::end(f())) {}\n" " if(std::begin(f())+1 == std::end(f())+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:5]: (error) Dereference of an invalid iterator: std::end(f())+1\n" + "[test.cpp:7]: (error) Dereference of an invalid iterator: std::end(f())+1\n", errout.str()); check("template\n" From 3fd2618385146834e2d3c7c90d9f2a3738fe224f Mon Sep 17 00:00:00 2001 From: ashimaru Date: Sun, 19 Feb 2023 21:33:05 +0100 Subject: [PATCH 11/15] Fix issues --- lib/astutils.cpp | 5 +++-- lib/symboldatabase.cpp | 12 ++++-------- lib/valueflow.cpp | 6 +++--- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 08ac0954800..e32e4772eea 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -298,11 +298,12 @@ Library::Container::Yield astFunctionYield(const Token* tok, const Settings* set if (!tok) return Library::Container::Yield::NO_YIELD; - auto function = settings->library.getFunction(tok); + const auto* function = settings->library.getFunction(tok); if (!function) return Library::Container::Yield::NO_YIELD; - *ftok = tok; + if (ftok) + *ftok = tok; return function->containerYield; } diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 455aabae40b..3848e4e6e98 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7054,14 +7054,10 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to continue; } - const auto function = mSettings->library.getFunction(tok->previous()); - if (!function) { - continue; - } - - if (function->containerYield == Library::Container::Yield::START_ITERATOR || - function->containerYield == Library::Container::Yield::END_ITERATOR || - function->containerYield == Library::Container::Yield::ITERATOR) { + 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; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 5470422d49d..360e3fc24b9 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2184,8 +2184,8 @@ class SelectValueFromVarIdMapRange { using pointer = value_type *; using reference = value_type &; - explicit Iterator(M::const_iterator it) - : mIt(std::move(it)) {} + explicit Iterator(const M::const_iterator & it) + : mIt(it) {} reference operator*() const { return mIt->second; @@ -4840,7 +4840,7 @@ 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()); - if (Token::Match(parent, "(")) + if (Token::simpleMatch(parent, "(")) setTokenValue(parent, value, tokenlist->getSettings()); else setTokenValue(parent->tokAt(2), value, tokenlist->getSettings()); From ea49a8b2caf4831f828ecbbd3d986c23e2137cfe Mon Sep 17 00:00:00 2001 From: Mateusz Michalak Date: Mon, 20 Feb 2023 08:05:35 +0100 Subject: [PATCH 12/15] Fix clang-tidy issue and remove test's unused declarations --- lib/symboldatabase.cpp | 2 +- test/teststl.cpp | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 3848e4e6e98..6d88e930955 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7047,7 +7047,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to } else if (Token::simpleMatch(tok->astOperand1(), "::") && tok->astOperand2() && tok->astOperand2()->isVariable()) { - const auto paramVariable = tok->astOperand2()->variable(); + const auto* const paramVariable = tok->astOperand2()->variable(); if (!paramVariable || !paramVariable->valueType() || !paramVariable->valueType()->container) { diff --git a/test/teststl.cpp b/test/teststl.cpp index 00d5b7d86bf..165f58ae623 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1882,7 +1882,6 @@ class TestStl : public TestFixture { 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,7 +1906,6 @@ class TestStl : public TestFixture { ASSERT_EQUALS("[test.cpp:10]: (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" @@ -1919,7 +1917,6 @@ class TestStl : public TestFixture { errout.str()); check("std::vector& f();\n" - "std::vector& g();\n" "void foo() {\n" " if(std::begin(f()) == std::end(f())) {}\n" " if(std::begin(f()) == std::end(f())+1) {}\n" From 363eae694167dec60f7629f7d72abe033a85991f Mon Sep 17 00:00:00 2001 From: ashimaru Date: Mon, 20 Feb 2023 16:46:30 +0100 Subject: [PATCH 13/15] Fix incorrect line numbers in tests --- test/teststl.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/teststl.cpp b/test/teststl.cpp index 165f58ae623..41df177e8aa 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -1903,7 +1903,7 @@ class TestStl : public TestFixture { " (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" "void foo() {\n" @@ -1912,8 +1912,8 @@ class TestStl : public TestFixture { " 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" @@ -1923,8 +1923,8 @@ class TestStl : public TestFixture { " if(std::begin(f())+1 == std::end(f())) {}\n" " if(std::begin(f())+1 == std::end(f())+1) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:5]: (error) Dereference of an invalid iterator: std::end(f())+1\n" - "[test.cpp:7]: (error) Dereference of an invalid iterator: 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" From 44869c8279fde0b7cacf3373a3189f298267e2e8 Mon Sep 17 00:00:00 2001 From: ashimaru Date: Wed, 8 Mar 2023 18:27:13 +0100 Subject: [PATCH 14/15] Resolve merge issues --- lib/symboldatabase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 78ffb8ebd9d..417a7d41536 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7081,7 +7081,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to continue; } - const auto yield = astFunctionYield(tok->previous(), mSettings); + 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) { From b04964f6348afb80ad2cccb288c3525e676f191b Mon Sep 17 00:00:00 2001 From: ashimaru Date: Wed, 8 Mar 2023 21:32:14 +0100 Subject: [PATCH 15/15] Fix issue from merge with #4837 --- lib/symboldatabase.cpp | 36 ++++++++++++++++++------------------ lib/valueflow.cpp | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 417a7d41536..ccb2f0c143e 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7052,24 +7052,6 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to continue; } } - } - if (isReturnIter) { - const std::vector args = getArguments(tok); - if (!args.empty()) { - const Library::ArgumentChecks::IteratorInfo* info = mSettings.library.getArgIteratorInfo(tok->previous(), 1); - if (info && info->it) { - const Token* contTok = args[0]; - if (Token::simpleMatch(args[0]->astOperand1(), ".") && args[0]->astOperand1()->astOperand1()) - contTok = args[0]->astOperand1()->astOperand1(); - if (contTok && contTok->variable() && contTok->variable()->valueType() && contTok->variable()->valueType()->container) { - ValueType vt; - vt.type = ValueType::Type::ITERATOR; - vt.container = contTok->variable()->valueType()->container; - vt.containerTypeToken = contTok->variable()->valueType()->containerTypeToken; - setValueType(tok, vt); - } - } - } //Is iterator fetching function called? } else if (Token::simpleMatch(tok->astOperand1(), "::") && tok->astOperand2() && @@ -7092,6 +7074,24 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to setValueType(tok, vt); } } + if (isReturnIter) { + const std::vector args = getArguments(tok); + if (!args.empty()) { + const Library::ArgumentChecks::IteratorInfo* info = mSettings.library.getArgIteratorInfo(tok->previous(), 1); + if (info && info->it) { + const Token* contTok = args[0]; + if (Token::simpleMatch(args[0]->astOperand1(), ".") && args[0]->astOperand1()->astOperand1()) + contTok = args[0]->astOperand1()->astOperand1(); + if (contTok && contTok->variable() && contTok->variable()->valueType() && contTok->variable()->valueType()->container) { + ValueType vt; + vt.type = ValueType::Type::ITERATOR; + vt.container = contTok->variable()->valueType()->container; + vt.containerTypeToken = contTok->variable()->valueType()->containerTypeToken; + setValueType(tok, vt); + } + } + } + } continue; } TokenList tokenList(&mSettings); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index a8b44a2ab60..09bc36ef87f 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4859,7 +4859,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro if (Token::simpleMatch(parent, "(")) setTokenValue(parent, value, settings); else - setTokenValue(parent->tokAt(2), std::move(value), settings); + setTokenValue(parent->tokAt(2), value, settings); if (!rt.token->variable()) { LifetimeStore ls = LifetimeStore{