Fix issue 9437: Dont assume init list constructor for strings (cppcheck-opensource#2366)

pfultz2 · danmar · commit 4ebf54d0902e · 2019-11-17T03:22:04.000+01:00
* Fix issue 9437: Dont assume init list constuctor for strings

* Update the schema

* Add documentation
diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng
@@ -369,6 +369,9 @@
         <optional>
           <attribute name="opLessAllowed"><ref name="DATA-BOOL"/></attribute>
         </optional>
+        <optional>
+          <attribute name="hasInitializerListConstructor"><ref name="DATA-BOOL"/></attribute>
+        </optional>
         <optional>
           <attribute name="itEndPattern"><text/></attribute>
         </optional>
diff --git a/cfg/std.cfg b/cfg/std.cfg
@@ -7821,7 +7821,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
     <realloc init="true" realloc-arg="3">freopen</realloc>
     <dealloc>fclose</dealloc>
   </resource>
-  <container id="stdContainer" endPattern="&gt; !!::" opLessAllowed="false" itEndPattern="&gt; :: iterator|const_iterator|reverse_iterator|const_reverse_iterator">
+  <container id="stdContainer" endPattern="&gt; !!::" opLessAllowed="false" itEndPattern="&gt; :: iterator|const_iterator|reverse_iterator|const_reverse_iterator" hasInitializerListConstructor="true">
     <type templateParameter="0"/>
     <size>
       <function name="resize" action="resize"/>
@@ -7960,7 +7960,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
       <function name="sort" action="change-content"/>
     </access>
   </container>
-  <container id="stdAllString" inherits="stdContainer" opLessAllowed="true">
+  <container id="stdAllString" inherits="stdContainer" opLessAllowed="true" hasInitializerListConstructor="false">
     <type string="std-like"/>
     <size>
       <function name="push_back" action="push"/>
diff --git a/lib/library.cpp b/lib/library.cpp
@@ -405,6 +405,9 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
             const char* const opLessAllowed = node->Attribute("opLessAllowed");
             if (opLessAllowed)
                 container.opLessAllowed = std::string(opLessAllowed) == "true";
+            const char* const hasInitializerListConstructor = node->Attribute("hasInitializerListConstructor");
+            if (hasInitializerListConstructor)
+                container.hasInitializerListConstructor = std::string(hasInitializerListConstructor) == "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
@@ -196,7 +196,8 @@ class CPPCHECKLIB Library {
             arrayLike_indexOp(false),
             stdStringLike(false),
             stdAssociativeLike(false),
-            opLessAllowed(true) {
+            opLessAllowed(true),
+            hasInitializerListConstructor(false) {
         }
 
         enum class Action {
@@ -219,6 +220,7 @@ class CPPCHECKLIB Library {
         bool stdStringLike;
         bool stdAssociativeLike;
         bool opLessAllowed;
+        bool hasInitializerListConstructor;
 
         Action getAction(const std::string& function) const {
             const std::map<std::string, Function>::const_iterator i = functions.find(function);
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
@@ -3695,16 +3695,30 @@ static void valueFlowLifetimeConstructor(Token* tok,
         }
     }
 }
+
+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;
     Token* parent = tok->astParent();
     while (Token::simpleMatch(parent, ","))
         parent = parent->astParent();
-    if (Token::simpleMatch(parent, "{") && (astIsContainer(parent->astParent()) || astIsPointer(parent->astParent()))) {
+    if (Token::simpleMatch(parent, "{") && hasInitList(parent->astParent())) {
         valueFlowLifetimeConstructor(tok, Token::typeOf(parent->previous()), tokenlist, errorLogger, settings);
-    } else if (Token::simpleMatch(tok, "{") && (astIsContainer(parent) || astIsPointer(parent))) {
+    } else if (Token::simpleMatch(tok, "{") && hasInitList(parent)) {
         std::vector<const Token *> args = getArguments(tok);
         for (const Token *argtok : args) {
             LifetimeStore ls{argtok, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object};
diff --git a/man/reference-cfg-format.md b/man/reference-cfg-format.md
@@ -530,6 +530,8 @@ The size of the type is specified in bytes. Possible values for the "sign" attri
 
 A lot of C++ libraries, among those the STL itself, provide containers with very similar functionality. Libraries can be used to tell cppcheck about their behaviour. Each container needs a unique ID. It can optionally have a startPattern, which must be a valid Token::Match pattern and an endPattern that is compared to the linked token of the first token with such a link. The optional attribute "inherits" takes an ID from a previously defined container.
 
+The `hasInitializerListConstructor` attribute can be set when the container has a constructor taking an initializer list.
+
 Inside the `<container>` tag, functions can be defined inside of the tags `<size>`, `<access>` and `<other>` (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/test/testautovariables.cpp b/test/testautovariables.cpp
@@ -2071,6 +2071,19 @@ class TestAutoVariables : public TestFixture {
               "    return var->c_str();\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("std::string f() {\n"
+              "    std::vector<char> data{};\n"
+              "    data.push_back('a');\n"
+              "    return std::string{ data.data(), data.size() };\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("std::vector<char*> f() {\n"
+              "    char a = 0;\n"
+              "    return std::vector<char*>{&a};\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning object that points to local variable 'a' that will be invalid when returning.\n", errout.str());
     }
 
     void danglingLifetime() {
