This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 01/10] change gdb to refcount bfd everywhere


>>> -  objfile->obfd = gdb_bfd_ref (abfd);
>>> +  objfile->obfd = abfd;

Jan> Caller could gdb_bfd_unref its reference but YMMV.

Tom> I will look into this tomorrow.

This changes allocate_objfile to acquire a reference to the BFD,
rather than steal a reference.  Then it changes all callers, direct or
indirect, to follow.

I'm still undecided as to whether this patch is an improvement.  I
think it does fix a couple of obscure latent memory leaks.

In the abstract it seems like it should be an improvement, since it
makes the rules regarding BFD reference counting more local.  In
practice, though, this means adding a bunch of cleanups, which tends
to obscure the code.

Regtested on x86-64 Fedora 16.

Let me know what you think.

	* coffread.c (coff_symfile_read): Make a cleanup for 'debugfile'
	and 'abfd'.
	* elfread.c (elf_symfile_read): Make a cleanup for 'debugfile'
	and 'abfd'.
	* jit.c (jit_bfd_try_read_symtab): Make a cleanup for 'nbfd'.
	* machoread.c (macho_add_oso_symfile): Make a cleanup for
	'abfd'.
	(macho_symfile_read): Make a cleanup for 'dsym_bfd'.
	* objfiles.c (allocate_objfile): Acquire a new reference.
	* rs6000-nat.c (add_vmap): Don't acquire a BFD reference.
	* solib.c (solib_read_symbols): Don't acquire a BFD reference.
	* spu-linux-nat.c (spu_symbol_file_add_from_memory): Make
	a cleanup for 'nbfd'.
	* symfile-mem.c (symbol_file_add_from_memory): Make a cleanup
	for 'nbfd'.
	* symfile.c (symbol_file_add_with_addrs_or_offsets): Don't
	make a cleanup for 'abfd'.
	(symbol_file_add): Make a BFD cleanup.
---
 gdb/coffread.c      |    3 ++-
 gdb/elfread.c       |    4 +++-
 gdb/jit.c           |    3 ++-
 gdb/machoread.c     |    6 ++++++
 gdb/objfiles.c      |    3 ++-
 gdb/rs6000-nat.c    |    1 -
 gdb/solib.c         |    1 -
 gdb/spu-linux-nat.c |    9 +++++++--
 gdb/symfile-mem.c   |   11 +++++------
 gdb/symfile.c       |   14 ++++++++------
 10 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index b0a8b82..0c7e6d9 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -653,13 +653,14 @@ coff_symfile_read (struct objfile *objfile, int symfile_flags)
       char *debugfile;
 
       debugfile = find_separate_debug_file_by_debuglink (objfile);
+      make_cleanup (xfree, debugfile);
 
       if (debugfile)
 	{
 	  bfd *abfd = symfile_bfd_open (debugfile);
 
+	  make_cleanup_bfd_unref (abfd);
 	  symbol_file_add_separate (abfd, symfile_flags, objfile);
-	  xfree (debugfile);
 	}
     }
 
diff --git a/gdb/elfread.c b/gdb/elfread.c
index de61a9a..608a868 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1445,10 +1445,12 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags)
 
       if (debugfile)
 	{
+	  struct cleanup *cleanup = make_cleanup (xfree, debugfile);
 	  bfd *abfd = symfile_bfd_open (debugfile);
 
+	  make_cleanup_bfd_unref (abfd);
 	  symbol_file_add_separate (abfd, symfile_flags, objfile);
-	  xfree (debugfile);
+	  do_cleanups (cleanup);
 	}
     }
 
diff --git a/gdb/jit.c b/gdb/jit.c
index 410b94d..cdd9f49 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -896,7 +896,8 @@ JITed symbol file is not an object file, ignoring it.\n"));
         ++i;
       }
 
-  /* This call takes ownership of NBFD.  It does not take ownership of SAI.  */
+  /* This call does not take ownership of SAI.  */
+  make_cleanup_bfd_unref (nbfd);
   objfile = symbol_file_add_from_bfd (nbfd, 0, sai, OBJF_SHARED, NULL);
 
   do_cleanups (old_cleanups);
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 5b9e2ba..0d7578a 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -454,6 +454,7 @@ macho_add_oso_symfile (oso_el *oso, bfd *abfd,
   asymbol **symp;
   struct bfd_hash_table table;
   int nbr_sections;
+  struct cleanup *cleanup;
 
   /* Per section flag to mark which section have been rebased.  */
   unsigned char *sections_rebased;
@@ -631,11 +632,13 @@ macho_add_oso_symfile (oso_el *oso, bfd *abfd,
 
   /* We need to clear SYMFILE_MAINLINE to avoid interractive question
      from symfile.c:symbol_file_add_with_addrs_or_offsets.  */
+  cleanup = make_cleanup_bfd_unref (abfd);
   symbol_file_add_from_bfd
     (abfd, symfile_flags & ~(SYMFILE_MAINLINE | SYMFILE_VERBOSE), NULL,
      main_objfile->flags & (OBJF_REORDERED | OBJF_SHARED
 			    | OBJF_READNOW | OBJF_USERLOADED),
      main_objfile);
+  do_cleanups (cleanup);
 }
 
 /* Read symbols from the vector of oso files.  */
@@ -897,6 +900,7 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags)
 	  int ix;
 	  oso_el *oso;
           struct bfd_section *asect, *dsect;
+	  struct cleanup *cleanup;
 
 	  if (mach_o_debug_level > 0)
 	    printf_unfiltered (_("dsym file found\n"));
@@ -917,7 +921,9 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags)
             }
 
 	  /* Add the dsym file as a separate file.  */
+	  cleanup = make_cleanup_bfd_unref (dsym_bfd);
           symbol_file_add_separate (dsym_bfd, symfile_flags, objfile);
+	  do_cleanups (cleanup);
 
 	  /* Don't try to read dwarf2 from main file or shared libraries.  */
           return;
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 5ff0eb2..411618f 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -194,9 +194,10 @@ allocate_objfile (bfd *abfd, int flags)
 
   /* Update the per-objfile information that comes from the bfd, ensuring
      that any data that is reference is saved in the per-objfile data
-     region.  Note that we steal a reference to ABFD.  */
+     region.  */
 
   objfile->obfd = abfd;
+  gdb_bfd_ref (abfd);
   if (abfd != NULL)
     {
       /* Look up the gdbarch associated with the BFD.  */
diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c
index 140012b..8c3f546 100644
--- a/gdb/rs6000-nat.c
+++ b/gdb/rs6000-nat.c
@@ -798,7 +798,6 @@ add_vmap (LdInfo *ldi)
       gdb_bfd_unref (abfd);
       return NULL;
     }
-  gdb_bfd_ref (vp->bfd);
   obj = allocate_objfile (vp->bfd, 0);
   vp->objfile = obj;
 
diff --git a/gdb/solib.c b/gdb/solib.c
index 9779e10..73773f1 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -610,7 +610,6 @@ solib_read_symbols (struct so_list *so, int flags)
 
 	  sap = build_section_addr_info_from_section_table (so->sections,
 							    so->sections_end);
-	  gdb_bfd_ref (so->abfd);
 	  so->objfile = symbol_file_add_from_bfd (so->abfd,
 						  flags, sap, OBJF_SHARED,
 						  NULL);
diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c
index a17ff3f..999f1ab 100644
--- a/gdb/spu-linux-nat.c
+++ b/gdb/spu-linux-nat.c
@@ -374,8 +374,13 @@ spu_symbol_file_add_from_memory (int inferior_fd)
   /* Open BFD representing SPE executable and read its symbols.  */
   nbfd = spu_bfd_open (addr);
   if (nbfd)
-    symbol_file_add_from_bfd (nbfd, SYMFILE_VERBOSE | SYMFILE_MAINLINE,
-                              NULL, 0, NULL);
+    {
+      struct cleanup *cleanup = make_cleanup_bfd_unref (nbfd);
+
+      symbol_file_add_from_bfd (nbfd, SYMFILE_VERBOSE | SYMFILE_MAINLINE,
+				NULL, 0, NULL);
+      do_cleanups (cleanup);
+    }
 }
 
 
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index ad87abd..2e53be0 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -111,15 +111,14 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
       xfree (name);
     }
 
+  cleanup = make_cleanup_bfd_unref (nbfd);
+
   if (!bfd_check_format (nbfd, bfd_object))
-    {
-      make_cleanup_bfd_unref (nbfd);
-      error (_("Got object file from memory but can't read symbols: %s."),
-	     bfd_errmsg (bfd_get_error ()));
-    }
+    error (_("Got object file from memory but can't read symbols: %s."),
+	   bfd_errmsg (bfd_get_error ()));
 
   sai = alloc_section_addr_info (bfd_count_sections (nbfd));
-  cleanup = make_cleanup (xfree, sai);
+  make_cleanup (xfree, sai);
   i = 0;
   for (sec = nbfd->sections; sec != NULL; sec = sec->next)
     if ((bfd_get_section_flags (nbfd, sec) & (SEC_ALLOC|SEC_LOAD)) != 0)
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 64c4a3b..e1d5c15 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1037,7 +1037,7 @@ new_symfile_objfile (struct objfile *objfile, int add_flags)
    loaded file.
 
    ABFD is a BFD already open on the file, as from symfile_bfd_open.
-   This BFD will be closed on error, and is always consumed by this function.
+   A new reference is acquired by this function.
 
    ADD_FLAGS encodes verbosity, whether this is main symbol file or
    extra, such as dynamically loaded code, and what to do with breakpoins.
@@ -1061,7 +1061,6 @@ symbol_file_add_with_addrs_or_offsets (bfd *abfd,
                                        int flags, struct objfile *parent)
 {
   struct objfile *objfile;
-  struct cleanup *my_cleanups;
   const char *name = bfd_get_filename (abfd);
   const int from_tty = add_flags & SYMFILE_VERBOSE;
   const int mainline = add_flags & SYMFILE_MAINLINE;
@@ -1075,8 +1074,6 @@ symbol_file_add_with_addrs_or_offsets (bfd *abfd,
       add_flags &= ~SYMFILE_NO_READ;
     }
 
-  my_cleanups = make_cleanup_bfd_unref (abfd);
-
   /* Give user a chance to burp if we'd be
      interactively wiping out any existing symbols.  */
 
@@ -1087,7 +1084,6 @@ symbol_file_add_with_addrs_or_offsets (bfd *abfd,
     error (_("Not confirmed."));
 
   objfile = allocate_objfile (abfd, flags | (mainline ? OBJF_MAINLINE : 0));
-  discard_cleanups (my_cleanups);
 
   if (parent)
     add_separate_debug_objfile (objfile, parent);
@@ -1208,8 +1204,14 @@ struct objfile *
 symbol_file_add (char *name, int add_flags, struct section_addr_info *addrs,
 		 int flags)
 {
-  return symbol_file_add_from_bfd (symfile_bfd_open (name), add_flags, addrs,
+  bfd *bfd = symfile_bfd_open (name);
+  struct cleanup *cleanup = make_cleanup_bfd_unref (bfd);
+  struct objfile *objf;
+
+  objf = symbol_file_add_from_bfd (symfile_bfd_open (name), add_flags, addrs,
                                    flags, NULL);
+  do_cleanups (cleanup);
+  return objf;
 }
 
 
-- 
1.7.7.6


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]