RE: Group identifier of a comdat group

["Jessica Han" <jessica at cup dot hp dot com>]

Alan,
  I tried your patch yesterday and I am able to generate the COMDAT
sections. I still have a couple of questions:
1. group_signature uses the name field of the symtab section to get the
group signature, but that field is empty. Could you tell me which utility
makes use of setup_group, so I can verify this?
2. for a text section gnu.linkonce.t.foo, gas generates
rela.gnu.linkeonce.t.foo, gnu.linkonce.ia64unwi.foo,
gnu.linkonce.ia64unw.foo, etc. Are they supposed to be in the COMDAT group
as well?
Thanks.

>-----Original Message-----
>From: Alan Modra [mailto:amodra@bigpond.net.au]
>Sent: Monday, June 03, 2002 6:01 PM
>To: Jessica Han; binutils@sources.redhat.com; reva@cup.hp.com
>Subject: Re: Group identifier of a comdat group
>
>
>I'm committing this one.  Differs from the last patch in that the BFD
>section for a SHT_GROUP ELF section takes it's name from the group
>signature symbol.  Gas makes the name of the group section the same as
>the signature, but the spec says the ELF section name is irrelevant.
>Also, gas now makes a group GRP_COMDAT if any component section is
>named .gnu.linkonce*.
>
>bfd/ChangeLog
>	* elf.c (setup_group): Set SEC_LINK_ONCE on GRP_COMDAT groups.
>	(bfd_section_from_shdr): Likewise.  Set section name of group
>	sections from signature.
>	(group_signature): Split out from setup_group.  Ensure
>symbol table
>	is available.
>	(bfd_elf_discard_group): New function.
>	(_bfd_elf_make_section_from_shdr): Don't set SEC_LINK_ONCE on
>	.gnu.linkonce* sections if they are members of a group.
>	(set_group_contents): Set GRP_COMDAT flag.
>	* section.c (bfd_discard_group): New function.
>	* bfd-in.h (bfd_elf_discard_group): Declare.
>	* bfd-in2.h: Regenerate.
>	* elf-bfd.h (struct bfd_elf_section_data): Add linkonce_p field.
>	(elf_linkonce_p): Define.
>
>gas/ChangeLog
>	* config/obj-elf.c (obj_elf_change_section): Set and check elf
>	linkonce flag.  Print all warnings.
>	(obj_elf_section): Parse ",comdat" for groups.
>	(elf_frob_file): Set SEC_LINK_ONCE on COMDAT groups.  Check
>	consistency of comdat flag.
>
>ld/ChangeLog
>	* ldlang.c (section_already_linked): Call
>bfd_discard_group.  Typo fix.
>
>Index: bfd/bfd-in.h
>===================================================================
>RCS file: /cvs/src/src/bfd/bfd-in.h,v
>retrieving revision 1.46
>diff -u -p -r1.46 bfd-in.h
>--- bfd/bfd-in.h	29 May 2002 16:03:04 -0000	1.46
>+++ bfd/bfd-in.h	4 Jun 2002 00:06:55 -0000
>@@ -657,6 +657,8 @@ extern boolean bfd_elf32_discard_info
>   PARAMS ((bfd *, struct bfd_link_info *));
> extern boolean bfd_elf64_discard_info
>   PARAMS ((bfd *, struct bfd_link_info *));
>+extern void bfd_elf_discard_group
>+  PARAMS ((bfd *, struct sec *));
>
> /* Return an upper bound on the number of bytes required to store a
>    copy of ABFD's program header table entries.  Return -1 if an error
>Index: bfd/elf-bfd.h
>===================================================================
>RCS file: /cvs/src/src/bfd/elf-bfd.h,v
>retrieving revision 1.73
>diff -u -p -r1.73 elf-bfd.h
>--- bfd/elf-bfd.h	31 May 2002 02:59:47 -0000	1.73
>+++ bfd/elf-bfd.h	4 Jun 2002 00:07:05 -0000
>@@ -926,11 +926,15 @@ struct bfd_elf_section_data
>
>   /* Nonzero if this section uses RELA relocations, rather
>than REL.  */
>   unsigned int use_rela_p:1;
>+
>+  /* Nonzero when a group is COMDAT.  */
>+  unsigned int linkonce_p:1;
> };
>
> #define elf_section_data(sec)  ((struct
>bfd_elf_section_data*)sec->used_by_bfd)
> #define elf_group_name(sec)    (elf_section_data(sec)->group_name)
> #define elf_next_in_group(sec) (elf_section_data(sec)->next_in_group)
>+#define elf_linkonce_p(sec)    (elf_section_data(sec)->linkonce_p)
>
> /* Return true if section has been discarded.  */
> #define elf_discarded_section(sec)
>	\
>Index: bfd/elf.c
>===================================================================
>RCS file: /cvs/src/src/bfd/elf.c,v
>retrieving revision 1.140
>diff -u -p -r1.140 elf.c
>--- bfd/elf.c	31 May 2002 02:59:47 -0000	1.140
>+++ bfd/elf.c	4 Jun 2002 00:07:10 -0000
>@@ -50,6 +50,7 @@ static boolean prep_headers PARAMS ((bfd
> static boolean swap_out_syms PARAMS ((bfd *, struct
>bfd_strtab_hash **, int));
> static boolean copy_private_bfd_data PARAMS ((bfd *, bfd *));
> static char *elf_read PARAMS ((bfd *, file_ptr, bfd_size_type));
>+static const char *group_signature PARAMS ((bfd *,
>Elf_Internal_Shdr *));
> static boolean setup_group PARAMS ((bfd *, Elf_Internal_Shdr
>*, asection *));
> static void merge_sections_remove_hook PARAMS ((bfd *, asection *));
> static void elf_fake_sections PARAMS ((bfd *, asection *, PTR));
>@@ -361,6 +362,35 @@ typedef union elf_internal_group {
>   unsigned int flags;
> } Elf_Internal_Group;
>
>+/* Return the name of the group signature symbol.  Why isn't the
>+   signature just a string?  */
>+
>+static const char *
>+group_signature (abfd, ghdr)
>+     bfd *abfd;
>+     Elf_Internal_Shdr *ghdr;
>+{
>+  struct elf_backend_data *bed;
>+  file_ptr pos;
>+  unsigned char ename[4];
>+  unsigned long iname;
>+
>+  /* First we need to ensure the symbol table is available.  */
>+  if (! bfd_section_from_shdr (abfd, ghdr->sh_link))
>+    return NULL;
>+
>+  /* Fortunately, the name index is at the same place in the external
>+     symbol for both 32 and 64 bit ELF.  */
>+  bed = get_elf_backend_data (abfd);
>+  pos = elf_tdata (abfd)->symtab_hdr.sh_offset;
>+  pos += ghdr->sh_info * bed->s->sizeof_sym;
>+  if (bfd_seek (abfd, pos, SEEK_SET) != 0
>+      || bfd_bread (ename, (bfd_size_type) 4, abfd) != 4)
>+    return NULL;
>+  iname = H_GET_32 (abfd, ename);
>+  return elf_string_from_elf_strtab (abfd, iname);
>+}
>+
> /* Set next_in_group list pointer, and group name for NEWSECT.  */
>
> static boolean
>@@ -440,6 +470,9 @@ setup_group (abfd, hdr, newsect)
> 		      if (src == shdr->contents)
> 			{
> 			  dest->flags = idx;
>+			  if (shdr->bfd_section != NULL && (idx
>& GRP_COMDAT))
>+			    shdr->bfd_section->flags
>+			      |= SEC_LINK_ONCE |
>SEC_LINK_DUPLICATES_DISCARD;
> 			  break;
> 			}
> 		      if (idx >= shnum)
>@@ -492,32 +525,20 @@ setup_group (abfd, hdr, newsect)
> 		  }
> 		else
> 		  {
>-		    struct elf_backend_data *bed;
>-		    file_ptr pos;
>-		    unsigned char ename[4];
>-		    unsigned long iname;
> 		    const char *gname;
>
>-		    /* Humbug.  Get the name from the group signature
>-		       symbol.  Why isn't the signature just a string?
>-		       Fortunately, the name index is at the same
>-		       place in the external symbol for both 32 and 64
>-		       bit ELF.  */
>-		    bed = get_elf_backend_data (abfd);
>-		    pos = elf_tdata (abfd)->symtab_hdr.sh_offset;
>-		    pos += shdr->sh_info * bed->s->sizeof_sym;
>-		    if (bfd_seek (abfd, pos, SEEK_SET) != 0
>-			|| bfd_bread (ename, (bfd_size_type) 4,
>abfd) != 4)
>+		    gname = group_signature (abfd, shdr);
>+		    if (gname == NULL)
> 		      return false;
>-		    iname = H_GET_32 (abfd, ename);
>-		    gname = elf_string_from_elf_strtab (abfd, iname);
> 		    elf_group_name (newsect) = gname;
>
> 		    /* Start a circular list with one element.  */
> 		    elf_next_in_group (newsect) = newsect;
> 		  }
>+
> 		if (shdr->bfd_section != NULL)
> 		  elf_next_in_group (shdr->bfd_section) = newsect;
>+
> 		i = num_group - 1;
> 		break;
> 	      }
>@@ -532,6 +553,24 @@ setup_group (abfd, hdr, newsect)
>   return true;
> }
>
>+void
>+bfd_elf_discard_group (abfd, group)
>+     bfd *abfd ATTRIBUTE_UNUSED;
>+     asection *group;
>+{
>+  asection *first = elf_next_in_group (group);
>+  asection *s = first;
>+
>+  while (s != NULL)
>+    {
>+      s->output_section = bfd_abs_section_ptr;
>+      s = elf_next_in_group (s);
>+      /* These lists are circular.  */
>+      if (s == first)
>+	break;
>+    }
>+}
>+
> /* Make a BFD section from an ELF section.  We store a pointer to the
>    BFD section in the bfd_section field of the header.  */
>
>@@ -620,7 +659,8 @@ _bfd_elf_make_section_from_shdr (abfd, h
>      The symbols will be defined as weak, so that multiple definitions
>      are permitted.  The GNU linker extension is to actually discard
>      all but one of the sections.  */
>-  if (strncmp (name, ".gnu.linkonce", sizeof ".gnu.linkonce"
>- 1) == 0)
>+  if (strncmp (name, ".gnu.linkonce", sizeof ".gnu.linkonce" - 1) == 0
>+      && elf_next_in_group (newsect) == NULL)
>     flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>
>   bed = get_elf_backend_data (abfd);
>@@ -1576,7 +1616,7 @@ bfd_section_from_shdr (abfd, shindex)
>   Elf_Internal_Shdr *hdr = elf_elfsections (abfd)[shindex];
>   Elf_Internal_Ehdr *ehdr = elf_elfheader (abfd);
>   struct elf_backend_data *bed = get_elf_backend_data (abfd);
>-  char *name;
>+  const char *name;
>
>   name = elf_string_from_elf_strtab (abfd, hdr->sh_name);
>
>@@ -1820,7 +1860,12 @@ bfd_section_from_shdr (abfd, shindex)
>       return true;
>
>     case SHT_GROUP:
>-      /* Make a section for objcopy and relocatable links.  */
>+      /* We need a BFD section for objcopy and relocatable linking,
>+	 and it's handy to have the signature available as the section
>+	 name.  */
>+      name = group_signature (abfd, hdr);
>+      if (name == NULL)
>+	return false;
>       if (!_bfd_elf_make_section_from_shdr (abfd, hdr, name))
> 	return false;
>       if (hdr->contents != NULL)
>@@ -1829,6 +1874,10 @@ bfd_section_from_shdr (abfd, shindex)
> 	  unsigned int n_elt = hdr->sh_size / 4;
> 	  asection *s;
>
>+	  if (idx->flags & GRP_COMDAT)
>+	    hdr->bfd_section->flags
>+	      |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>+
> 	  while (--n_elt != 0)
> 	    if ((s = (++idx)->shdr->bfd_section) != NULL
> 		&& elf_next_in_group (s) != NULL)
>@@ -2369,7 +2418,7 @@ set_group_contents (abfd, sec, failedptr
>       while (elt != elf_next_in_group (l->u.indirect.section));
>
>   loc -= 4;
>-  H_PUT_32 (abfd, 0, loc);
>+  H_PUT_32 (abfd, sec->flags & SEC_LINK_ONCE ? GRP_COMDAT : 0, loc);
>
>   BFD_ASSERT (loc == sec->contents);
> }
>Index: bfd/section.c
>===================================================================
>RCS file: /cvs/src/src/bfd/section.c,v
>retrieving revision 1.45
>diff -u -p -r1.45 section.c
>--- bfd/section.c	23 May 2002 13:12:46 -0000	1.45
>+++ bfd/section.c	4 Jun 2002 00:07:49 -0000
>@@ -1375,3 +1375,24 @@ _bfd_strip_section_from_output (info, s)
>
>   s->flags |= SEC_EXCLUDE;
> }
>+
>+/*
>+FUNCTION
>+	bfd_discard_group
>+
>+SYNOPSIS
>+	void bfd_discard_group (bfd *abfd, asection *group);
>+
>+DESCRIPTION
>+	Remove all members of @var{group} from the output.
>+*/
>+
>+void
>+bfd_discard_group (abfd, group)
>+     bfd *abfd;
>+     asection *group;
>+{
>+  if ((group->flags & SEC_GROUP) != 0
>+      && abfd->xvec->flavour == bfd_target_elf_flavour)
>+    bfd_elf_discard_group (abfd, group);
>+}
>Index: gas/config/obj-elf.c
>===================================================================
>RCS file: /cvs/src/src/gas/config/obj-elf.c,v
>retrieving revision 1.50
>diff -u -p -r1.50 obj-elf.c
>--- gas/config/obj-elf.c	23 May 2002 13:12:47 -0000	1.50
>+++ gas/config/obj-elf.c	3 Jun 2002 23:53:29 -0000
>@@ -77,7 +77,7 @@ static void obj_elf_weak PARAMS ((int));
> static void obj_elf_local PARAMS ((int));
> static void obj_elf_visibility PARAMS ((int));
> static void obj_elf_change_section
>-  PARAMS ((const char *, int, int, int, const char *, int));
>+  PARAMS ((const char *, int, int, int, const char *, int, int));
> static int obj_elf_parse_section_letters PARAMS ((char *, size_t));
> static int obj_elf_section_word PARAMS ((char *, size_t));
> static char *obj_elf_section_name PARAMS ((void));
>@@ -664,12 +664,13 @@ static struct special_section const spec
> };
>
> static void
>-obj_elf_change_section (name, type, attr, entsize, group_name, push)
>+obj_elf_change_section (name, type, attr, entsize,
>group_name, linkonce, push)
>      const char *name;
>      int type;
>      int attr;
>      int entsize;
>      const char *group_name;
>+     int linkonce;
>      int push;
> {
>   asection *old_sec;
>@@ -758,6 +759,7 @@ obj_elf_change_section (name, type, attr
>       if (flags & SEC_MERGE)
> 	sec->entsize = entsize;
>       elf_group_name (sec) = group_name;
>+      elf_linkonce_p (sec) = linkonce;
>
>       /* Add a symbol for this section to the symbol table.  */
>       secsym = symbol_find (name);
>@@ -771,15 +773,16 @@ obj_elf_change_section (name, type, attr
>       /* If section attributes are specified the second time we see a
> 	 particular section, then check that they are the same as we
> 	 saw the first time.  */
>-      if ((old_sec->flags ^ flags)
>-	  & (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>-	     | SEC_EXCLUDE | SEC_SORT_ENTRIES | SEC_MERGE | SEC_STRINGS
>-	     | SEC_THREAD_LOCAL))
>+      if (((old_sec->flags ^ flags)
>+	   & (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>+	      | SEC_EXCLUDE | SEC_SORT_ENTRIES | SEC_MERGE | SEC_STRINGS
>+	      | SEC_THREAD_LOCAL))
>+	  || linkonce != elf_linkonce_p (sec))
> 	as_warn (_("ignoring changed section attributes for %s"), name);
>-      else if ((flags & SEC_MERGE) && old_sec->entsize !=
>(unsigned) entsize)
>+      if ((flags & SEC_MERGE) && old_sec->entsize !=
>(unsigned) entsize)
> 	as_warn (_("ignoring changed section entity size for
>%s"), name);
>-      else if ((attr & SHF_GROUP) != 0
>-	       && strcmp (elf_group_name (old_sec), group_name) != 0)
>+      if ((attr & SHF_GROUP) != 0
>+	  && strcmp (elf_group_name (old_sec), group_name) != 0)
> 	as_warn (_("ignoring new section group for %s"), name);
>     }
>
>@@ -947,6 +950,7 @@ obj_elf_section (push)
>   char *name, *group_name, *beg;
>   int type, attr, dummy;
>   int entsize;
>+  int linkonce;
>
> #ifndef TC_I370
>   if (flag_mri)
>@@ -977,6 +981,7 @@ obj_elf_section (push)
>   attr = 0;
>   group_name = NULL;
>   entsize = 0;
>+  linkonce = 0;
>
>   if (*input_line_pointer == ',')
>     {
>@@ -1050,6 +1055,13 @@ obj_elf_section (push)
> 	      group_name = obj_elf_section_name ();
> 	      if (group_name == NULL)
> 		attr &= ~SHF_GROUP;
>+	      else if (strncmp (input_line_pointer, ",comdat", 7) == 0)
>+		{
>+		  input_line_pointer += 7;
>+		  linkonce = 1;
>+		}
>+	      else if (strncmp (name, ".gnu.linkonce", 13) == 0)
>+		linkonce = 1;
> 	    }
> 	  else if ((attr & SHF_GROUP) != 0)
> 	    {
>@@ -1085,7 +1097,7 @@ obj_elf_section (push)
>
>   demand_empty_rest_of_line ();
>
>-  obj_elf_change_section (name, type, attr, entsize,
>group_name, push);
>+  obj_elf_change_section (name, type, attr, entsize,
>group_name, linkonce, push);
> }
>
> /* Change to the .data section.  */
>@@ -2015,8 +2027,20 @@ elf_frob_file ()
>       asection *s;
>       flagword flags;
>
>-      s = subseg_force_new (group_name, 0);
>       flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY
>| SEC_GROUP;
>+      for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
>+	if (elf_linkonce_p (s) != ((flags & SEC_LINK_ONCE) != 0))
>+	  {
>+	    flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>+	    if (s != list.head[i])
>+	      {
>+		as_warn (_("assuming all members of group `%s'
>are COMDAT"),
>+			 group_name);
>+		break;
>+	      }
>+	  }
>+
>+      s = subseg_force_new (group_name, 0);
>       if (s == NULL
> 	  || !bfd_set_section_flags (stdoutput, s, flags)
> 	  || !bfd_set_section_alignment (stdoutput, s, 2))
>Index: ld/ldlang.c
>===================================================================
>RCS file: /cvs/src/src/ld/ldlang.c,v
>retrieving revision 1.88
>diff -u -p -r1.88 ldlang.c
>--- ld/ldlang.c	27 May 2002 08:22:07 -0000	1.88
>+++ ld/ldlang.c	3 Jun 2002 23:54:02 -0000
>@@ -985,7 +985,7 @@ section_already_linked (abfd, sec, data)
>      of having link once sections in the first place.
>
>      Also, not merging link once sections in a relocatable link
>-     causes trouble for MIPS ELF, which relies in link once semantics
>+     causes trouble for MIPS ELF, which relies on link once semantics
>      to handle the .reginfo section correctly.  */
>
>   name = bfd_get_section_name (abfd, sec);
>@@ -1038,6 +1038,9 @@ section_already_linked (abfd, sec, data)
> 	     does not create a lang_input_section structure for this
> 	     section.  */
> 	  sec->output_section = bfd_abs_section_ptr;
>+
>+	  if (flags & SEC_GROUP)
>+	    bfd_discard_group (abfd, sec);
>
> 	  return;
> 	}
>
>--
>Alan Modra
>IBM OzLabs - Linux Technology Centre
>