RFA: fix Darwin bug, PR gdb/14290

[Tom Tromey <tromey at redhat dot com>]

This fixes the bug reported in PR gdb/14290.

The bug is a double free.  The problem is that gdb doesn't wrap
gdb_bfd_mach_o_fat_extract, which can return either a contained BFD or
its argument.

The fix is to write a wrapper for this functionality so that the BFD
reference counting is handled properly.  This required exposing a bit
more API from gdb_bfd.c.

One of the commenters on the bug reported that this worked.
I doubt he did a full test suite run, so perhaps someone could try that.

Tom

2012-11-27  Tom Tromey  <tromey@redhat.com>

	PR gdb/14290:
	* solib-darwin.c (gdb_bfd_mach_o_fat_extract): New function.
	(darwin_solib_get_all_image_info_addr_at_init, darwin_bfd_open):
	Use it.
	* gdb_bfd.h (gdb_bfd_mark_parent): Declare.
	* gdb_bfd.c (gdb_bfd_mark_parent): New function.
	(gdb_bfd_openr_next_archived_file): Use it.

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 2bcc4b4..f0e349b 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -499,24 +499,34 @@ gdb_bfd_openr_iovec (const char *filename, const char *target,
 
 /* See gdb_bfd.h.  */
 
+void
+gdb_bfd_mark_parent (bfd *child, bfd *parent)
+{
+  struct gdb_bfd_data *gdata;
+
+  gdb_bfd_ref (child);
+  /* No need to stash the filename here, because we also keep a
+     reference on the parent archive.  */
+
+  gdata = bfd_usrdata (child);
+  if (gdata->archive_bfd == NULL)
+    {
+      gdata->archive_bfd = parent;
+      gdb_bfd_ref (parent);
+    }
+  else
+    gdb_assert (gdata->archive_bfd == parent);
+}
+
+/* See gdb_bfd.h.  */
+
 bfd *
 gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous)
 {
   bfd *result = bfd_openr_next_archived_file (archive, previous);
 
   if (result)
-    {
-      struct gdb_bfd_data *gdata;
-
-      gdb_bfd_ref (result);
-      /* No need to stash the filename here, because we also keep a
-	 reference on the parent archive.  */
-
-      gdata = bfd_usrdata (result);
-      gdb_assert (gdata->archive_bfd == NULL || gdata->archive_bfd == archive);
-      gdata->archive_bfd = archive;
-      gdb_bfd_ref (archive);
-    }
+    gdb_bfd_mark_parent (result, archive);
 
   return result;
 }
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 5fd361c..bb70b27 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -50,6 +50,15 @@ void gdb_bfd_ref (struct bfd *abfd);
 
 void gdb_bfd_unref (struct bfd *abfd);
 
+/* Mark the CHILD BFD as being a member of PARENT.  Also, increment
+   the reference count of CHILD.  Calling this function ensures that
+   as along as CHILD remains alive, PARENT will as well.  Both CHILD
+   and PARENT must be non-NULL.  This can be called more than once
+   with the same arguments; but it is not allowed to call it for a
+   single CHILD with different values for PARENT.  */
+
+void gdb_bfd_mark_parent (bfd *child, bfd *parent);
+
 /* Try to read or map the contents of the section SECT.  If
    successful, the section data is returned and *SIZE is set to the
    size of the section data; this may not be the same as the size
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 4ea0722..735d1c3 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -348,6 +348,27 @@ darwin_special_symbol_handling (void)
 {
 }
 
+/* A wrapper for bfd_mach_o_fat_extract that handles reference
+   counting properly.  This will either return NULL, or return a new
+   reference to a BFD.  */
+
+static bfd *
+gdb_bfd_mach_o_fat_extract (bfd *abfd, bfd_format format,
+			    const bfd_arch_info_type *arch)
+{
+  bfd *result = bfd_mach_o_fat_extract (abfd, format, arch);
+
+  if (result == NULL)
+    return NULL;
+
+  if (result == abfd)
+    gdb_bfd_ref (result);
+  else
+    gdb_bfd_mark_parent (result, abfd);
+
+  return result;
+}
+
 /* Extract dyld_all_image_addr when the process was just created, assuming the
    current PC is at the entry of the dynamic linker.  */
 
@@ -377,12 +398,11 @@ darwin_solib_get_all_image_info_addr_at_init (struct darwin_info *info)
       bfd *sub;
 
       make_cleanup_bfd_unref (dyld_bfd);
-      sub = bfd_mach_o_fat_extract (dyld_bfd, bfd_object,
-				    gdbarch_bfd_arch_info (target_gdbarch ()));
+      sub = gdb_bfd_mach_o_fat_extract (dyld_bfd, bfd_object,
+					gdbarch_bfd_arch_info (target_gdbarch ()));
       if (sub)
 	{
 	  dyld_bfd = sub;
-	  gdb_bfd_ref (sub);
 	  make_cleanup_bfd_unref (sub);
 	}
       else
@@ -514,14 +534,16 @@ darwin_bfd_open (char *pathname)
   /* Open bfd for shared library.  */
   abfd = solib_bfd_fopen (found_pathname, found_file);
 
-  res = bfd_mach_o_fat_extract (abfd, bfd_object,
-				gdbarch_bfd_arch_info (target_gdbarch ()));
+  res = gdb_bfd_mach_o_fat_extract (abfd, bfd_object,
+				    gdbarch_bfd_arch_info (target_gdbarch ()));
   if (!res)
     {
       make_cleanup_bfd_unref (abfd);
       error (_("`%s': not a shared-library: %s"),
 	     bfd_get_filename (abfd), bfd_errmsg (bfd_get_error ()));
     }
+
+  gdb_bfd_unref (abfd);
   return res;
 }