Use class to manage BFD reference counts

This introduces a new specialization of gdb::ref_ptr that can be used
to manage BFD reference counts.  Then it changes most places in gdb to
use this new class, rather than explicit reference-counting or
cleanups.  This patch removes make_cleanup_bfd_unref.

If you look you will see a couple of spots using "release" where a use
of gdb_bfd_ref_ptr would be cleaner.  These will be fixed in the next
patch.

I think this patch fixes some latent bugs.  For example, it seems to
me that previously objfpy_add_separate_debug_file leaked a BFD.

I'm not 100% certain that the macho_symfile_read_all_oso change is
correct.  The existing code here is hard for me to follow.  One goal
of this sort of automated reference counting, though, is to make it
more difficult to make logic errors; so hopefully the code is clear
now.

2017-01-10  Tom Tromey  <tom@tromey.com>

	* windows-tdep.c (windows_xfer_shared_library): Update.
	* windows-nat.c (windows_make_so): Update.
	* utils.h (make_cleanup_bfd_unref): Remove.
	* utils.c (do_bfd_close_cleanup, make_cleanup_bfd_unref): Remove.
	* symfile.h (symfile_bfd_open)
	(find_separate_debug_file_in_section): Return gdb_bfd_ref_ptr.
	* symfile.c (read_symbols, symbol_file_add)
	(separate_debug_file_exists): Update.
	(symfile_bfd_open): Return gdb_bfd_ref_ptr.
	(generic_load, reread_symbols): Update.
	* symfile-mem.c (symbol_file_add_from_memory): Update.
	* spu-linux-nat.c (spu_bfd_open): Return gdb_bfd_ref_ptr.
	(spu_symbol_file_add_from_memory): Update.
	* solist.h (struct target_so_ops) <bfd_open>: Return
	gdb_bfd_ref_ptr.
	(solib_bfd_fopen, solib_bfd_open): Return gdb_bfd_ref_ptr.
	* solib.c (solib_bfd_fopen, solib_bfd_open): Return
	gdb_bfd_ref_ptr.
	(solib_map_sections, reload_shared_libraries_1): Update.
	* solib-svr4.c (enable_break): Update.
	* solib-spu.c (spu_bfd_fopen): Return gdb_bfd_ref_ptr.
	* solib-frv.c (enable_break2): Update.
	* solib-dsbt.c (enable_break): Update.
	* solib-darwin.c (gdb_bfd_mach_o_fat_extract): Return
	gdb_bfd_ref_ptr.
	(darwin_solib_get_all_image_info_addr_at_init): Update.
	(darwin_bfd_open): Return gdb_bfd_ref_ptr.
	* solib-aix.c (solib_aix_bfd_open): Return gdb_bfd_ref_ptr.
	* record-full.c (record_full_save): Update.
	* python/py-objfile.c (objfpy_add_separate_debug_file): Update.
	* procfs.c (insert_dbx_link_bpt_in_file): Update.
	* minidebug.c (find_separate_debug_file_in_section): Return
	gdb_bfd_ref_ptr.
	* machoread.c (macho_add_oso_symfile): Change abfd to
	gdb_bfd_ref_ptr.
	(macho_symfile_read_all_oso): Update.
	(macho_check_dsym): Return gdb_bfd_ref_ptr.
	(macho_symfile_read): Update.
	* jit.c (bfd_open_from_target_memory): Return gdb_bfd_ref_ptr.
	(jit_bfd_try_read_symtab): Update.
	* gdb_bfd.h (gdb_bfd_open, gdb_bfd_fopen, gdb_bfd_openr)
	(gdb_bfd_openw, gdb_bfd_openr_iovec)
	(gdb_bfd_openr_next_archived_file, gdb_bfd_fdopenr): Return
	gdb_bfd_ref_ptr.
	(gdb_bfd_ref_policy): New struct.
	(gdb_bfd_ref_ptr): New typedef.
	* gdb_bfd.c (gdb_bfd_open, gdb_bfd_fopen, gdb_bfd_openr)
	(gdb_bfd_openw, gdb_bfd_openr_iovec)
	(gdb_bfd_openr_next_archived_file, gdb_bfd_fdopenr): Return
	gdb_bfd_ref_ptr.
	* gcore.h (create_gcore_bfd): Return gdb_bfd_ref_ptr.
	* gcore.c (create_gcore_bfd): Return gdb_bfd_ref_ptr.
	(gcore_command): Update.
	* exec.c (exec_file_attach): Update.
	* elfread.c (elf_symfile_read): Update.
	* dwarf2read.c (dwarf2_get_dwz_file): Update.
	(try_open_dwop_file, open_dwo_file): Return gdb_bfd_ref_ptr.
	(open_and_init_dwo_file): Update.
	(open_dwp_file): Return gdb_bfd_ref_ptr.
	(open_and_init_dwp_file): Update.
	* corelow.c (core_open): Update.
	* compile/compile-object-load.c (compile_object_load): Update.
	* common/gdb_ref_ptr.h (ref_ptr::operator->): New operator.
	* coffread.c (coff_symfile_read): Update.
	* cli/cli-dump.c (bfd_openr_or_error, bfd_openw_or_error): Return
	gdb_bfd_ref_ptr.  Rename.
	(dump_bfd_file, restore_command): Update.
	* build-id.h (build_id_to_debug_bfd): Return gdb_bfd_ref_ptr.
	* build-id.c (build_id_to_debug_bfd): Return gdb_bfd_ref_ptr.
	(find_separate_debug_file_by_buildid): Update.

1 files changed, 28 insertions, 53 deletions

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 61b9e5b..f31075d 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -876,8 +876,7 @@ read_symbols (struct objfile *objfile, symfile_add_flags add_flags)
       && objfile->separate_debug_objfile == NULL
       && objfile->separate_debug_objfile_backlink == NULL)
     {
-      bfd *abfd = find_separate_debug_file_in_section (objfile);
-      struct cleanup *cleanup = make_cleanup_bfd_unref (abfd);
+      gdb_bfd_ref_ptr abfd (find_separate_debug_file_in_section (objfile));
 
       if (abfd != NULL)
 	{
@@ -885,11 +884,9 @@ read_symbols (struct objfile *objfile, symfile_add_flags add_flags)
 	     virtual section-as-bfd like the bfd filename containing the
 	     section.  Therefore use also non-canonical name form for the same
 	     file containing the section.  */
-	  symbol_file_add_separate (abfd, objfile->original_name, add_flags,
-				    objfile);
+	  symbol_file_add_separate (abfd.get (), objfile->original_name,
+				    add_flags, objfile);
 	}
-
-      do_cleanups (cleanup);
     }
   if ((add_flags & SYMFILE_NO_READ) == 0)
     require_partial_symbols (objfile, 0);
@@ -1287,13 +1284,10 @@ struct objfile *
 symbol_file_add (const char *name, symfile_add_flags add_flags,
 		 struct section_addr_info *addrs, objfile_flags flags)
 {
-  bfd *bfd = symfile_bfd_open (name);
-  struct cleanup *cleanup = make_cleanup_bfd_unref (bfd);
-  struct objfile *objf;
+  gdb_bfd_ref_ptr bfd (symfile_bfd_open (name));
 
-  objf = symbol_file_add_from_bfd (bfd, name, add_flags, addrs, flags, NULL);
-  do_cleanups (cleanup);
-  return objf;
+  return symbol_file_add_from_bfd (bfd.get (), name, add_flags, addrs,
+				   flags, NULL);
 }
 
 /* Call symbol_file_add() with default values and update whatever is
@@ -1354,7 +1348,6 @@ separate_debug_file_exists (const char *name, unsigned long crc,
 {
   unsigned long file_crc;
   int file_crc_p;
-  bfd *abfd;
   struct stat parent_stat, abfd_stat;
   int verified_as_different;
 
@@ -1367,9 +1360,9 @@ separate_debug_file_exists (const char *name, unsigned long crc,
   if (filename_cmp (name, objfile_name (parent_objfile)) == 0)
     return 0;
 
-  abfd = gdb_bfd_open (name, gnutarget, -1);
+  gdb_bfd_ref_ptr abfd (gdb_bfd_open (name, gnutarget, -1));
 
-  if (!abfd)
+  if (abfd == NULL)
     return 0;
 
   /* Verify symlinks were not the cause of filename_cmp name difference above.
@@ -1383,24 +1376,19 @@ separate_debug_file_exists (const char *name, unsigned long crc,
      numbers will never set st_ino to zero, this is merely an
      optimization, so we do not need to worry about false negatives.  */
 
-  if (bfd_stat (abfd, &abfd_stat) == 0
+  if (bfd_stat (abfd.get (), &abfd_stat) == 0
       && abfd_stat.st_ino != 0
       && bfd_stat (parent_objfile->obfd, &parent_stat) == 0)
     {
       if (abfd_stat.st_dev == parent_stat.st_dev
 	  && abfd_stat.st_ino == parent_stat.st_ino)
-	{
-	  gdb_bfd_unref (abfd);
-	  return 0;
-	}
+	return 0;
       verified_as_different = 1;
     }
   else
     verified_as_different = 0;
 
-  file_crc_p = gdb_bfd_crc (abfd, &file_crc);
-
-  gdb_bfd_unref (abfd);
+  file_crc_p = gdb_bfd_crc (abfd.get (), &file_crc);
 
   if (!file_crc_p)
     return 0;
@@ -1721,10 +1709,9 @@ set_initial_language (void)
    includes a newly malloc'd` copy of NAME (tilde-expanded and made
    absolute).  In case of trouble, error() is called.  */
 
-bfd *
+gdb_bfd_ref_ptr
 symfile_bfd_open (const char *name)
 {
-  bfd *sym_bfd;
   int desc = -1;
   struct cleanup *back_to = make_cleanup (null_cleanup, 0);
 
@@ -1760,20 +1747,17 @@ symfile_bfd_open (const char *name)
       name = absolute_name;
     }
 
-  sym_bfd = gdb_bfd_open (name, gnutarget, desc);
-  if (!sym_bfd)
+  gdb_bfd_ref_ptr sym_bfd (gdb_bfd_open (name, gnutarget, desc));
+  if (sym_bfd == NULL)
     error (_("`%s': can't open to read symbols: %s."), name,
 	   bfd_errmsg (bfd_get_error ()));
 
-  if (!gdb_bfd_has_target_filename (sym_bfd))
-    bfd_set_cacheable (sym_bfd, 1);
+  if (!gdb_bfd_has_target_filename (sym_bfd.get ()))
+    bfd_set_cacheable (sym_bfd.get (), 1);
 
-  if (!bfd_check_format (sym_bfd, bfd_object))
-    {
-      make_cleanup_bfd_unref (sym_bfd);
-      error (_("`%s': can't read symbols: %s."), name,
-	     bfd_errmsg (bfd_get_error ()));
-    }
+  if (!bfd_check_format (sym_bfd.get (), bfd_object))
+    error (_("`%s': can't read symbols: %s."), name,
+	   bfd_errmsg (bfd_get_error ()));
 
   do_cleanups (back_to);
 
@@ -2074,7 +2058,6 @@ static void print_transfer_performance (struct ui_file *stream,
 void
 generic_load (const char *args, int from_tty)
 {
-  bfd *loadfile_bfd;
   char *filename;
   struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0);
   struct load_section_data cbdata;
@@ -2115,25 +2098,23 @@ generic_load (const char *args, int from_tty)
     }
 
   /* Open the file for loading.  */
-  loadfile_bfd = gdb_bfd_open (filename, gnutarget, -1);
+  gdb_bfd_ref_ptr loadfile_bfd (gdb_bfd_open (filename, gnutarget, -1));
   if (loadfile_bfd == NULL)
     {
       perror_with_name (filename);
       return;
     }
 
-  make_cleanup_bfd_unref (loadfile_bfd);
-
-  if (!bfd_check_format (loadfile_bfd, bfd_object))
+  if (!bfd_check_format (loadfile_bfd.get (), bfd_object))
     {
       error (_("\"%s\" is not an object file: %s"), filename,
 	     bfd_errmsg (bfd_get_error ()));
     }
 
-  bfd_map_over_sections (loadfile_bfd, add_section_size_callback,
+  bfd_map_over_sections (loadfile_bfd.get (), add_section_size_callback,
 			 (void *) &total_progress.total_size);
 
-  bfd_map_over_sections (loadfile_bfd, load_section_callback, &cbdata);
+  bfd_map_over_sections (loadfile_bfd.get (), load_section_callback, &cbdata);
 
   using namespace std::chrono;
 
@@ -2145,7 +2126,7 @@ generic_load (const char *args, int from_tty)
 
   steady_clock::time_point end_time = steady_clock::now ();
 
-  entry = bfd_get_start_address (loadfile_bfd);
+  entry = bfd_get_start_address (loadfile_bfd.get ());
   entry = gdbarch_addr_bits_remove (target_gdbarch (), entry);
   uiout->text ("Start address ");
   uiout->field_fmt ("address", "%s", paddress (target_gdbarch (), entry));
@@ -2569,22 +2550,16 @@ reread_symbols (void)
 
 	  /* Clean up any state BFD has sitting around.  */
 	  {
-	    struct bfd *obfd = objfile->obfd;
+	    gdb_bfd_ref_ptr obfd (objfile->obfd);
 	    char *obfd_filename;
 
 	    obfd_filename = bfd_get_filename (objfile->obfd);
 	    /* Open the new BFD before freeing the old one, so that
 	       the filename remains live.  */
-	    objfile->obfd = gdb_bfd_open (obfd_filename, gnutarget, -1);
+	    gdb_bfd_ref_ptr temp (gdb_bfd_open (obfd_filename, gnutarget, -1));
+	    objfile->obfd = temp.release ();
 	    if (objfile->obfd == NULL)
-	      {
-		/* We have to make a cleanup and error here, rather
-		   than erroring later, because once we unref OBFD,
-		   OBFD_FILENAME will be freed.  */
-		make_cleanup_bfd_unref (obfd);
-		error (_("Can't open %s to read symbols."), obfd_filename);
-	      }
-	    gdb_bfd_unref (obfd);
+	      error (_("Can't open %s to read symbols."), obfd_filename);
 	  }
 
 	  original_name = xstrdup (objfile->original_name);