Fix #12327 FP memleak when storing pointer in object / #12332 FN meml…

…eak for allocation functions with std:: prefix (danmar#5861)
chrchr-github · Jan 18, 2024 · 303652c · 303652c
1 parent c8bc6f5
commit 303652c
Show file tree

Hide file tree

Showing 11 changed files with 148 additions and 71 deletions.
diff --git a/cfg/bsd.cfg b/cfg/bsd.cfg
@@ -518,6 +518,27 @@
     </arg>
     <arg nr="4"/>
   </function>
+  <!-- void *reallocarray(void *ptr, size_t nmemb, size_t size); -->
+  <function name="reallocarray">
+    <returnValue type="void *"/>
+    <use-retval/>
+    <noreturn>false</noreturn>
+    <arg nr="1">
+      <not-uninit/>
+    </arg>
+    <arg nr="2" direction="in">
+      <not-uninit/>
+      <valid>0:</valid>
+    </arg>
+    <arg nr="3" direction="in">
+      <not-uninit/>
+      <valid>0:</valid>
+    </arg>
+  </function>
+  <memory>
+    <realloc init="false" buffer-size="calloc:2,3">reallocarray</realloc>
+    <dealloc>free</dealloc>
+  </memory>
   <podtype name="FTS"/>
   <podtype name="FTSENT"/>
   <!-- This type definitions refer to https://mandoc.bsd.lv/includes/sys/types.h.html -->

diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng
@@ -35,7 +35,7 @@
               <optional>
                 <attribute name="buffer-size"><ref name="DATA-BUFFER-SIZE"/></attribute>
               </optional>
-              <ref name="DATA-EXTNAME-SINGLE"/>
+              <ref name="DATA-EXTNAME"/>
             </element>
             <element name="realloc">
               <optional>
@@ -50,7 +50,7 @@
               <optional>
                 <attribute name="realloc-arg"><ref name="ARGNO"/></attribute>
               </optional>
-              <ref name="DATA-EXTNAME-SINGLE"/>
+              <ref name="DATA-EXTNAME"/>
             </element>
             <element name="use"><ref name="DATA-EXTNAME"/></element>
           </choice>
@@ -75,7 +75,7 @@
               <optional>
                 <attribute name="arg"><ref name="ARGNO"/></attribute>
               </optional>
-              <ref name="DATA-EXTNAME-SINGLE"/>
+              <ref name="DATA-EXTNAME"/>
             </element>
             <element name="realloc">
               <optional>
@@ -87,7 +87,7 @@
               <optional>
                 <attribute name="realloc-arg"><ref name="ARGNO"/></attribute>
               </optional>
-              <ref name="DATA-EXTNAME-SINGLE"/>
+              <ref name="DATA-EXTNAME"/>
             </element>
             <element name="use"><ref name="DATA-EXTNAME"/></element>
           </choice>

diff --git a/cfg/std.cfg b/cfg/std.cfg
@@ -965,7 +965,7 @@
     </arg>
   </function>
   <!-- void * calloc(size_t nitems, size_t size); -->
-  <function name="calloc">
+  <function name="calloc,std::calloc">
     <use-retval/>
     <noreturn>false</noreturn>
     <returnValue type="void *"/>
@@ -3172,7 +3172,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
     </arg>
   </function>
   <!-- int rand(void); -->
-  <function name="rand">
+  <function name="rand,std::rand">
     <use-retval/>
     <returnValue type="int" unknownValues="all"/>
     <noreturn>false</noreturn>
@@ -4572,23 +4572,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
       <valid>0:</valid>
     </arg>
   </function>
-  <!-- void *reallocarray(void *ptr, size_t nmemb, size_t size); -->
-  <function name="reallocarray,std::reallocarray">
-    <returnValue type="void *"/>
-    <use-retval/>
-    <noreturn>false</noreturn>
-    <arg nr="1">
-      <not-uninit/>
-    </arg>
-    <arg nr="2" direction="in">
-      <not-uninit/>
-      <valid>0:</valid>
-    </arg>
-    <arg nr="3" direction="in">
-      <not-uninit/>
-      <valid>0:</valid>
-    </arg>
-  </function>
   <!-- int remove(const char *filename); -->
   <function name="remove,std::remove">
     <returnValue type="int"/>
@@ -8599,22 +8582,21 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
     <returnValue type="const char *"/>
   </function>
   <memory>
-    <alloc init="false" buffer-size="malloc">malloc</alloc>
-    <alloc init="true" buffer-size="calloc">calloc</alloc>
-    <alloc init="false" buffer-size="malloc:2">aligned_alloc</alloc>
-    <realloc init="false" buffer-size="malloc:2">realloc</realloc>
-    <realloc init="false" buffer-size="calloc:2,3">reallocarray</realloc>
-    <dealloc>free</dealloc>
+    <alloc init="false" buffer-size="malloc">malloc,std::malloc</alloc>
+    <alloc init="true" buffer-size="calloc">calloc,std::calloc</alloc>
+    <alloc init="false" buffer-size="malloc:2">aligned_alloc,std::aligned_alloc</alloc>
+    <realloc init="false" buffer-size="malloc:2">realloc,std::realloc</realloc>
+    <dealloc>free,std::free</dealloc>
   </memory>
   <memory>
-    <alloc init="true" buffer-size="strdup">strdup</alloc>
-    <dealloc>free</dealloc>
+    <alloc init="true" buffer-size="strdup">strdup,std::strdup</alloc>
+    <dealloc>free,std::free</dealloc>
   </memory>
   <resource>
-    <alloc init="true">fopen</alloc>
-    <alloc init="true">tmpfile</alloc>
-    <realloc init="true" realloc-arg="3">freopen</realloc>
-    <dealloc>fclose</dealloc>
+    <alloc init="true">fopen,std::fopen</alloc>
+    <alloc init="true">tmpfile,std::tmpfile</alloc>
+    <realloc init="true" realloc-arg="3">freopen,std::freopen</realloc>
+    <dealloc>fclose,std::fclose</dealloc>
   </resource>
   <container id="stdContainer" endPattern="&gt; !!::" opLessAllowed="false" itEndPattern="&gt; :: iterator|const_iterator|reverse_iterator|const_reverse_iterator" hasInitializerListConstructor="true">
     <type templateParameter="0"/>

diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp
@@ -181,7 +181,9 @@ void CheckLeakAutoVar::configurationInfo(const Token* tok, const std::pair<const
 {
     if (mSettings->checkLibrary && functionUsage.second == VarInfo::USED &&
         (!functionUsage.first || !functionUsage.first->function() || !functionUsage.first->function()->hasBody())) {
-        const std::string funcStr = functionUsage.first ? mSettings->library.getFunctionName(functionUsage.first) : "f";
+        std::string funcStr = functionUsage.first ? mSettings->library.getFunctionName(functionUsage.first) : "f";
+        if (funcStr.empty())
+            funcStr = "unknown::" + functionUsage.first->str();
         reportError(tok,
                     Severity::information,
                     "checkLibraryUseIgnore",
@@ -329,7 +331,7 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken,
 
         // check each token
         {
-            const bool isInit = Token::Match(tok, "%var% {|(") && tok->variable() && tok == tok->variable()->nameToken();
+            const bool isInit = Token::Match(tok, "%var% {|(") && tok->variable() && tok == tok->variable()->nameToken() && tok->variable()->isPointer();
             const Token * nextTok = isInit ? nullptr : checkTokenInsideExpression(tok, varInfo);
             if (nextTok) {
                 tok = nextTok;
@@ -1184,7 +1186,7 @@ void CheckLeakAutoVar::ret(const Token *tok, VarInfo &varInfo, const bool isEndO
                 const auto use = possibleUsage.find(varid);
                 if (use == possibleUsage.end()) {
                     leakError(tok, var->name(), it->second.type);
-                } else {
+                } else if (!use->second.first->variable()) { // TODO: handle constructors
                     configurationInfo(tok, use->second);
                 }
             }

diff --git a/lib/library.cpp b/lib/library.cpp
@@ -216,11 +216,16 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
             int allocationId = 0;
             for (const tinyxml2::XMLElement *memorynode = node->FirstChildElement(); memorynode; memorynode = memorynode->NextSiblingElement()) {
                 if (strcmp(memorynode->Name(),"dealloc")==0) {
-                    const std::map<std::string, AllocFunc>::const_iterator it = mDealloc.find(memorynode->GetText());
-                    if (it != mDealloc.end()) {
-                        allocationId = it->second.groupId;
-                        break;
+                    const auto names = getnames(memorynode->GetText());
+                    for (const auto& n : names) {
+                        const std::map<std::string, AllocFunc>::const_iterator it = mDealloc.find(n);
+                        if (it != mDealloc.end()) {
+                            allocationId = it->second.groupId;
+                            break;
+                        }
                     }
+                    if (allocationId != 0)
+                        break;
                 }
             }
             if (allocationId == 0) {
@@ -234,6 +239,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
             // add alloc/dealloc/use functions..
             for (const tinyxml2::XMLElement *memorynode = node->FirstChildElement(); memorynode; memorynode = memorynode->NextSiblingElement()) {
                 const std::string memorynodename = memorynode->Name();
+                const auto names = getnames(memorynode->GetText());
                 if (memorynodename == "alloc" || memorynodename == "realloc") {
                     AllocFunc temp = {0};
                     temp.groupId = allocationId;
@@ -268,17 +274,18 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
                     if (memorynodename == "realloc")
                         temp.reallocArg = memorynode->IntAttribute("realloc-arg", 1);
 
-                    if (memorynodename != "realloc")
-                        mAlloc[memorynode->GetText()] = temp;
-                    else
-                        mRealloc[memorynode->GetText()] = temp;
+                    auto& map = (memorynodename == "realloc") ? mRealloc : mAlloc;
+                    for (const auto& n : names)
+                        map[n] = temp;
                 } else if (memorynodename == "dealloc") {
                     AllocFunc temp = {0};
                     temp.groupId = allocationId;
                     temp.arg = memorynode->IntAttribute("arg", 1);
-                    mDealloc[memorynode->GetText()] = temp;
+                    for (const auto& n : names)
+                        mDealloc[n] = temp;
                 } else if (memorynodename == "use")
-                    functions[memorynode->GetText()].use = true;
+                    for (const auto& n : names)
+                        functions[n].use = true;
                 else
                     unknown_elements.insert(memorynodename);
             }

diff --git a/test/cfg/bsd.c b/test/cfg/bsd.c
@@ -144,3 +144,30 @@ void uninitvar(void)
     // cppcheck-suppress uninitvar
     (void) arc4random_uniform(uint32Uninit);
 }
+
+void arrayIndexOutOfBounds(void)
+{
+    char * pAlloc = calloc(2, 3);
+    pAlloc[5] = 'a';
+    // cppcheck-suppress arrayIndexOutOfBounds
+    pAlloc[6] = 1;
+    // cppcheck-suppress memleakOnRealloc
+    pAlloc = reallocarray(pAlloc, 3, 3);
+    pAlloc[8] = 'a';
+    // cppcheck-suppress arrayIndexOutOfBounds
+    pAlloc[9] = 1;
+    free(pAlloc);
+}
+
+void reallocarray_memleak(void) {
+    char *a = (char *)malloc(10);
+    // cppcheck-suppress [memleakOnRealloc, unreadVariable]
+    a = reallocarray(a, 100, 2);
+    // cppcheck-suppress memleak
+}
+
+void reallocarray_notused(void)
+{
+    // cppcheck-suppress [leakReturnValNotUsed, ignoredReturnValue]
+    reallocarray(NULL, 10, 10);
+}
diff --git a/test/cfg/std.c b/test/cfg/std.c
@@ -348,11 +348,6 @@ void arrayIndexOutOfBounds()
     pAlloc3[5] = 'a';
     // cppcheck-suppress arrayIndexOutOfBounds
     pAlloc3[6] = 1;
-    // cppcheck-suppress memleakOnRealloc
-    pAlloc3 = reallocarray(pAlloc3, 3,3);
-    pAlloc3[8] = 'a';
-    // cppcheck-suppress arrayIndexOutOfBounds
-    pAlloc3[9] = 1;
     free(pAlloc3);
 }
 

diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp
@@ -1615,6 +1615,7 @@ void uninitvar_fdim(void)
 
 void uninitvar_fclose(void)
 {
+    // cppcheck-suppress unassignedVariable
     FILE *stream;
     // cppcheck-suppress uninitvar
     (void)std::fclose(stream);
@@ -1847,6 +1848,7 @@ void uninitvar_fread(void)
 
 void uninitvar_free(void)
 {
+    // cppcheck-suppress unassignedVariable
     void *block;
     // cppcheck-suppress uninitvar
     std::free(block);
@@ -1859,7 +1861,7 @@ void uninitvar_freopen(void)
     FILE *stream;
     // cppcheck-suppress uninitvar
     FILE * p = std::freopen(filename,mode,stream);
-    free(p);
+    std::fclose(p);
 }
 
 void uninitvar_frexp(void)
@@ -2557,6 +2559,8 @@ void uninitvar_srand(void)
     unsigned int seed;
     // cppcheck-suppress uninitvar
     (void)std::srand(seed);
+    // cppcheck-suppress ignoredReturnValue
+    std::rand();
 }
 
 void uninitvar_ldiv(void)
@@ -4893,8 +4897,29 @@ void std_vector_data_arithmetic()
     memcpy(buf.data() + 0, "", 1);
 }
 
+void memleak_std_malloc() // #12332
+{
+    //cppcheck-suppress [unreadVariable, constVariablePointer]
+    void* p = std::malloc(1);
+    //cppcheck-suppress memleak
+}
+
+void memleak_std_realloc(void* block, size_t newsize)
+{
+    //cppcheck-suppress [unreadVariable, constVariablePointer]
+    void* p = std::realloc(block, newsize);
+    //cppcheck-suppress memleak
+}
+
+void unusedAllocatedMemory_std_free()
+{
+    //cppcheck-suppress unusedAllocatedMemory
+    void* p = std::malloc(1);
+    std::free(p);
+}
+
 std::string global_scope_std() // #12355
 {
     ::std::stringstream ss;
     return ss.str();
-}
+}
diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp
@@ -597,6 +597,33 @@ class TestLeakAutoVar : public TestFixture {
               "    fd->exec();\n"
               "}\n", true);
         ASSERT_EQUALS("", errout.str());
+
+        check("struct C {\n" // #12327
+              "    char* m_p;\n"
+              "    C(char* p) : m_p(p) {}\n"
+              "};\n"
+              "std::list<C> gli;\n"
+              "void f() {\n"
+              "    std::list<C> li;\n"
+              "    char* p = new char[1];\n"
+              "    C c(p);\n"
+              "    li.push_back(c);\n"
+              "    C c2(li.front());\n"
+              "    delete[] c2.m_p;\n"
+              "}\n"
+              "void g() {\n"
+              "    char* p = new char[1];\n"
+              "    C c(p);\n"
+              "    gli.push_back(c);\n"
+              "}\n"
+              "void h() {\n"
+              "    std::list<C> li;\n"
+              "    char* p = new char[1];\n"
+              "    C c(p);\n"
+              "    li.push_back(c);\n"
+              "    delete[] li.front().m_p;\n"
+              "}\n", true);
+        ASSERT_EQUALS("", errout.str());
     }
 
     void isAutoDealloc() {
@@ -2976,6 +3003,12 @@ class TestLeakAutoVar : public TestFixture {
               "    bar(p);\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("void f(int n) {\n"
+              "    char* p = new char[n];\n"
+              "    v.push_back(p);\n"
+              "}\n", /*cpp*/ true);
+        ASSERT_EQUALS("[test.cpp:4]: (information) --check-library: Function unknown::push_back() should have <use>/<leak-ignore> configuration\n", errout.str());
     }
 };