Add std::*begin and std::*end cfg (#4796)

cfg/std.cfg

@@ -8539,6 +8539,22 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
       <valid>0:</valid>
     </arg>
   </function>
+  <function name="std::begin,std::cbegin,std::rbegin,std::crbegin">
+    <use-retval/>
+    <leak-ignore/>
+    <noreturn>false</noreturn>
+    <arg nr="1" direction="in"/>
+    <container yields="start-iterator"/>
+    <returnValue type="iterator" container="1"/>
+  </function>
+  <function name="std::end,std::cend,std::rend,std::crend">
+    <use-retval/>
+    <leak-ignore/>
+    <noreturn>false</noreturn>
+    <arg nr="1" direction="in"/>
+    <container yields="end-iterator"/>
+    <returnValue type="iterator" container="1"/>
+  </function>
   <memory>
     <alloc init="false" buffer-size="malloc">malloc</alloc>
     <alloc init="true" buffer-size="calloc">calloc</alloc>

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(), "(");

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);
 

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<std::string, Library::Container::Function>::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<const Token*> args = getArguments(tok);

lib/tokenize.cpp

@@ -8898,7 +8898,9 @@ static const std::set<std::string> 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"
 };
 
 

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;
     }
 
@@ -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,7 +4814,11 @@ 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)) ||
+            } 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;
@@ -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();

test/cfg/std.cpp

@@ -4662,3 +4662,46 @@ void stdspan()
     spn3.subspan<1, 1>();
     #endif
 }
+
+void beginEnd()
+{
+    std::vector<int> 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);
+}

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<int> 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<int> x;\n"
               "    auto p = x.data();\n"

test/teststl.cpp

@@ -1885,7 +1885,6 @@ private:
         ASSERT_EQUALS("", errout.str());
 
         check("std::vector<int>& f();\n"
-              "std::vector<int>& 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<int>& f();\n"
-              "std::vector<int>& 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"
+        ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: f().end()+1\n"
-                      "[test.cpp:7]: (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<int>& 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<int N>\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 <int> v;\n"
+              "    std::vector <int>::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 <int> v) {\n"
               "    std::vector <int>::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 <int> v) {\n"
+              "    std::vector <int>::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 <int> v, bool b) {\n"
               "    std::vector <int>::iterator i = v.begin();\n"
               "    if (b)\n"