public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++tools: Fix memory leak
@ 2021-10-21 13:28 Jonathan Wakely
  2021-10-21 19:38 ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Wakely @ 2021-10-21 13:28 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Jason Merrill

The allocated memory is not freed when returning early due to an error.

c++tools/ChangeLog:

	* resolver.cc (module_resolver::read_tuple_file): Use unique_ptr
	to ensure memory is freed before returning.
---
 c++tools/resolver.cc | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/c++tools/resolver.cc b/c++tools/resolver.cc
index 421fdaa55fe..d1b73a47778 100644
--- a/c++tools/resolver.cc
+++ b/c++tools/resolver.cc
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "resolver.h"
 // C++
 #include <algorithm>
+#include <memory>
 // C
 #include <cstring>
 // OS
@@ -114,10 +115,17 @@ module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
   buffer = mmap (nullptr, stat.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
   if (buffer == MAP_FAILED)
     return -errno;
+  struct Deleter {
+    void operator()(void* p) const { munmap(p, size); }
+    size_t size;
+  };
+  std::unique_ptr<void, Deleter> guard(buffer, Deleter{(size_t)stat.st_size});
 #else
   buffer = xmalloc (stat.st_size);
   if (!buffer)
     return -errno;
+  struct Deleter { void operator()(void* p) const { free(p); } };
+  std::unique_ptr<void, Deleter> guard;
   if (read (fd, buffer, stat.st_size) != stat.st_size)
     return -errno;
 #endif
@@ -179,12 +187,6 @@ module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
 	}
     }
 
-#if MAPPED_READING
-  munmap (buffer, stat.st_size);
-#else
-  free (buffer);
-#endif
-
   return 0;
 }
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++tools: Fix memory leak
  2021-10-21 13:28 [PATCH] c++tools: Fix memory leak Jonathan Wakely
@ 2021-10-21 19:38 ` Jason Merrill
  2021-10-21 21:34   ` [PATCH v2] " Jonathan Wakely
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2021-10-21 19:38 UTC (permalink / raw)
  To: Jonathan Wakely, libstdc++, gcc-patches

On 10/21/21 09:28, Jonathan Wakely wrote:
> The allocated memory is not freed when returning early due to an error.
> 
> c++tools/ChangeLog:
> 
> 	* resolver.cc (module_resolver::read_tuple_file): Use unique_ptr
> 	to ensure memory is freed before returning.
> ---
>   c++tools/resolver.cc | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/c++tools/resolver.cc b/c++tools/resolver.cc
> index 421fdaa55fe..d1b73a47778 100644
> --- a/c++tools/resolver.cc
> +++ b/c++tools/resolver.cc
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "resolver.h"
>   // C++
>   #include <algorithm>
> +#include <memory>
>   // C
>   #include <cstring>
>   // OS
> @@ -114,10 +115,17 @@ module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
>     buffer = mmap (nullptr, stat.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
>     if (buffer == MAP_FAILED)
>       return -errno;
> +  struct Deleter {
> +    void operator()(void* p) const { munmap(p, size); }
> +    size_t size;
> +  };
> +  std::unique_ptr<void, Deleter> guard(buffer, Deleter{(size_t)stat.st_size});
>   #else
>     buffer = xmalloc (stat.st_size);
>     if (!buffer)
>       return -errno;
> +  struct Deleter { void operator()(void* p) const { free(p); } };
> +  std::unique_ptr<void, Deleter> guard;

Don't you need to initialize guard from buffer?

>     if (read (fd, buffer, stat.st_size) != stat.st_size)
>       return -errno;
>   #endif
> @@ -179,12 +187,6 @@ module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
>   	}
>       }
>   
> -#if MAPPED_READING
> -  munmap (buffer, stat.st_size);
> -#else
> -  free (buffer);
> -#endif
> -
>     return 0;
>   }
>   
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] c++tools: Fix memory leak
  2021-10-21 19:38 ` Jason Merrill
@ 2021-10-21 21:34   ` Jonathan Wakely
  2021-10-26 14:19     ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Wakely @ 2021-10-21 21:34 UTC (permalink / raw)
  To: Jason Merrill; +Cc: libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

On Thu, 21 Oct 2021 at 20:38, Jason Merrill wrote:

> On 10/21/21 09:28, Jonathan Wakely wrote:
> >   #else
> >     buffer = xmalloc (stat.st_size);
> >     if (!buffer)
> >       return -errno;
> > +  struct Deleter { void operator()(void* p) const { free(p); } };
> > +  std::unique_ptr<void, Deleter> guard;
>
> Don't you need to initialize guard from buffer?
>

Oops, yes!  Updated patch attached.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1622 bytes --]

commit b280f6b5b4339586446eec99e49074e091c27ea5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 21 22:32:23 2021

    c++tools: Fix memory leak
    
    The allocated memory is not freed when returning early due to an error.
    
    c++tools/ChangeLog:
    
            * resolver.cc (module_resolver::read_tuple_file): Use unique_ptr
            to ensure memory is freed before returning.

diff --git a/c++tools/resolver.cc b/c++tools/resolver.cc
index 421fdaa55fe..a1837b3ee10 100644
--- a/c++tools/resolver.cc
+++ b/c++tools/resolver.cc
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "resolver.h"
 // C++
 #include <algorithm>
+#include <memory>
 // C
 #include <cstring>
 // OS
@@ -114,10 +115,17 @@ module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
   buffer = mmap (nullptr, stat.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
   if (buffer == MAP_FAILED)
     return -errno;
+  struct Deleter {
+    void operator()(void* p) const { munmap(p, size); }
+    size_t size;
+  };
+  std::unique_ptr<void, Deleter> guard(buffer, Deleter{(size_t)stat.st_size});
 #else
   buffer = xmalloc (stat.st_size);
   if (!buffer)
     return -errno;
+  struct Deleter { void operator()(void* p) const { free(p); } };
+  std::unique_ptr<void, Deleter> guard(buffer);
   if (read (fd, buffer, stat.st_size) != stat.st_size)
     return -errno;
 #endif
@@ -179,12 +187,6 @@ module_resolver::read_tuple_file (int fd, char const *prefix, bool force)
 	}
     }
 
-#if MAPPED_READING
-  munmap (buffer, stat.st_size);
-#else
-  free (buffer);
-#endif
-
   return 0;
 }
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] c++tools: Fix memory leak
  2021-10-21 21:34   ` [PATCH v2] " Jonathan Wakely
@ 2021-10-26 14:19     ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2021-10-26 14:19 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc Patches

On 10/21/21 17:34, Jonathan Wakely wrote:
> On Thu, 21 Oct 2021 at 20:38, Jason Merrill wrote:
> 
>     On 10/21/21 09:28, Jonathan Wakely wrote:
>      >   #else
>      >     buffer = xmalloc (stat.st_size);
>      >     if (!buffer)
>      >       return -errno;
>      > +  struct Deleter { void operator()(void* p) const { free(p); } };
>      > +  std::unique_ptr<void, Deleter> guard;
> 
>     Don't you need to initialize guard from buffer?
> 
> 
> Oops, yes!  Updated patch attached.

OK, thanks.

Jason



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-26 14:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 13:28 [PATCH] c++tools: Fix memory leak Jonathan Wakely
2021-10-21 19:38 ` Jason Merrill
2021-10-21 21:34   ` [PATCH v2] " Jonathan Wakely
2021-10-26 14:19     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).