Skip to content

Commit

Permalink
Remove dwarf2_per_objfile_free and use after free of dwarf2_per_objfile
Browse files Browse the repository at this point in the history
I got some crashes while doing some work with dwarf2_per_objfile.  It
turns out that dwarf2_per_objfile_free is using the dwarf2_per_objfile
objects after their destructor has ran.

The easiest way to reproduce this is to run the inferior twice (do
"start" twice).  Currently, it goes unnoticed, but when I tried to
change all_comp_units and all_type_units to std::vectors, things started
crashing.

The dwarf2_per_objfile objects get destroyed here:

 #0  dwarf2_per_objfile::~dwarf2_per_objfile (this=0x35afe70, __in_chrg=<optimized out>) at /home/emaisin/src/binutils-gdb/gdb/dwarf2read.c:2422
 #1  0x0000000000833282 in dwarf2_free_objfile (objfile=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/dwarf2read.c:25363
 #2  0x0000000000699255 in elf_symfile_finish (objfile=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/elfread.c:1309
 #3  0x0000000000911ed3 in objfile::~objfile (this=0x356cff0, __in_chrg=<optimized out>) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:674

and just after that the dwarf2read per-objfile registry cleanup function
gets called:

 #0  dwarf2_per_objfile_free (objfile=0x356cff0, d=0x35afe70) at /home/emaisin/src/binutils-gdb/gdb/dwarf2read.c:25667
 ... registry boilerplate ...
 bminor#4  0x00000000009103ea in objfile_free_data (container=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:61
 bminor#5  0x0000000000911ee2 in objfile::~objfile (this=0x356cff0, __in_chrg=<optimized out>) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:678

In dwarf2_per_objfile_free, we access fields of the dwarf2_per_objfile
object, which is invalid since its destructor has been executed.

This patch moves the content of dwarf2_per_objfile_free to the
destructor of dwarf2_per_objfile.  The call to
register_objfile_data_with_cleanup in _initialize_dwarf2_read can be
changed to the simpler register_objfile_data.

gdb/ChangeLog:

	* dwarf2read.c (free_dwo_files): Add forward-declaration.
	(dwarf2_per_objfile::~dwarf2_per_objfile): Move content from
	dwarf2_per_objfile_free here.
	(dwarf2_per_objfile_free): Remove.
	(_initialize_dwarf2_read): Don't register
	dwarf2_per_objfile_free as a registry cleanup.
  • Loading branch information
Simon Marchi authored and simark committed Jan 28, 2018
1 parent a8d6d6a commit fc8e7e7
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 33 deletions.
9 changes: 9 additions & 0 deletions gdb/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
2018-01-28 Simon Marchi <simon.marchi@ericsson.com>

* dwarf2read.c (free_dwo_files): Add forward-declaration.
(dwarf2_per_objfile::~dwarf2_per_objfile): Move content from
dwarf2_per_objfile_free here.
(dwarf2_per_objfile_free): Remove.
(_initialize_dwarf2_read): Don't register
dwarf2_per_objfile_free as a registry cleanup.

2018-01-27 Eli Zaretskii <eliz@gnu.org>

Avoid compilation errors in MinGW native builds
Expand Down
57 changes: 24 additions & 33 deletions gdb/dwarf2read.c
Original file line number Diff line number Diff line change
Expand Up @@ -2418,6 +2418,8 @@ dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
locate_sections (obfd, sec, *names);
}

static void free_dwo_files (htab_t dwo_files, struct objfile *objfile);

dwarf2_per_objfile::~dwarf2_per_objfile ()
{
/* Cached DIE trees use xmalloc and the comp_unit_obstack. */
Expand All @@ -2429,6 +2431,27 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()
if (line_header_hash)
htab_delete (line_header_hash);

for (int ix = 0; ix < n_comp_units; ++ix)
VEC_free (dwarf2_per_cu_ptr, all_comp_units[ix]->imported_symtabs);

for (int ix = 0; ix < n_type_units; ++ix)
VEC_free (dwarf2_per_cu_ptr,
all_type_units[ix]->per_cu.imported_symtabs);
xfree (all_type_units);

VEC_free (dwarf2_section_info_def, types);

if (dwo_files != NULL)
free_dwo_files (dwo_files, objfile);
if (dwp_file != NULL)
gdb_bfd_unref (dwp_file->dbfd);

if (dwz_file != NULL && dwz_file->dwz_bfd)
gdb_bfd_unref (dwz_file->dwz_bfd);

if (index_table != NULL)
index_table->~mapped_index ();

/* Everything else should be on the objfile obstack. */
}

Expand Down Expand Up @@ -25470,37 +25493,6 @@ show_dwarf_cmd (const char *args, int from_tty)
cmd_show_list (show_dwarf_cmdlist, from_tty, "");
}

/* Free data associated with OBJFILE, if necessary. */

static void
dwarf2_per_objfile_free (struct objfile *objfile, void *d)
{
struct dwarf2_per_objfile *data = (struct dwarf2_per_objfile *) d;
int ix;

for (ix = 0; ix < data->n_comp_units; ++ix)
VEC_free (dwarf2_per_cu_ptr, data->all_comp_units[ix]->imported_symtabs);

for (ix = 0; ix < data->n_type_units; ++ix)
VEC_free (dwarf2_per_cu_ptr,
data->all_type_units[ix]->per_cu.imported_symtabs);
xfree (data->all_type_units);

VEC_free (dwarf2_section_info_def, data->types);

if (data->dwo_files)
free_dwo_files (data->dwo_files, objfile);
if (data->dwp_file)
gdb_bfd_unref (data->dwp_file->dbfd);

if (data->dwz_file && data->dwz_file->dwz_bfd)
gdb_bfd_unref (data->dwz_file->dwz_bfd);

if (data->index_table != NULL)
data->index_table->~mapped_index ();
}


/* The "save gdb-index" command. */

/* Write SIZE bytes from the buffer pointed to by DATA to FILE, with
Expand Down Expand Up @@ -27132,8 +27124,7 @@ _initialize_dwarf2_read (void)
{
struct cmd_list_element *c;

dwarf2_objfile_data_key
= register_objfile_data_with_cleanup (NULL, dwarf2_per_objfile_free);
dwarf2_objfile_data_key = register_objfile_data ();

add_prefix_cmd ("dwarf", class_maintenance, set_dwarf_cmd, _("\
Set DWARF specific variables.\n\
Expand Down

0 comments on commit fc8e7e7

Please sign in to comment.