[Bug gdb/29257] New: Double free of demangled symbol name

["mhov at undo dot io" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=29257

            Bug ID: 29257
           Summary: Double free of demangled symbol name
           Product: gdb
           Version: 10.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: mhov at undo dot io
  Target Milestone: ---

We've captured a GDB crash where a demangled C++ symbol name is freed twice -
double free or corruption (fasttop):


#0  0x00007f270ffcdc8e in abort () from
/tmp/undodb.639706.1655366126.1865206.f7b28b36e39ecdfc/debuggee-1-il8cy4y9/symbol-files/lib64/libc.so.6
#1  0x00007f271003d057 in __libc_message () from
/tmp/undodb.639706.1655366126.1865206.f7b28b36e39ecdfc/debuggee-1-il8cy4y9/symbol-files/lib64/libc.so.6
#2  0x00007f27100441bc in malloc_printerr () from
/tmp/undodb.639706.1655366126.1865206.f7b28b36e39ecdfc/debuggee-1-il8cy4y9/symbol-files/lib64/libc.so.6
#3  0x00007f2710045c04 in _int_free () from
/tmp/undodb.639706.1655366126.1865206.f7b28b36e39ecdfc/debuggee-1-il8cy4y9/symbol-files/lib64/libc.so.6
#4  0x00000000008f8a59 in xfree<char> (ptr=0x5daaff0 "\300\304\070\a") at
.././gdb-10.2/gdb/../gdbsupport/common-utils.h:62
#5  gdb::xfree_deleter<char>::operator() (this=<synthetic pointer>,
ptr=0x5daaff0 "\300\304\070\a") at
.././gdb-10.2/gdb/../gdbsupport/gdb_unique_ptr.h:34
#6  std::unique_ptr<char, gdb::xfree_deleter<char> >::~unique_ptr
(this=<synthetic pointer>, __in_chrg=<optimised out>) at
/opt/rh/devtoolset-8/root/usr/include/c++/8/bits/unique_ptr.h:274
#7  general_symbol_info::compute_and_set_names (this=this@entry=0x7f26ebcd0808,
linkage_name=..., copy_name=copy_name@entry=false, per_bfd=0x5742db0, hash=...)
    at .././gdb-10.2/gdb/symtab.c:881
#8  0x000000000079ad85 in minimal_symbol_reader::<lambda(minimal_symbol*,
minimal_symbol*)>::operator() (end=<optimised out>, start=<optimised out>,
__closure=<synthetic pointer>)
    at .././gdb-10.2/gdb/symtab.h:421
#9  gdb::parallel_for_each<minimal_symbol*,
minimal_symbol_reader::install()::<lambda(minimal_symbol*, minimal_symbol*)> >
(callback=..., last=<optimised out>, first=<optimised out>)
    at .././gdb-10.2/gdb/../gdbsupport/parallel-for.h:76
#10 minimal_symbol_reader::install (this=this@entry=0x7ffdedc85590) at
.././gdb-10.2/gdb/minsyms.c:1409
#11 0x00000000006ad0a2 in elf_read_minimal_symbols (symfile_flags=8,
ei=0x7ffdedc85570, objfile=0x82e8980) at .././gdb-10.2/gdb/elfread.c:1175
#12 elf_symfile_read (objfile=0x82e8980, symfile_flags=...) at
.././gdb-10.2/gdb/elfread.c:1216
#13 0x00000000008f0c95 in read_symbols (objfile=0x82e8980, add_flags=...) at
.././gdb-10.2/gdb/symfile.c:782
#14 0x00000000008f0595 in syms_from_objfile_1 (add_flags=...,
addrs=0x7ffdedc856f0, objfile=0x82e8980) at .././gdb-10.2/gdb/symfile.c:979
#15 syms_from_objfile (add_flags=..., addrs=<optimised out>, objfile=0x82e8980)
at .././gdb-10.2/gdb/symfile.c:996
#16 symbol_file_add_with_addrs (abfd=<optimised out>, name=0x3b21d00 ...,
add_flags=..., addrs=<optimised out>, flags=..., parent=<optimised out>)
    at .././gdb-10.2/gdb/symfile.c:1099
#17 0x00000000008c9aa4 in solib_read_symbols (so=0x3b21af0, flags=...) at
.././gdb-10.2/gdb/solib.c:688
#18 0x00000000008ca5d2 in solib_add (pattern=pattern@entry=0x0,
from_tty=from_tty@entry=0, readsyms=1) at .././gdb-10.2/gdb/solib.c:993
#19 0x00000000008ca769 in handle_solib_event () at
.././gdb-10.2/gdb/solib.c:1266


I've been trying to understand how interning of demangled symbol names works
and I'm still learning, so apologies if this report isn't very precise.

During this crash, in minimal_symbol_reader::install I can see that we're
finished collecting, sorting and compacting our list of existing and new
minimal symbols. In my instance it looks like we have 9673 minsyms already
defined for this objfile, and we're about to potentially add 8509 more. We're
in the point where we compute and insert demangled names in parallel in
gdb::parallel_for_each (though the machine has decided to use a single thread
in this instance, so the main thread is doing all of the work).

We're crashing in the second phase of the parallel workload.
general_symbol_info::compute_and_set_names has just finished and the demangled
name that is managed by the following unique_xmalloc_ptr is released:
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/symtab.c;hb=ce35d7163e779b1321058b22f005c70ce1524b25#l880.

It turns out that the same instance of this demangled name is assigned to
multiple symbols. These symbols have identical mangled and demangled names, but
they come from different source files and have different addresses.

At the time of crash, the first of these similar symbols I can trace back as
being one of the existing symbols for this objfile. I.e we've called
elf_symfile_read on this object before and we loaded a bunch of symbols then.
At that point the demangled name got allocated from libc heap while first
computing the demangled name. Then, because the demangled name hadn't been
encountered yet, it was std::move'ed into the hash table that contains the
demangled names.

Fair enough, we allocate the name while demangling it, and the ownership of
that storage is transferred to the hash table slot.

Now - the second time we're doing elf_symfile_read for this object, we're again
considering this symbol which was already interned, and this time the symbol
already has had a demangled name assigned to, so we don't allocate a new name
for it. The demangled name is already present in the hash table, so we don't go
through the effort of allocating a new hash table slot for the demangled name
and we also don't end up moving the storage for the demangled name into a new
hash slot.

Because of this, the unique_xmalloc_ptr is still the rightful owner of this
demangled name, so at the end of general_symbol_info::compute_and_set_names it
decides to free that storage when it goes out of scope. But wait - since this
is a symbol that is being re-examined this demangled name is the exact same one
that was previously inserted into the hash table! We go ahead freeing it - and
when the next pre-existing symbol that has the same demangled name is being
considered we try to free the same name again - leading to our crash.

general_symbol_info::compute_and_set_names seems to do exactly one of these:
1. transfer the storage of the demangled name into the hash table
2. let the unique ptr free the storage on scope exit

I think the rationale behind doing this is to allow multiple identical
demangled names to be allocated upfront while ensuring that only one unique
name is interned into the hash table. Subsequent duplicates will find the
existing hash table slot, will not have their storage transferred into the hash
table and so will be freed by the unique ptr.

However, I'm not sure that this takes into account the fact that, for existing
symbols, the demangled name may actually already be in the hashtable instead of
having been allocated by symbol_find_demangled_name.

We're seeing this problem with our Time-Travel debugger UDB, which will move
around in execution history and might make it seem like the same library is
being loaded multiple times. Is it otherwise normal to have elf_symfile_read
called on the same object multiple times? 

Note that I have a recording of this failure so I can produce more details if
required. I only have this failure captured for GDB 10.2.

-- 
You are receiving this mail because:
You are on the CC list for the bug.