[binutils-gdb] PR ld/20828: Reorder the symbol sweep stage of section GC

[Maciej W.Rozycki <macro at sourceware dot org>]

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

commit b531344c34b05fcd55ce65776ff50b9a752d60c1
Author: Maciej W. Rozycki <macro@imgtec.com>
Date:   Mon Jan 23 11:38:20 2017 +0000

    PR ld/20828: Reorder the symbol sweep stage of section GC
    
    Complement commit 81ff47b3a546 ("PR ld/20828: Fix linker script symbols
    wrongly forced local with section GC") and move the symbol sweep stage
    of section GC from `elf_gc_sweep' to `bfd_elf_size_dynamic_sections',
    avoiding the need to clear the `forced_local' marker, problematic for
    targets that have special processing in their `elf_backend_hide_symbol'
    handler.  Set `mark' instead in `bfd_elf_record_link_assignment' and,
    matching changes from commit 3bd43ebcb602 ("ld --gc-sections fail with
    __tls_get_addr_opt"), also in PowerPC `__tls_get_addr_opt' handling
    code, removing a:
    
    FAIL: PR ld/20828 dynamic symbols with section GC (version script)
    
    test suite failure with the `score-elf' target.
    
    The rationale is it is enough if symbols are swept at the beginning of
    `bfd_elf_size_dynamic_sections' as it is only in this function that the
    size of the GOT, the dynamic symbol table and other dynamic sections is
    determined, which will depend on the number of symbols making it to the
    dynamic symbol table.  It is also appropriate to do the sweep at this
    point as it is already after any changes have been made to symbols with
    `bfd_elf_record_link_assignment', and not possible any earlier as calls
    to that function are only made just beforehand -- barring audit entry
    processing -- via `gld${EMULATION_NAME}_find_statement_assignment'
    invoked from `gld${EMULATION_NAME}_before_allocation' which is the ELF
    handler for `ldemul_before_allocation'.
    
    	bfd/
    	PR ld/20828
    	* elflink.c (bfd_elf_record_link_assignment): Revert last
    	change and don't ever clear `forced_local'.  Set `mark'
    	unconditionally.
    	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
    	file.
    	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
    	(bfd_elf_size_dynamic_sections): ... here.
    	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
    	and set `mark' instead in `__tls_get_addr_opt' processing.
    	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.

Diff:
---
 bfd/ChangeLog   |  14 ++++++++
 bfd/elf32-ppc.c |   2 +-
 bfd/elf64-ppc.c |   4 +--
 bfd/elflink.c   | 105 +++++++++++++++++++++++++++++---------------------------
 4 files changed, 71 insertions(+), 54 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 5102221..c26ab49 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,17 @@
+2017-01-24  Maciej W. Rozycki  <macro@imgtec.com>
+
+	PR ld/20828
+	* elflink.c (bfd_elf_record_link_assignment): Revert last
+	change and don't ever clear `forced_local'.  Set `mark'
+	unconditionally.
+	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
+	file.
+	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
+	(bfd_elf_size_dynamic_sections): ... here.
+	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
+	and set `mark' instead in `__tls_get_addr_opt' processing.
+	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.
+
 2017-01-24  Alan Modra  <amodra@gmail.com>
 
 	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Merge two cases
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 7d69ea0..353845a 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -5287,7 +5287,7 @@ ppc_elf_tls_setup (bfd *obfd, struct bfd_link_info *info)
 		  tga->root.type = bfd_link_hash_indirect;
 		  tga->root.u.i.link = &opt->root;
 		  ppc_elf_copy_indirect_symbol (info, opt, tga);
-		  opt->forced_local = 0;
+		  opt->mark = 1;
 		  if (opt->dynindx != -1)
 		    {
 		      /* Use __tls_get_addr_opt in dynamic relocations.  */
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 1e9429c..1c77cc6 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -8324,7 +8324,7 @@ ppc64_elf_tls_setup (struct bfd_link_info *info)
 		  tga_fd->root.type = bfd_link_hash_indirect;
 		  tga_fd->root.u.i.link = &opt_fd->root;
 		  ppc64_elf_copy_indirect_symbol (info, opt_fd, tga_fd);
-		  opt_fd->forced_local = 0;
+		  opt_fd->mark = 1;
 		  if (opt_fd->dynindx != -1)
 		    {
 		      /* Use __tls_get_addr_opt in dynamic relocations.  */
@@ -8341,7 +8341,7 @@ ppc64_elf_tls_setup (struct bfd_link_info *info)
 		      tga->root.type = bfd_link_hash_indirect;
 		      tga->root.u.i.link = &opt->root;
 		      ppc64_elf_copy_indirect_symbol (info, opt, tga);
-		      opt->forced_local = 0;
+		      opt->mark = 1;
 		      _bfd_elf_link_hash_hide_symbol (info, opt,
 						      tga->forced_local);
 		      htab->tls_get_addr = (struct ppc_link_hash_entry *) opt;
diff --git a/bfd/elflink.c b/bfd/elflink.c
index d7ed8ce..9c71d57 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -671,17 +671,15 @@ bfd_elf_record_link_assignment (bfd *output_bfd,
 
   /* If this symbol is not being provided by the linker script, and it is
      currently defined by a dynamic object, but not by a regular object,
-     then undo any forced local marking that may have been set by input
-     section garbage collection and clear out any version information
-     because the symbol will not be associated with the dynamic object
-     any more.  */
+     then clear out any version information because the symbol will not be
+     associated with the dynamic object any more.  */
   if (!provide
       && h->def_dynamic
       && !h->def_regular)
-    {
-      h->forced_local = 0;
-      h->verinfo.verdef = NULL;
-    }
+    h->verinfo.verdef = NULL;
+
+  /* Make sure this symbol is not garbage collected.  */
+  h->mark = 1;
 
   h->def_regular = 1;
 
@@ -5876,6 +5874,38 @@ bfd_elf_stack_segment_size (bfd *output_bfd,
   return TRUE;
 }
 
+/* Sweep symbols in swept sections.  Called via elf_link_hash_traverse.  */
+
+struct elf_gc_sweep_symbol_info
+{
+  struct bfd_link_info *info;
+  void (*hide_symbol) (struct bfd_link_info *, struct elf_link_hash_entry *,
+		       bfd_boolean);
+};
+
+static bfd_boolean
+elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)
+{
+  if (!h->mark
+      && (((h->root.type == bfd_link_hash_defined
+	    || h->root.type == bfd_link_hash_defweak)
+	   && !((h->def_regular || ELF_COMMON_DEF_P (h))
+		&& h->root.u.def.section->gc_mark))
+	  || h->root.type == bfd_link_hash_undefined
+	  || h->root.type == bfd_link_hash_undefweak))
+    {
+      struct elf_gc_sweep_symbol_info *inf;
+
+      inf = (struct elf_gc_sweep_symbol_info *) data;
+      (*inf->hide_symbol) (inf->info, h, TRUE);
+      h->def_regular = 0;
+      h->ref_regular = 0;
+      h->ref_regular_nonweak = 0;
+    }
+
+  return TRUE;
+}
+
 /* Set up the sizes and contents of the ELF dynamic sections.  This is
    called by the ELF linker emulation before_allocation routine.  We
    must set the sizes of the sections before the linker sets the
@@ -5906,6 +5936,22 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 
   bed = get_elf_backend_data (output_bfd);
 
+  if (info->gc_sections && bed->can_gc_sections)
+    {
+      struct elf_gc_sweep_symbol_info sweep_info;
+      unsigned long section_sym_count;
+
+      /* Remove the symbols that were in the swept sections from the
+	 dynamic symbol table.  GCFIXME: Anyone know how to get them
+	 out of the static symbol table as well?  */
+      sweep_info.info = info;
+      sweep_info.hide_symbol = bed->elf_backend_hide_symbol;
+      elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol,
+			      &sweep_info);
+
+      _bfd_elf_link_renumber_dynsyms (output_bfd, info, &section_sym_count);
+    }
+
   /* Any syms created from now on start with -1 in
      got.refcount/offset and plt.refcount/offset.  */
   elf_hash_table (info)->init_got_refcount
@@ -12853,38 +12899,6 @@ _bfd_elf_gc_mark_extra_sections (struct bfd_link_info *info,
   return TRUE;
 }
 
-/* Sweep symbols in swept sections.  Called via elf_link_hash_traverse.  */
-
-struct elf_gc_sweep_symbol_info
-{
-  struct bfd_link_info *info;
-  void (*hide_symbol) (struct bfd_link_info *, struct elf_link_hash_entry *,
-		       bfd_boolean);
-};
-
-static bfd_boolean
-elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)
-{
-  if (!h->mark
-      && (((h->root.type == bfd_link_hash_defined
-	    || h->root.type == bfd_link_hash_defweak)
-	   && !((h->def_regular || ELF_COMMON_DEF_P (h))
-		&& h->root.u.def.section->gc_mark))
-	  || h->root.type == bfd_link_hash_undefined
-	  || h->root.type == bfd_link_hash_undefweak))
-    {
-      struct elf_gc_sweep_symbol_info *inf;
-
-      inf = (struct elf_gc_sweep_symbol_info *) data;
-      (*inf->hide_symbol) (inf->info, h, TRUE);
-      h->def_regular = 0;
-      h->ref_regular = 0;
-      h->ref_regular_nonweak = 0;
-    }
-
-  return TRUE;
-}
-
 /* The sweep phase of garbage collection.  Remove all garbage sections.  */
 
 typedef bfd_boolean (*gc_sweep_hook_fn)
@@ -12896,8 +12910,6 @@ elf_gc_sweep (bfd *abfd, struct bfd_link_info *info)
   bfd *sub;
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   gc_sweep_hook_fn gc_sweep_hook = bed->gc_sweep_hook;
-  unsigned long section_sym_count;
-  struct elf_gc_sweep_symbol_info sweep_info;
 
   for (sub = info->input_bfds; sub != NULL; sub = sub->link.next)
     {
@@ -12963,15 +12975,6 @@ elf_gc_sweep (bfd *abfd, struct bfd_link_info *info)
 	}
     }
 
-  /* Remove the symbols that were in the swept sections from the dynamic
-     symbol table.  GCFIXME: Anyone know how to get them out of the
-     static symbol table as well?  */
-  sweep_info.info = info;
-  sweep_info.hide_symbol = bed->elf_backend_hide_symbol;
-  elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol,
-			  &sweep_info);
-
-  _bfd_elf_link_renumber_dynsyms (abfd, info, &section_sym_count);
   return TRUE;
 }