From 57d750026550cf3a589e3f28a0cdc303ba5ed039 Mon Sep 17 00:00:00 2001 From: Christian Biesinger Date: Tue, 7 Jan 2020 19:10:40 -0600 Subject: [PATCH] Fix memory leak of the demangled symbol name 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-09 Christian Biesinger * 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/ChangeLog | 5 +++++ gdb/symtab.c | 22 +++++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b10989a410d..2ba16340089 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2020-01-09 Christian Biesinger + + * symtab.c (general_symbol_info::compute_and_set_names): Move the + unique_xmalloc_ptr outside the if to always free the demangled name. + 2020-01-08 Tom Tromey * xcoffread.c (enter_line_range, read_xcoff_symtab) diff --git a/gdb/symtab.c b/gdb/symtab.c index 5274b88f7ab..cdd9f2e4c12 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 demangled_name + (const_cast (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 demangled_name - (language_specific.demangled_name - ? const_cast (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.30.2