Re: Solibs and objfile BFD ownership

[Paul Pluzhnikov <ppluzhnikov@google.com>]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On Tue, Aug 4, 2009 at 7:53 AM, Daniel Jacobowitz<drow@false.org> wrote:

> A comment somewhere about the usage of the usrdata field would be
> nice.  I gather that NULL and 1 behave the same?

Yes. I added comment to gdb_bfd_unref.
Updated patch attached. Tested on Linux/x86_64 with no new failures.

Daniel, could you confirm that this fixes the ARM failures?

Thanks,
-- 
Paul Pluzhnikov

2009-08-04  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * objfiles.h (OBJF_KEEPBFD): Delete.
       (gdb_bfd_unref): New prototype.
       * objfiles.c (gdb_bfd_unref): New function.
       (free_objfile): Call gdb_bfd_unref.
       * solib.c (free_so): Likewise.
       (symbol_add_stub): Set refcount.

[-- Attachment #2: gdb-bfd-ownership-20090804.txt --]
[-- Type: text/plain, Size: 4357 bytes --]

Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.60
diff -u -p -u -r1.60 objfiles.h
--- objfiles.h	22 Jul 2009 19:21:31 -0000	1.60
+++ objfiles.h	4 Aug 2009 17:22:33 -0000
@@ -414,12 +414,6 @@ struct objfile
 
 #define OBJF_USERLOADED	(1 << 3)	/* User loaded */
 
-/* The bfd of this objfile is used outside of the objfile (e.g. by solib).
-   Do not try to free it.  */
-
-#define OBJF_KEEPBFD	(1 << 4)	/* Do not delete bfd */
-
-
 /* The object file that the main symbol table was loaded from (e.g. the
    argument to the "symbol-file" or "file" command).  */
 
@@ -510,6 +504,8 @@ extern void set_objfile_data (struct obj
 			      const struct objfile_data *data, void *value);
 extern void *objfile_data (struct objfile *objfile,
 			   const struct objfile_data *data);
+
+extern void gdb_bfd_unref (struct bfd *abfd);
 \f
 
 /* Traverse all object files.  ALL_OBJFILES_SAFE works even if you delete
Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.88
diff -u -p -u -r1.88 objfiles.c
--- objfiles.c	28 Jul 2009 16:39:06 -0000	1.88
+++ objfiles.c	4 Aug 2009 17:22:33 -0000
@@ -452,16 +452,7 @@ free_objfile (struct objfile *objfile)
   /* Discard any data modules have associated with the objfile.  */
   objfile_free_data (objfile);
 
-  /* We always close the bfd, unless the OBJF_KEEPBFD flag is set.  */
-
-  if (objfile->obfd != NULL && !(objfile->flags & OBJF_KEEPBFD))
-    {
-      char *name = bfd_get_filename (objfile->obfd);
-      if (!bfd_close (objfile->obfd))
-	warning (_("cannot close \"%s\": %s"),
-		 name, bfd_errmsg (bfd_get_error ()));
-      xfree (name);
-    }
+  gdb_bfd_unref (objfile->obfd);
 
   /* Remove it from the chain of all objfiles. */
 
@@ -1020,3 +1011,35 @@ objfiles_changed (void)
 {
   objfiles_changed_p = 1;  /* Rebuild section map next time we need it.  */
 }
+
+/* Unreference and possibly close abfd.  */
+void
+gdb_bfd_unref (struct bfd *abfd)
+{
+  int *p_refcount;
+  char *name;
+
+  if (abfd == NULL)
+    return;
+
+  p_refcount = abfd->usrdata;
+
+  /* Valid range for p_refcount: NULL (single owner), or a pointer
+     to int counter, which has a value of 1 (single owner) or 2 (shared).  */
+  gdb_assert (p_refcount == NULL || *p_refcount == 1 || *p_refcount == 2);
+
+  if (p_refcount != NULL)
+    {
+      *p_refcount -= 1;
+      if (*p_refcount > 0)
+	return;
+    }
+  xfree (p_refcount);
+  abfd->usrdata = NULL;  /* Paranoia.  */
+
+  name = bfd_get_filename (abfd);
+  if (!bfd_close (abfd))
+    warning (_("cannot close \"%s\": %s"),
+	     name, bfd_errmsg (bfd_get_error ()));
+  xfree (name);
+}
Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.122
diff -u -p -u -r1.122 solib.c
--- solib.c	17 Jul 2009 17:08:23 -0000	1.122
+++ solib.c	4 Aug 2009 17:22:33 -0000
@@ -421,21 +421,11 @@ void
 free_so (struct so_list *so)
 {
   struct target_so_ops *ops = solib_ops (target_gdbarch);
-  char *bfd_filename = 0;
 
   if (so->sections)
     xfree (so->sections);
-      
-  if (so->abfd)
-    {
-      bfd_filename = bfd_get_filename (so->abfd);
-      if (! bfd_close (so->abfd))
-	warning (_("cannot close \"%s\": %s"),
-		 bfd_filename, bfd_errmsg (bfd_get_error ()));
-    }
 
-  if (bfd_filename)
-    xfree (bfd_filename);
+  gdb_bfd_unref (so->abfd);
 
   ops->free_so (so);
 
@@ -454,6 +444,7 @@ static void
 symbol_add_stub (struct so_list *so, int flags)
 {
   struct section_addr_info *sap;
+  int *p_refcount;
 
   /* Have we already loaded this shared object?  */
   ALL_OBJFILES (so->objfile)
@@ -465,8 +456,11 @@ symbol_add_stub (struct so_list *so, int
   sap = build_section_addr_info_from_section_table (so->sections,
                                                     so->sections_end);
 
-  so->objfile = symbol_file_add_from_bfd (so->abfd, flags,
-					  sap, OBJF_SHARED | OBJF_KEEPBFD);
+  so->objfile = symbol_file_add_from_bfd (so->abfd, flags, sap, OBJF_SHARED);
+  p_refcount = xmalloc (sizeof (*p_refcount));
+  *p_refcount = 2;  /* Both solib and objfile refer to this abfd.  */
+  so->abfd->usrdata = p_refcount;
+
   free_section_addr_info (sap);
 
   return;