Remove dwarf2_per_objfile_free and use after free of dwarf2_per_objfile
Commit Message
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 ...
#4 0x00000000009103ea in objfile_free_data (container=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:61
#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.
---
gdb/dwarf2read.c | 57 ++++++++++++++++++++++++--------------------------------
1 file changed, 24 insertions(+), 33 deletions(-)
Comments
On 2018-01-13 13:10, Simon Marchi wrote:
> 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 ...
> #4 0x00000000009103ea in objfile_free_data (container=0x356cff0) at
> /home/emaisin/src/binutils-gdb/gdb/objfiles.c:61
> #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.
> ---
> gdb/dwarf2read.c | 57
> ++++++++++++++++++++++++--------------------------------
> 1 file changed, 24 insertions(+), 33 deletions(-)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index dca2fe9..8a43b8d 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -2416,6 +2416,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. */
> @@ -2427,6 +2429,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)
> + free_dwo_files (dwo_files, objfile);
> + if (dwp_file)
> + gdb_bfd_unref (dwp_file->dbfd);
> +
> + if (dwz_file && 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. */
> }
>
> @@ -25659,37 +25682,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
> @@ -27321,8 +27313,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\
I pushed this patch.
Simon
@@ -2416,6 +2416,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. */
@@ -2427,6 +2429,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)
+ free_dwo_files (dwo_files, objfile);
+ if (dwp_file)
+ gdb_bfd_unref (dwp_file->dbfd);
+
+ if (dwz_file && 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. */
}
@@ -25659,37 +25682,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
@@ -27321,8 +27313,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\