From fc8e7e75c2be02237a7961688b06869814f36a18 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 28 Jan 2018 12:07:30 -0500 Subject: [PATCH] Remove dwarf2_per_objfile_free and use after free of dwarf2_per_objfile 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=) 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=) 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=) 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/ChangeLog | 9 +++++++++ gdb/dwarf2read.c | 57 ++++++++++++++++++++++++-------------------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5369e02..c422005 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2018-01-28 Simon Marchi + + * 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 Avoid compilation errors in MinGW native builds diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 96026a8..51d0f39 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -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. */ @@ -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. */ } @@ -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 @@ -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\ -- 2.7.4