[PATCH,plugins,head+2.21] Handle COMDAT groups on ELF.

[Dave Korn <dave dot korn dot cygwin at gmail dot com>]

    Hi again,

  One of the advantages of the two-stage-link tear-down-and-rebuild approach
was that we got COMDAT group selection correct for free, as a consequence of
resetting the already-linked table.  Since we're not doing that now, we have
to handle COMDAT in the linker plugin code by creating synthetic .group
sections in the dummy IR BFD, but making sure that these sections don't
actually win in group selection decisions.

  That's what this patch implements, and on x86_64-linux, with this, we now
get a completely clean GCC testrun (entire c-family and fortran), which brings
ld.bfd into parity with ld.gold.  :-)

bfd/ChangeLog:

2011-05-07  Dave Korn  <dave.korn.cygwin@...

	* elflink.c (elf_link_add_object_symbols): Don't mark symbols undef
	when the kept section comes from a plugin BFD that we expect to be
	replaced later.
	(_bfd_elf_section_already_linked): When there is a duplicate section,
	but the initial kept section comes from a plugin BFD and the newer
	duplicate section does not, keep the newer one and discard the initial
	kept section.

ld/ChangeLog:

	* plugin.c (struct group_sec_hash_table): New derived hash table type.
	(struct group_sec_hash_entry): Derived entry type for the above.
	(group_sec_hash_newfunc): Derived ctor for the table entries creates
	an ELF comdat group section with the given signature.
	(init_group_sec_hash_table): Derived ctor for the table.
	(group_sec_hash_lookup): Derived lookup function.
	(group_sec_hash_traverse): Derived traverse function.
	(group_sec_hash_table_free): Derived dtor.
	(struct add_syms_info): Helper struct to wrap args to callback.
	(get_elf_comdat_section): New helper function for ELF targets.
	(get_comdat_section): Likewise for all targets.
	(asymbol_from_plugin_symbol): Take an add_syms_info rather than a
	bfd pointer argument and adjust usage accordingly.  Generate comdat
	sections or groups for defined symbols with non-NULL comdat_key.
	(add_symbols):  Wrap parameters to pass down in an add_syms_info, and
	handle ELF comdat group section creation.

  Tested on x86_64-unknown-linux-gnu as mentioned.  I'm going to start running
a GCC testsuite on i686-pc-cygwin, but that could take a couple of days for
the final results to come in.  If this patch is OK, I'll wait until that's got
far enough to be sure it's working before I commit.

  Any comments on the implementation, while we're waiting?  Or OK as it is?

    cheers,
      DaveK

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.403
diff -p -u -r1.403 elflink.c
--- bfd/elflink.c	7 May 2011 14:12:59 -0000	1.403
+++ bfd/elflink.c	7 May 2011 14:30:34 -0000
@@ -3923,7 +3923,8 @@ error_free_dyn:
 	  sec = bfd_section_from_elf_index (abfd, isym->st_shndx);
 	  if (sec == NULL)
 	    sec = bfd_abs_section_ptr;
-	  else if (sec->kept_section)
+	  else if (sec->kept_section
+		   && !(sec->kept_section->owner->flags & BFD_PLUGIN))
 	    {
 	      /* Symbols from discarded section are undefined.  We keep
 		 its visibility.  */
@@ -12487,6 +12488,7 @@ _bfd_elf_section_already_linked (bfd *ab
 
   for (l = already_linked_list->entry; l != NULL; l = l->next)
     {
+      bfd_boolean keep_newer = FALSE;
       /* We may have 2 different types of sections on the list: group
 	 sections and linkonce sections.  Match like sections.  */
       if ((flags & SEC_GROUP) == (l->sec->flags & SEC_GROUP)
@@ -12501,6 +12503,10 @@ _bfd_elf_section_already_linked (bfd *ab
 	      abort ();
 
 	    case SEC_LINK_DUPLICATES_DISCARD:
+	      /* If the group section we linked first time comes from a
+		 linker plugin dummy BFD, keep this newer section instead.  */
+	      keep_newer = ((l->sec->owner->flags & BFD_PLUGIN) != 0)
+			   && ((sec->owner->flags & BFD_PLUGIN) == 0);
 	      break;
 
 	    case SEC_LINK_DUPLICATES_ONE_ONLY:
@@ -12547,6 +12553,20 @@ _bfd_elf_section_already_linked (bfd *ab
 	      break;
 	    }
 
+	  /* If we are going to keep the newer section, update the already
+	     linked table to record that as the kept section, and discard
+	     all the sections from the original group instead of this one.  */
+	  if (keep_newer)
+	    {
+	      asection *t;
+	      /* Changing sec here so that the loop below now discards
+		 the old group members instead of the new one is safe
+		 because we're going to return immediately after.  */
+	      t = sec;
+	      sec = l->sec;
+	      l->sec = t;
+	    }
+
 	  /* Set the output_section field so that lang_add_section
 	     does not create a lang_input_section structure for this
 	     section.  Since there might be a symbol in the section
Index: ld/plugin.c
===================================================================
RCS file: /cvs/src/src/ld/plugin.c,v
retrieving revision 1.33
diff -p -u -r1.33 plugin.c
--- ld/plugin.c	24 Apr 2011 10:02:14 -0000	1.33
+++ ld/plugin.c	7 May 2011 14:30:34 -0000
@@ -265,14 +265,222 @@ is_ir_dummy_bfd (const bfd *abfd)
 }
 
 /* Helpers to convert between BFD and GOLD symbol formats.  */
+
+struct group_sec_hash_table
+{
+  struct bfd_hash_table table;
+  /* Count of total group sections = unique comdat keys.  */
+  int n_comdat_keys;
+  /* BFD that we are creating.  */
+  bfd *abfd;
+};
+
+struct group_sec_hash_entry
+{
+  /* Hash table keys are comdat_key values.  */
+  struct bfd_hash_entry entry;
+  /* We have one .group section for each.  */
+  struct bfd_section *sec;
+  /* COMDAT group name symbol index.  */
+  int n_symndx;
+  /* Counts the number of separate sections in this group.  */
+  unsigned int sec_count;
+  /* Tail of the circular list.  */
+  struct bfd_section *last_in_group;
+};
+
+static struct bfd_hash_entry *
+group_sec_hash_newfunc (struct bfd_hash_entry *entry,
+			struct bfd_hash_table *table,
+			const char *string)
+{
+  struct group_sec_hash_entry *ret = (struct group_sec_hash_entry *) entry;
+  struct group_sec_hash_table *tbl = (struct group_sec_hash_table *) table;
+  flagword flg = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP
+		 | SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
+
+ /* Allocate the structure if it has not already been allocated by a
+    derived class.  */
+  if (ret == NULL)
+    {
+      ret = bfd_hash_allocate (table, sizeof *ret);
+      if (ret == NULL)
+        return NULL;
+    }
+
+ /* Call the allocation method of the base class.  */
+  ret = ((struct group_sec_hash_entry *)
+	bfd_hash_newfunc (&ret->entry, table, string));
+
+ /* Initialize the local fields here.  */
+  ret->sec = bfd_make_section_anyway_with_flags (tbl->abfd, ".group", flg);
+  ret->sec->output_section = ret->sec;
+  ret->sec->output_offset = 0;
+  ret->sec_count = 0;
+  elf_section_type (ret->sec) = SHT_GROUP;
+  elf_section_data (ret->sec)->this_hdr.sh_info = 0;
+  elf_sec_group (ret->sec) = NULL;
+  elf_next_in_group (ret->sec) = NULL;
+  /* Will fill in these two later when signature sym is created.  */
+  ret->n_symndx = -1;
+  elf_group_id (ret->sec) = NULL;
+  /* Update count of entries in the owning table.  */
+  tbl->n_comdat_keys++;
+  return &ret->entry;
+}
+
+static bfd_boolean
+init_group_sec_hash_table (struct group_sec_hash_table *tbl, bfd *abfd)
+{
+  if (!bfd_hash_table_init (&tbl->table, group_sec_hash_newfunc, sizeof *tbl))
+      return FALSE;
+  tbl->n_comdat_keys = 0;
+  tbl->abfd = abfd;
+  return TRUE;
+}
+
+static struct group_sec_hash_entry *
+group_sec_hash_lookup (struct group_sec_hash_table *tbl,
+		       const char *name,
+		       bfd_boolean create,
+		       bfd_boolean copy)
+{
+  return (struct group_sec_hash_entry *) bfd_hash_lookup (&tbl->table,
+							  name,
+							  create,
+							  copy);
+}
+
+static void
+group_sec_hash_traverse (struct group_sec_hash_table *tbl,
+			 bfd_boolean (*func)
+			   (struct group_sec_hash_entry *, void *),
+			 void *info)
+{
+  bfd_hash_traverse (&tbl->table,
+		     (bfd_boolean (*) (struct bfd_hash_entry *, void *)) func,
+		     info);
+}
+
+static void
+group_sec_hash_table_free (struct group_sec_hash_table *tbl)
+{
+  bfd_hash_table_free (&tbl->table);
+}
+
+struct add_syms_info
+{
+  struct group_sec_hash_table sec_hash;
+  bfd *abfd;
+  asymbol **symptrs;
+  int n_nextsym;
+  bfd_boolean is_elf;
+};
+
+/* Create the signature symbol and size section contents.  */
+static bfd_boolean
+fix_comdat_groups (struct group_sec_hash_entry *entry, void *info)
+{
+  struct add_syms_info *asi = (struct add_syms_info *) info;
+  size_t sz = 4 * (entry->sec_count + 1);
+  asymbol *bfdsym;
+  elf_symbol_type *elfsym;
+
+  /* Usable signature symbol may already exist.  */
+  if (entry->n_symndx == -1)
+    {
+      bfdsym = bfd_make_empty_symbol (asi->abfd);
+      bfdsym->the_bfd = asi->abfd;
+      bfdsym->name = entry->entry.string;
+      bfdsym->value = 0;
+      bfdsym->section = entry->sec;
+      bfdsym->flags = BSF_WEAK | BSF_LOCAL;
+      elfsym = elf_symbol_from (asi->abfd, bfdsym);
+      elfsym->internal_elf_sym.st_other
+	= (STV_DEFAULT | (elfsym->internal_elf_sym.st_other
+			  & ~ELF_ST_VISIBILITY (-1)));
+      entry->n_symndx = asi->n_nextsym++;
+      asi->symptrs[entry->n_symndx] = bfdsym;
+      elf_group_id (entry->sec) = bfdsym;
+    }
+
+  /* Causes backend to allocate contents for us.  */
+  bfd_set_section_size (asi->abfd, entry->sec, sz);
+  return TRUE;
+}
+
+static struct bfd_section *
+get_elf_comdat_section (struct add_syms_info *asi,
+			      const struct ld_plugin_symbol *ldsym)
+{
+  struct bfd_section *sec;
+  struct group_sec_hash_entry *ent;
+  const char *secname = concat (".comdat.", ldsym->name, NULL);
+  sec = bfd_make_section_with_flags (asi->abfd, secname,
+				     SEC_CODE | SEC_READONLY | SEC_LOAD
+				     | SEC_ALLOC | SEC_HAS_CONTENTS);
+  if (sec == NULL)
+    sec = bfd_get_section_by_name (asi->abfd, secname);
+  else
+    {
+      sec->output_section = sec;
+      sec->output_offset = 0;
+    }
+  ent = group_sec_hash_lookup (&asi->sec_hash, ldsym->comdat_key,
+			       FALSE, FALSE);
+  elf_group_name (sec) = ldsym->comdat_key;
+  elf_sec_group (sec) = ent->sec;
+
+  if (elf_next_in_group (ent->sec) == NULL)
+    {
+      elf_next_in_group (ent->sec) = sec;
+      elf_next_in_group (sec) = sec;
+      ent->last_in_group = sec;
+    }
+  else
+    {
+      elf_next_in_group (sec) = elf_next_in_group (ent->sec);
+      elf_next_in_group (ent->sec) = sec;
+      elf_next_in_group (ent->last_in_group) = sec;
+    }
+
+  ent->sec_count++;
+  return sec;
+}
+
+static struct bfd_section *
+get_comdat_section (struct add_syms_info *asi,
+		    const struct ld_plugin_symbol *ldsym)
+{
+  struct bfd_section *sec;
+
+  if (asi->is_elf)
+    return get_elf_comdat_section (asi, ldsym);
+
+  sec = bfd_make_section_with_flags (asi->abfd, ldsym->comdat_key,
+				     SEC_CODE | SEC_HAS_CONTENTS | SEC_READONLY
+				     | SEC_ALLOC | SEC_LOAD | SEC_LINK_ONCE
+				     | SEC_LINK_DUPLICATES_DISCARD);
+  if (sec == NULL)
+    sec = bfd_get_section_by_name (asi->abfd, ldsym->comdat_key);
+  else
+    {
+      sec->output_section = sec;
+      sec->output_offset = 0;
+    }
+  
+  return sec;
+}
+
 static enum ld_plugin_status
-asymbol_from_plugin_symbol (bfd *abfd, asymbol *asym,
+asymbol_from_plugin_symbol (struct add_syms_info *asi,
+			    asymbol *asym,
 			    const struct ld_plugin_symbol *ldsym)
 {
   flagword flags = BSF_NO_FLAGS;
   struct bfd_section *section;
 
-  asym->the_bfd = abfd;
+  asym->the_bfd = asi->abfd;
   asym->name = (ldsym->version
 		? concat (ldsym->name, "@", ldsym->version, (const char *) NULL)
 		: ldsym->name);
@@ -284,7 +492,9 @@ asymbol_from_plugin_symbol (bfd *abfd, a
       /* FALLTHRU */
     case LDPK_DEF:
       flags |= BSF_GLOBAL;
-      section = bfd_get_section_by_name (abfd, ".text");
+      section = ldsym->comdat_key
+		  ? get_comdat_section (asi, ldsym)
+		  : bfd_get_section_by_name (asi->abfd, ".text");
       break;
 
     case LDPK_WEAKUNDEF:
@@ -299,7 +509,7 @@ asymbol_from_plugin_symbol (bfd *abfd, a
       section = bfd_com_section_ptr;
       asym->value = ldsym->size;
       /* For ELF targets, set alignment of common symbol to 1.  */
-      if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+      if (asi->is_elf)
 	{
 	  ((elf_symbol_type *) asym)->internal_elf_sym.st_shndx = SHN_COMMON;
 	  ((elf_symbol_type *) asym)->internal_elf_sym.st_value = 1;
@@ -313,9 +523,9 @@ asymbol_from_plugin_symbol (bfd *abfd, a
   asym->section = section;
 
   /* Visibility only applies on ELF targets.  */
-  if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+  if (asi->is_elf)
     {
-      elf_symbol_type *elfsym = elf_symbol_from (abfd, asym);
+      elf_symbol_type *elfsym = elf_symbol_from (asi->abfd, asym);
       unsigned char visibility;
 
       if (!elfsym)
@@ -374,24 +584,62 @@ register_cleanup (ld_plugin_cleanup_hand
 }
 
 /* Add symbols from a plugin-claimed input file.  */
+
 static enum ld_plugin_status
 add_symbols (void *handle, int nsyms, const struct ld_plugin_symbol *syms)
 {
-  asymbol **symptrs;
-  bfd *abfd = handle;
-  int n;
+  struct add_syms_info asi;
+  int n, totalsyms;
   ASSERT (called_plugin);
-  symptrs = xmalloc (nsyms * sizeof *symptrs);
+  asi.abfd = handle;
+  asi.is_elf = (bfd_get_flavour (asi.abfd) == bfd_target_elf_flavour);
+  /* For ELF create comdat group sections first.  */
+  if (asi.is_elf)
+    {
+      init_group_sec_hash_table (&asi.sec_hash, asi.abfd);
+      for (n = 0; n < nsyms; n++)
+	{
+	  const char *key = syms[n].comdat_key;
+	  if (key == NULL)
+	    continue;
+	  if ((syms[n].def == LDPK_UNDEF) || (syms[n].def == LDPK_WEAKUNDEF))
+	    continue;
+	  if (group_sec_hash_lookup (&asi.sec_hash, key, TRUE, TRUE) == NULL)
+	    return LDPS_ERR;
+	}
+    }
+  totalsyms = nsyms + (asi.is_elf ? asi.sec_hash.n_comdat_keys : 0);
+  asi.symptrs = xmalloc (totalsyms * sizeof *asi.symptrs);
+  /* Convert symbols.  */
   for (n = 0; n < nsyms; n++)
     {
       enum ld_plugin_status rv;
-      asymbol *bfdsym = bfd_make_empty_symbol (abfd);
-      symptrs[n] = bfdsym;
-      rv = asymbol_from_plugin_symbol (abfd, bfdsym, syms + n);
+      struct group_sec_hash_entry *ent;
+      asymbol *bfdsym = bfd_make_empty_symbol (asi.abfd);
+      asi.symptrs[n] = bfdsym;
+      rv = asymbol_from_plugin_symbol (&asi, bfdsym, syms + n);
       if (rv != LDPS_OK)
 	return rv;
-    }
-  bfd_set_symtab (abfd, symptrs, nsyms);
+      if (asi.is_elf)
+	{
+	  ent = group_sec_hash_lookup (&asi.sec_hash, syms[n].name, FALSE,
+				       FALSE);
+	  if (ent)
+	    {
+	      ent->n_symndx = n;
+	      elf_group_id (ent->sec) = bfdsym;
+	    }
+	}
+    }
+  /* Now create the comdat group signature symbols.  */
+  asi.n_nextsym = nsyms;
+  if (asi.is_elf)
+      group_sec_hash_traverse (&asi.sec_hash, fix_comdat_groups, &asi);
+  /* And insert into the BFD.  */
+  bfd_set_symtab (asi.abfd, asi.symptrs, asi.n_nextsym);
+  /* Done with the group section hash.  */
+  if (asi.is_elf)
+    group_sec_hash_table_free (&asi.sec_hash);
   return LDPS_OK;
 }