[binutils-gdb] Remove dwarf2_per_objfile_free and use after free of dwarf2_per_objfile

[Simon Marchi <simark at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=fc8e7e75c2be02237a7961688b06869814f36a18

commit fc8e7e75c2be02237a7961688b06869814f36a18
Author: Simon Marchi <simon.marchi@ericsson.com>
Date:   Sun Jan 28 12:07:30 2018 -0500

    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=<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.

Diff:
---
 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  <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
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\