gdb: Don't leak memory with TYPE_ALLOC / TYPE_ZALLOC
authorAndrew Burgess <andrew.burgess@embecosm.com>
Mon, 10 Sep 2018 12:50:34 +0000 (13:50 +0100)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Fri, 14 Sep 2018 22:10:09 +0000 (23:10 +0100)
commit2fabdf3381dfc4bc89f6339c3db3e4bfe7ca8b14
tree9d03730f888cdfe2c294f68d03be4ad98e2897d1
parent982d0151e9ada98e2b5577f9af797b396e48f815
gdb: Don't leak memory with TYPE_ALLOC / TYPE_ZALLOC

This patch started as an observation from valgrind that GDB appeared
to be loosing track of some memory associated with types.  An example
valgrind stack would be:

  24 bytes in 1 blocks are possibly lost in loss record 419 of 5,361
     at 0x4C2EA1E: calloc (vg_replace_malloc.c:711)
     by 0x623D26: xcalloc (common-utils.c:85)
     by 0x623D65: xzalloc(unsigned long) (common-utils.c:95)
     by 0x72A066: make_function_type(type*, type**) (gdbtypes.c:510)
     by 0x72A098: lookup_function_type(type*) (gdbtypes.c:521)
     by 0x73635D: gdbtypes_post_init(gdbarch*) (gdbtypes.c:5439)
     by 0x727590: gdbarch_data(gdbarch*, gdbarch_data*) (gdbarch.c:5230)
     by 0x735B99: builtin_type(gdbarch*) (gdbtypes.c:5313)
     by 0x514D95: elf_rel_plt_read(minimal_symbol_reader&, objfile*, bfd_symbol**) (elfread.c:542)
     by 0x51662F: elf_read_minimal_symbols(objfile*, int, elfinfo const*) (elfread.c:1121)
     by 0x5168A5: elf_symfile_read(objfile*, enum_flags<symfile_add_flag>) (elfread.c:1207)
     by 0x8520F5: read_symbols(objfile*, enum_flags<symfile_add_flag>) (symfile.c:794)

When we look in make_function_type we find a call to TYPE_ZALLOC
(inside the INIT_FUNC_SPECIFIC macro).  It is this call to TYPE_ZALLOC
that is allocating memory with xcalloc, that is then getting lost.

The problem is tht calling TYPE_ALLOC or TYPE_ZALLOC currently
allocates memory from either the objfile obstack or by using malloc.
The problem with this is that types are allocated either on the
objfile obstack, or on the gdbarch obstack.

As a result, if we discard a type associated with an objfile then
auxiliary data allocated with TYPE_(Z)ALLOC will be correctly
discarded.  But, if we were ever to discard a gdbarch then any
auxiliary type data would be leaked.  Right now there are very few
places in GDB where a gdbarch is ever discarded, but it shouldn't hurt
to close down these bugs as we spot them.

This commit ensures that auxiliary type data is allocated from the
same obstack as the type itself, which should reduce leaked memory.

The one problem case that I found with this change was in eval.c,
where in one place we allocate a local type structure, and then used
TYPE_ZALLOC to allocate some space for the type.  This local type is
neither object file owned, nor gdbarch owned, and so the updated
TYPE_ALLOC code is unable to find an objstack to allocate space on.

My proposed solution for this issue is that the space should be
allocated with a direct call to xzalloc.  We could extend TYPE_ALLOC
to check for type->gdbarch being null, and then fall back to a direct
call to xzalloc, however, I think that making this rare case of a
local type require special handling is not a bad thing, this serves to
highlight that clearing up the memory will require special handling
too.

This special case of a local type is interesting as the types owner
field (contained within the main_type) is completely null.  While
reflecting on this I looked at how types use the get_type_arch
function.  It seems clear that, based on how this is used, it is never
intended that null will be returned from this function.  This only
goes to reinforce, how locally alloctaed types, with no owner, are
both special, and need to be handled carefully.  To help spot errors
earlier, I added an assert into get_type_arch that the returned arch
is not null.

Inside gdbarch.c I found a few other places where auxiliary type data
was being allocated directly on the heap rather than on the types
obstack.  I have fixed these to call TYPE_ALLOC now.

Finally, it is worth noting that as we don't clean up our gdbarch
objects yet, then this will not make much of an impact on the amount
of memory reported as lost at program termination time.  Memory
allocated for auxiliary type information is still not freed, however,
it is now on the correct obstack.  If we do ever start freeing our
gdbarch structures then the associated type data will be cleaned up
correctly.

Tested on X86-64 GNU/Linux with no regressions.

gdb/ChangeLog:

* eval.c (fake_method::fake_method): Call xzalloc directly for a
type that is neither object file owned, nor gdbarch owned.
* gdbtypes.c (get_type_gdbarch): Add an assert that returned
gdbarch is non-NULL.
(alloc_type_instance): Allocate non-objfile owned types on the
gdbarch obstack.
(copy_type_recursive): Allocate TYPE_FIELDS and TYPE_RANGE_DATA
using TYPE_ALLOC to ensure memory is allocated on the correct
obstack.
* gdbtypes.h (TYPE_ALLOC): Allocate space on either the objfile
obstack, or the gdbarch obstack.
(TYPE_ZALLOC): Rewrite using TYPE_ALLOC.
gdb/ChangeLog
gdb/eval.c
gdb/gdbtypes.c
gdb/gdbtypes.h