[PATCH] Fix memory leak of the demangled symbol name

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix memory leak of the demangled symbol name

Sourceware - gdb-patches mailing list
compute_and_set_names would only free the name if we did not find the name
in the hashtable, but it needs to always free it.  Solve this by moving the
smart pointer outside the if.

Thanks to PhilippeW for finding this.

gdb/ChangeLog:

2020-01-07  Christian Biesinger  <[hidden email]>

        * symtab.c (general_symbol_info::compute_and_set_names): Move the
        unique_xmalloc_ptr outside the if to always free the demangled name.

Change-Id: Id7c6b8408432183700ccb5ff634818d6c5a3ac95
---
 gdb/symtab.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 88b8faedb5..122f9051af 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -892,6 +892,16 @@ general_symbol_info::compute_and_set_names (gdb::string_view linkage_name,
           htab_find_slot_with_hash (per_bfd->demangled_names_hash.get (),
     &entry, *hash, INSERT));
 
+  /* The const_cast is safe because the only reason it is already
+     initialized is if we purposefully set it from a background
+     thread to avoid doing the work here.  However, it is still
+     allocated from the heap and needs to be freed by us, just
+     like if we called symbol_find_demangled_name here.  If this is
+     nullptr, we call symbol_find_demangled_name below, but we put
+     this smart pointer here to be sure that we don't leak this name.  */
+  gdb::unique_xmalloc_ptr<char> demangled_name
+    (const_cast<char *> (language_specific.demangled_name));
+
   /* If this name is not in the hash table, add it.  */
   if (*slot == NULL
       /* A C version of the symbol may have already snuck into the table.
@@ -914,15 +924,9 @@ general_symbol_info::compute_and_set_names (gdb::string_view linkage_name,
       else
  linkage_name_copy = linkage_name;
 
-      /* The const_cast is safe because the only reason it is already
-         initialized is if we purposefully set it from a background
-         thread to avoid doing the work here.  However, it is still
-         allocated from the heap and needs to be freed by us, just
-         like if we called symbol_find_demangled_name here.  */
-      gdb::unique_xmalloc_ptr<char> demangled_name
- (language_specific.demangled_name
- ? const_cast<char *> (language_specific.demangled_name)
- : symbol_find_demangled_name (this, linkage_name_copy.data ()));
+      if (demangled_name.get () == nullptr)
+ demangled_name.reset
+   (symbol_find_demangled_name (this, linkage_name_copy.data ()));
 
       /* Suppose we have demangled_name==NULL, copy_name==0, and
  linkage_name_copy==linkage_name.  In this case, we already have the
--
2.24.1.735.g03f4e72817-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix memory leak of the demangled symbol name

Tom Tromey-2
>>>>> "Christian" == Christian Biesinger via gdb-patches <[hidden email]> writes:

Christian> 2020-01-07  Christian Biesinger  <[hidden email]>

Christian> * symtab.c (general_symbol_info::compute_and_set_names): Move the
Christian> unique_xmalloc_ptr outside the if to always free the demangled name.

Thanks, this is ok.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix memory leak of the demangled symbol name

Sourceware - gdb-patches mailing list
On Thu, Jan 9, 2020 at 8:49 AM Tom Tromey <[hidden email]> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <[hidden email]> writes:
>
> Christian> 2020-01-07  Christian Biesinger  <[hidden email]>
>
> Christian>      * symtab.c (general_symbol_info::compute_and_set_names): Move the
> Christian>      unique_xmalloc_ptr outside the if to always free the demangled name.
>
> Thanks, this is ok.

Thanks, pushed.

To ssh://sourceware.org/git/binutils-gdb.git
   ffebb0bbde..57d7500265  HEAD -> master

Christian