[binutils-gdb] Allow BFD to recognize macOS universal libraries

[Tom Tromey <tromey at sourceware dot org>]

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

commit eac61af65bcd24a48633da375527eb3f36ab47ed
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Jun 28 08:02:42 2018 -0600

    Allow BFD to recognize macOS universal libraries
    
    Bug #13157 is about a gdb regression, where previously it could handle
    universal libraries, but now cannot.
    
    gdb isn't working for me on macOS for other reasons, so I wrote this
    small test program to show the problem:
    
        #include <config.h>
        #include <stdio.h>
        #include <stdlib.h>
        #include <bfd.h>
    
        void
        die (const char *what)
        {
          fprintf (stderr, "die: %s\n", what);
          exit (1);
        }
    
        int
        main (int argc, char **argv)
        {
          bfd *file = bfd_openr (argv[1], NULL);
          if (file == NULL)
    	die ("couldn't open");
    
          if (!bfd_check_format (file, bfd_archive))
    	die ("not an archive");
    
          printf ("yay\n");
    
          bfd_close (file);
          return 0;
        }
    
    Then I built a simple universal binary.  With git master BFD, I get:
    
        $ ./doit ./universal-exe
        die: not an archive
    
    Jeff Muizelaar tracked this down to the BFD change for PR binutils/21787.
    This patch changed bfd_generic_archive_p to sometimes reset the BFD's
    "format" field.
    
    However, simply changing bfd_generic_archive_p regressed the test case
    in that bug.
    
    Debugging PR binutils/21787 again, what I saw is that the mach-o
    universal binary support acts like a bfd_archive but does not provide
    a _close_and_cleanup function.  However, if a BFD appears as an
    archive member, it must always remove its own entry from its parent's
    map.  Otherwise, when the parent is destroyed, the already-destroyed
    child BFD will be referenced.  mach-o does not use the usual archive
    member support, so simply using _bfd_archive_close_and_cleanup (as
    other targets do) will not work.
    
    This patch fixes the problem by introducing a new
    _bfd_unlink_from_archive_parent function, then arranging for it to be
    called in the mach-o case.
    
    Ok?
    
    bfd/ChangeLog
    2018-07-02  Jeff Muizelaar  <jrmuizel@gmail.com>
    	    Tom Tromey  <tom@tromey.com>
    
    	PR 13157
    	PR 21787
    	* mach-o.c (bfd_mach_o_fat_close_and_cleanup): New function.
    	(bfd_mach_o_close_and_cleanup): Redefine.
    	* archive.c (_bfd_unlink_from_archive_parent): New function,
    	extracted from..
    	(_bfd_archive_close_and_cleanup): ..here.
    	(bfd_generic_archive_p): Do not clear archive's format.
    	* libbfd-in.h (_bfd_unlink_from_archive_parent): Declare.
    	* libbfd.h: Regenerate.

Diff:
---
 bfd/ChangeLog   | 14 ++++++++++++++
 bfd/archive.c   | 44 +++++++++++++++++++++++++-------------------
 bfd/libbfd-in.h |  1 +
 bfd/libbfd.h    |  1 +
 bfd/mach-o.c    |  9 ++++++++-
 5 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 96d7a4d..a7d8ce7 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,17 @@
+2018-07-02  Jeff Muizelaar  <jrmuizel@gmail.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	PR 13157
+	PR 21787
+	* mach-o.c (bfd_mach_o_fat_close_and_cleanup): New function.
+	(bfd_mach_o_close_and_cleanup): Redefine.
+	* archive.c (_bfd_unlink_from_archive_parent): New function,
+	extracted from..
+	(_bfd_archive_close_and_cleanup): ..here.
+	(bfd_generic_archive_p): Do not clear archive's format.
+	* libbfd-in.h (_bfd_unlink_from_archive_parent): Declare.
+	* libbfd.h: Regenerate.
+
 2018-07-02  Thomas Preud'homme  <thomas.preudhomme@arm.com>
 
 	* archures.c (bfd_mach_arm_5TEJ, bfd_mach_arm_6, bfd_mach_arm_6KZ,
diff --git a/bfd/archive.c b/bfd/archive.c
index d56bb06..a51c135 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -850,8 +850,6 @@ bfd_generic_archive_p (bfd *abfd)
       && ! bfd_is_thin_archive (abfd))
     {
       bfd_set_error (bfd_error_wrong_format);
-      if (abfd->format == bfd_archive)
-	abfd->format = bfd_unknown;
       return NULL;
     }
 
@@ -2765,6 +2763,30 @@ archive_close_worker (void **slot, void *inf ATTRIBUTE_UNUSED)
   return 1;
 }
 
+void
+_bfd_unlink_from_archive_parent (bfd *abfd)
+{
+  if (arch_eltdata (abfd) != NULL)
+    {
+      struct areltdata *ared = arch_eltdata (abfd);
+      htab_t htab = (htab_t) ared->parent_cache;
+
+      if (htab)
+	{
+	  struct ar_cache ent;
+	  void **slot;
+
+	  ent.ptr = ared->key;
+	  slot = htab_find_slot (htab, &ent, NO_INSERT);
+	  if (slot != NULL)
+	    {
+	      BFD_ASSERT (((struct ar_cache *) *slot)->arbfd == abfd);
+	      htab_clear_slot (htab, slot);
+	    }
+	}
+    }
+}
+
 bfd_boolean
 _bfd_archive_close_and_cleanup (bfd *abfd)
 {
@@ -2789,25 +2811,9 @@ _bfd_archive_close_and_cleanup (bfd *abfd)
 	  bfd_ardata (abfd)->cache = NULL;
 	}
     }
-  if (arch_eltdata (abfd) != NULL)
-    {
-      struct areltdata *ared = arch_eltdata (abfd);
-      htab_t htab = (htab_t) ared->parent_cache;
 
-      if (htab)
-	{
-	  struct ar_cache ent;
-	  void **slot;
+  _bfd_unlink_from_archive_parent (abfd);
 
-	  ent.ptr = ared->key;
-	  slot = htab_find_slot (htab, &ent, NO_INSERT);
-	  if (slot != NULL)
-	    {
-	      BFD_ASSERT (((struct ar_cache *) *slot)->arbfd == abfd);
-	      htab_clear_slot (htab, slot);
-	    }
-	}
-    }
   if (abfd->is_linker_output)
     (*abfd->link.hash->hash_table_free) (abfd);
 
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 3c55adf..e53b255 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -277,6 +277,7 @@ extern int bfd_generic_stat_arch_elt
 #define _bfd_generic_close_and_cleanup _bfd_archive_close_and_cleanup
 extern bfd_boolean _bfd_archive_close_and_cleanup
   (bfd *) ATTRIBUTE_HIDDEN;
+extern void _bfd_unlink_from_archive_parent (bfd *) ATTRIBUTE_HIDDEN;
 #define _bfd_generic_bfd_free_cached_info _bfd_bool_bfd_true
 extern bfd_boolean _bfd_generic_new_section_hook
   (bfd *, asection *) ATTRIBUTE_HIDDEN;
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 85f61b2..a884aab 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -282,6 +282,7 @@ extern int bfd_generic_stat_arch_elt
 #define _bfd_generic_close_and_cleanup _bfd_archive_close_and_cleanup
 extern bfd_boolean _bfd_archive_close_and_cleanup
   (bfd *) ATTRIBUTE_HIDDEN;
+extern void _bfd_unlink_from_archive_parent (bfd *) ATTRIBUTE_HIDDEN;
 #define _bfd_generic_bfd_free_cached_info _bfd_bool_bfd_true
 extern bfd_boolean _bfd_generic_new_section_hook
   (bfd *, asection *) ATTRIBUTE_HIDDEN;
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index d58e62d..ce72a3d 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -5532,6 +5532,13 @@ bfd_mach_o_fat_extract (bfd *abfd,
   return NULL;
 }
 
+static bfd_boolean
+bfd_mach_o_fat_close_and_cleanup (bfd *abfd)
+{
+  _bfd_unlink_from_archive_parent (abfd);
+  return TRUE;
+}
+
 int
 bfd_mach_o_lookup_command (bfd *abfd,
 			   bfd_mach_o_load_command_type type,
@@ -6026,7 +6033,7 @@ bfd_mach_o_free_cached_info (bfd *abfd)
 /* Not yet handled: creating an archive.  */
 #define bfd_mach_o_mkarchive			  _bfd_noarchive_mkarchive
 
-#define bfd_mach_o_close_and_cleanup		  _bfd_bool_bfd_true
+#define bfd_mach_o_close_and_cleanup		  bfd_mach_o_fat_close_and_cleanup
 
 /* Not used.  */
 #define bfd_mach_o_generic_stat_arch_elt	  bfd_mach_o_fat_stat_arch_elt