Re: RFC: [PATCH] New attempt at fixing MIPS --gc-sections et al.

[Alan Modra <amodra at bigpond dot net dot au>]

On Wed, Aug 17, 2005 at 11:18:57AM +0930, Alan Modra wrote:
> Hmm, I see elf_gc_sweep_symbol won't do the right thing for shared libs,
> so I'd better fix that now that we are supposed to support --gc-sections
> when generating shared libs.  I think elf_gc_sweep_symbol ought to call
> elf_backend_hide_symbol for symbols in removed sections, and leave the
> renumbering to _bfd_elf_link_renumber_dynsyms as you do in your patch.
> Fixing..

	* elflink.c (_bfd_elf_define_linkage_sym): Don't call
	bfd_elf_link_record_dynamic_symbol.  Call elf_backend_hide_symbol.
	(_bfd_elf_link_renumber_dynsyms): Formatting.
	(struct elf_gc_sweep_symbol_info): New.
	(elf_gc_sweep_symbol): Rewrite.
	(elf_gc_sweep): Adjust params and elf_gc_sweep_symbol call.
	Call _bfd_elf_link_renumber_dynsyms.
	(bfd_elf_gc_sections): Adjust elf_gc_sweep call.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.188
diff -u -p -r1.188 elflink.c
--- bfd/elflink.c	15 Aug 2005 15:39:07 -0000	1.188
+++ bfd/elflink.c	17 Aug 2005 02:58:20 -0000
@@ -37,6 +37,7 @@ _bfd_elf_define_linkage_sym (bfd *abfd,
 {
   struct elf_link_hash_entry *h;
   struct bfd_link_hash_entry *bh;
+  const struct elf_backend_data *bed;
 
   h = elf_link_hash_lookup (elf_hash_table (info), name, FALSE, FALSE, FALSE);
   if (h != NULL)
@@ -59,10 +60,8 @@ _bfd_elf_define_linkage_sym (bfd *abfd,
   h->type = STT_OBJECT;
   h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
 
-  if (!info->executable
-      && !bfd_elf_link_record_dynamic_symbol (info, h))
-    return NULL;
-
+  bed = get_elf_backend_data (abfd);
+  (*bed->elf_backend_hide_symbol) (info, h, TRUE);
   return h;
 }
 
@@ -736,7 +735,8 @@ _bfd_elf_link_renumber_dynsyms (bfd *out
   if (dynsymcount != 0)
     ++dynsymcount;
 
-  return elf_hash_table (info)->dynsymcount = dynsymcount;
+  elf_hash_table (info)->dynsymcount = dynsymcount;
+  return dynsymcount;
 }
 
 /* This function is called when we want to define a new symbol.  It
@@ -8852,19 +8852,26 @@ _bfd_elf_gc_mark (struct bfd_link_info *
 
 /* 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 *idxptr)
+elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)
 {
-  int *idx = idxptr;
-
   if (h->root.type == bfd_link_hash_warning)
     h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
-  if (h->dynindx != -1
-      && ((h->root.type != bfd_link_hash_defined
-	   && h->root.type != bfd_link_hash_defweak)
-	  || h->root.u.def.section->gc_mark))
-    h->dynindx = (*idx)++;
+  if ((h->root.type == bfd_link_hash_defined
+       || h->root.type == bfd_link_hash_defweak)
+      && !h->root.u.def.section->gc_mark
+      && !(h->root.u.def.section->owner->flags & DYNAMIC))
+    {
+      struct elf_gc_sweep_symbol_info *inf = data;
+      (*inf->hide_symbol) (inf->info, h, TRUE);
+    }
 
   return TRUE;
 }
@@ -8875,9 +8882,13 @@ typedef bfd_boolean (*gc_sweep_hook_fn)
   (bfd *, struct bfd_link_info *, asection *, const Elf_Internal_Rela *);
 
 static bfd_boolean
-elf_gc_sweep (struct bfd_link_info *info, gc_sweep_hook_fn gc_sweep_hook)
+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)
     {
@@ -8934,20 +8945,12 @@ elf_gc_sweep (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?  */
-  {
-    int i = 0;
-
-    elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol, &i);
-
-    /* There is an unused NULL entry at the head of the table which
-       we must account for in our count.  Unless there weren't any
-       symbols, which means we'll have no table at all.  */
-    if (i != 0)
-      ++i;
-
-    elf_hash_table (info)->dynsymcount = i;
-  }
+  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;
 }
 
@@ -9184,10 +9187,7 @@ bfd_elf_gc_sections (bfd *abfd, struct b
     }
 
   /* ... and mark SEC_EXCLUDE for those that go.  */
-  if (!elf_gc_sweep (info, get_elf_backend_data (abfd)->gc_sweep_hook))
-    return FALSE;
-
-  return TRUE;
+  return elf_gc_sweep (abfd, info);
 }
 
 /* Called from check_relocs to record the existence of a VTINHERIT reloc.  */

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre