PATCH: Fix PR/161/251: comdat group and linkonce

H. J. Lu hjl@lucon.org
Thu Jul 22 20:48:00 GMT 2004


The current linker doesn't support mix comdat group and linkonce
sections:

http://sources.redhat.com/bugzilla/show_bug.cgi?id=161

This is needed for gcc to generate comdat group without requiring
the whole world should comdat group. Also "ld -r" doesn't work
when relocation section in comdat group:

http://sources.redhat.com/bugzilla/show_bug.cgi?id=251

This patch addresses the first problem by matching linkonce section
with single member comdat group. The second problem is solved by
skipping relocation section in comdat group when setting up group
pointer.


H.J.
-------------- next part --------------
2004-07-21  H.J. Lu  <hongjiu.lu@intel.com>

	PR 161/251
	* elf-bfd.h (bfd_elf_section_data): Add sec_group.
	(elf_sec_group): Defined.
	(bfd_elf_match_symbols_in_sections): New prototype.
	(_bfd_elf_setup_group_pointers): Likewise.

	* elf.c (bfd_elf_discard_group): Abort.
	(bfd_elf_set_group_contents): Also include relocation sections.
	Remove zero-fill for ld -r.
	(_bfd_elf_setup_group_pointers): New function.
	(elf_sort_elf_symbol): Likewise.
	(elf_sym_name_compare): Likewise.
	(bfd_elf_match_symbols_in_sections): Likewise.

	* elfcode.h (elf_object_p): Call _bfd_elf_setup_group_pointers.

	* elflink.c (match_group_member): New.
	(elf_link_input_bfd): Check group member for discarded section.
	(try_match_symbols_in_sections): New function.
	(already_linked): Likewise.
	(_bfd_elf_section_already_linked): Support mixing comdat group
	and linkonce section.

	* libbfd-in.h (bfd_section_already_linked_table_traverse): New.
	* linker.c (bfd_section_already_linked_table_traverse): New.

	* libbfd.h: Regenerated.

--- bfd/elf-bfd.h.group	2004-07-03 10:04:17.000000000 -0700
+++ bfd/elf-bfd.h	2004-07-03 10:04:17.143415338 -0700
@@ -1048,6 +1048,10 @@ struct bfd_elf_section_data
     struct bfd_symbol *id;
   } group;
 
+  /* Optional information about section group; NULL if it doesn't
+     belongs to any section group. */
+  asection *sec_group;
+
   /* A linked list of sections in the group.  Circular when used by
      the linker.  */
   asection *next_in_group;
@@ -1062,6 +1066,7 @@ struct bfd_elf_section_data
 #define elf_group_name(sec)    (elf_section_data(sec)->group.name)
 #define elf_group_id(sec)      (elf_section_data(sec)->group.id)
 #define elf_next_in_group(sec) (elf_section_data(sec)->next_in_group)
+#define elf_sec_group(sec)	(elf_section_data(sec)->sec_group)
 
 /* Return TRUE if section has been discarded.  */
 #define elf_discarded_section(sec)				\
@@ -1565,6 +1570,12 @@ extern bfd_boolean _bfd_elf_dynamic_symb
 extern bfd_boolean _bfd_elf_symbol_refs_local_p
   (struct elf_link_hash_entry *, struct bfd_link_info *, bfd_boolean);
 
+extern bfd_boolean bfd_elf_match_symbols_in_sections
+  (asection *sec1, asection *sec2);
+
+extern bfd_boolean _bfd_elf_setup_group_pointers
+  (bfd *);
+
 extern const bfd_target *bfd_elf32_object_p
   (bfd *);
 extern const bfd_target *bfd_elf32_core_file_p
--- bfd/elf.c.group	2004-06-28 08:58:05.000000000 -0700
+++ bfd/elf.c	2004-07-03 10:04:17.152414172 -0700
@@ -613,14 +613,58 @@ setup_group (bfd *abfd, Elf_Internal_Shd
 }
 
 bfd_boolean
+_bfd_elf_setup_group_pointers (bfd *abfd)
+{
+  unsigned int i;
+  unsigned int num_group = elf_tdata (abfd)->num_group;
+  bfd_boolean result = TRUE;
+
+  if (num_group == (unsigned) -1)
+    return result;
+
+  for (i = 0; i < num_group; i++)
+    {
+      Elf_Internal_Shdr *shdr = elf_tdata (abfd)->group_sect_ptr[i];
+      Elf_Internal_Group *idx = (Elf_Internal_Group *) shdr->contents;
+      unsigned int n_elt = shdr->sh_size / 4;
+
+      while (--n_elt != 0)
+	if ((++idx)->shdr->bfd_section)
+	  elf_sec_group (idx->shdr->bfd_section) = shdr->bfd_section;
+	else if (idx->shdr->sh_type == SHT_RELA
+		 || idx->shdr->sh_type == SHT_REL)
+	  /* We won't include relocation sections in section groups in
+	     output object files. We adjust the group section size here
+	     so that relocatable link will work correctly when
+	     relocation sections are in section group in input object
+	     files.  */
+	  shdr->bfd_section->size -= 4;
+	else
+	  {
+	    /* There are some unknown sections in the group.  */
+	    (*_bfd_error_handler)
+	      (_("%s: unknown [%d] section `%s' in group [%s]"),
+	       bfd_archive_filename (abfd),
+	       (unsigned int) idx->shdr->sh_type,
+	       elf_string_from_elf_strtab (abfd, idx->shdr->sh_name),
+	       shdr->bfd_section->name);
+	    result = FALSE;
+	  }
+    }
+  return result;
+}
+
+bfd_boolean
 bfd_elf_is_group_section (bfd *abfd ATTRIBUTE_UNUSED, const asection *sec)
 {
   return elf_next_in_group (sec) != NULL;
 }
 
 bfd_boolean
-bfd_elf_discard_group (bfd *abfd ATTRIBUTE_UNUSED, asection *group)
+bfd_elf_discard_group (bfd *abfd ATTRIBUTE_UNUSED,
+		       asection *group ATTRIBUTE_UNUSED)
 {
+#if 0
   asection *first = elf_next_in_group (group);
   asection *s = first;
 
@@ -632,6 +676,10 @@ bfd_elf_discard_group (bfd *abfd ATTRIBU
       if (s == first)
 	break;
     }
+#else
+  /* FIXME: Never used. Remove it!  */
+  abort ();
+#endif
   return TRUE;
 }
 
@@ -2654,13 +2702,7 @@ bfd_elf_set_group_contents (bfd *abfd, a
 	}
       while (elt != elf_next_in_group (l->u.indirect.section));
 
-  /* With ld -r, merging SHT_GROUP sections results in wasted space
-     due to allowing for the flag word on each input.  We may well
-     duplicate entries too.  */
-  while ((loc -= 4) > sec->contents)
-    H_PUT_32 (abfd, 0, loc);
-
-  if (loc != sec->contents)
+  if ((loc -= 4) != sec->contents)
     abort ();
 
   H_PUT_32 (abfd, sec->flags & SEC_LINK_ONCE ? GRP_COMDAT : 0, loc);
@@ -7702,3 +7744,212 @@ _bfd_elf_get_synthetic_symtab (bfd *abfd
 
   return n;
 }
+
+/* Sort symbol by binding and section. We want to put definitions
+   sorted by section at the beginning.  */
+
+static int
+elf_sort_elf_symbol (const void *arg1, const void *arg2)
+{
+  const Elf_Internal_Sym *s1;
+  const Elf_Internal_Sym *s2;
+  int shndx;
+
+  /* Make sure that undefined symbols are at the end.  */
+  s1 = (const Elf_Internal_Sym *) arg1;
+  if (s1->st_shndx == SHN_UNDEF)
+    return 1;
+  s2 = (const Elf_Internal_Sym *) arg2;
+  if (s2->st_shndx == SHN_UNDEF)
+    return -1;
+
+  /* Sorted by section index.  */
+  shndx = s1->st_shndx - s2->st_shndx;
+  if (shndx != 0)
+    return shndx;
+
+  /* Sorted by binding.  */
+  return ELF_ST_BIND (s1->st_info)  - ELF_ST_BIND (s2->st_info);
+}
+
+struct elf_symbol
+{
+  Elf_Internal_Sym *sym;
+  const char *name;
+};
+
+static int
+elf_sym_name_compare (const void *arg1, const void *arg2)
+{
+  const struct elf_symbol *s1 = (const struct elf_symbol *) arg1;
+  const struct elf_symbol *s2 = (const struct elf_symbol *) arg2;
+  return strcmp (s1->name, s2->name);
+}
+
+/* Check if 2 sections define the same set of local and global
+   symbols.  */
+
+bfd_boolean
+bfd_elf_match_symbols_in_sections (asection *sec1, asection *sec2)
+{
+  bfd *bfd1, *bfd2;
+  const struct elf_backend_data *bed1, *bed2;
+  Elf_Internal_Shdr *hdr1, *hdr2;
+  bfd_size_type symcount1, symcount2;
+  Elf_Internal_Sym *isymbuf1, *isymbuf2;
+  Elf_Internal_Sym *isymstart1 = NULL, *isymstart2 = NULL, *isym;
+  Elf_Internal_Sym *isymend;
+  struct elf_symbol *symp, *symtable1 = NULL, *symtable2 = NULL;
+  bfd_size_type count1, count2, i;
+  int shndx1, shndx2;
+  bfd_boolean result;
+
+  bfd1 = sec1->owner;
+  bfd2 = sec2->owner;
+
+  /* If both are .gnu.linkonce sections, they have to have the same
+     section name.  */
+  if (strncmp (sec1->name, ".gnu.linkonce",
+	       sizeof ".gnu.linkonce" - 1) == 0
+      && strncmp (sec2->name, ".gnu.linkonce",
+		  sizeof ".gnu.linkonce" - 1) == 0)
+    return strcmp (sec1->name + sizeof ".gnu.linkonce",
+		   sec2->name + sizeof ".gnu.linkonce") == 0;
+
+  /* Both sections have to be in ELF.  */
+  if (bfd_get_flavour (bfd1) != bfd_target_elf_flavour
+      || bfd_get_flavour (bfd2) != bfd_target_elf_flavour)
+    return FALSE;
+
+  if (elf_section_type (sec1) != elf_section_type (sec2))
+    return FALSE;
+
+  if ((elf_section_flags (sec1) & SHF_GROUP) != 0
+      && (elf_section_flags (sec2) & SHF_GROUP) != 0)
+    {
+      /* If both are members of section groups, they have to have the
+	 same group name.  */
+      if (strcmp (elf_group_name (sec1), elf_group_name (sec2)) != 0)
+	return FALSE;
+    }
+
+  shndx1 = _bfd_elf_section_from_bfd_section (bfd1, sec1);
+  shndx2 = _bfd_elf_section_from_bfd_section (bfd2, sec2);
+  if (shndx1 == -1 || shndx2 == -1)
+    return FALSE;
+
+  bed1 = get_elf_backend_data (bfd1);
+  bed2 = get_elf_backend_data (bfd2);
+  hdr1 = &elf_tdata (bfd1)->symtab_hdr;
+  symcount1 = hdr1->sh_size / bed1->s->sizeof_sym;
+  hdr2 = &elf_tdata (bfd2)->symtab_hdr;
+  symcount2 = hdr2->sh_size / bed2->s->sizeof_sym;
+
+  if (symcount1 == 0 || symcount2 == 0)
+    return FALSE;
+
+  isymbuf1 = bfd_elf_get_elf_syms (bfd1, hdr1, symcount1, 0,
+				   NULL, NULL, NULL);
+  isymbuf2 = bfd_elf_get_elf_syms (bfd2, hdr2, symcount2, 0,
+				   NULL, NULL, NULL);
+
+  result = FALSE;
+  if (isymbuf1 == NULL || isymbuf2 == NULL)
+    goto done;
+
+  /* Sort symbols by binding and section. Global definitions are at
+     the beginning.  */
+  qsort (isymbuf1, symcount1, sizeof (Elf_Internal_Sym),
+	 elf_sort_elf_symbol);
+  qsort (isymbuf2, symcount2, sizeof (Elf_Internal_Sym),
+	 elf_sort_elf_symbol);
+
+  /* Count definitions in the section.  */
+  count1 = 0;
+  for (isym = isymbuf1, isymend = isym + symcount1;
+       isym < isymend; isym++)
+    {
+      if (isym->st_shndx == (unsigned int) shndx1)
+	{
+	  if (count1 == 0)
+	    isymstart1 = isym;
+	  count1++;
+	}
+
+      if (count1 && isym->st_shndx != (unsigned int) shndx1)
+	break;
+    }
+
+  count2 = 0;
+  for (isym = isymbuf2, isymend = isym + symcount2;
+       isym < isymend; isym++)
+    {
+      if (isym->st_shndx == (unsigned int) shndx2)
+	{
+	  if (count2 == 0)
+	    isymstart2 = isym;
+	  count2++;
+	}
+
+      if (count2 && isym->st_shndx != (unsigned int) shndx2)
+	break;
+    }
+
+  if (count1 == 0 || count2 == 0 || count1 != count2)
+    goto done;
+
+  symtable1 = bfd_malloc (count1 * sizeof (struct elf_symbol));
+  symtable2 = bfd_malloc (count1 * sizeof (struct elf_symbol));
+
+  if (symtable1 == NULL || symtable2 == NULL)
+    goto done;
+
+  symp = symtable1;
+  for (isym = isymstart1, isymend = isym + count1;
+       isym < isymend; isym++)
+    {
+      symp->sym = isym;
+      symp->name = bfd_elf_string_from_elf_section (bfd1,
+						    hdr1->sh_link,
+						    isym->st_name);
+      symp++;
+    }
+ 
+  symp = symtable2;
+  for (isym = isymstart2, isymend = isym + count1;
+       isym < isymend; isym++)
+    {
+      symp->sym = isym;
+      symp->name = bfd_elf_string_from_elf_section (bfd2,
+						    hdr2->sh_link,
+						    isym->st_name);
+      symp++;
+    }
+  
+  /* Sort symbol by name.  */
+  qsort (symtable1, count1, sizeof (struct elf_symbol),
+	 elf_sym_name_compare);
+  qsort (symtable2, count1, sizeof (struct elf_symbol),
+	 elf_sym_name_compare);
+
+  for (i = 0; i < count1; i++)
+    /* Two symbols must have the same binding, type and name.  */
+    if (symtable1 [i].sym->st_info != symtable2 [i].sym->st_info
+	|| symtable1 [i].sym->st_other != symtable2 [i].sym->st_other
+	|| strcmp (symtable1 [i].name, symtable2 [i].name) != 0)
+      goto done;
+
+  result = TRUE;
+
+done:
+  if (symtable1)
+    free (symtable1);
+  if (symtable2)
+    free (symtable2);
+  if (isymbuf1)
+    free (isymbuf1);
+  if (isymbuf2)
+    free (isymbuf2);
+
+  return result;
+}
--- bfd/elfcode.h.group	2004-06-24 08:32:01.000000000 -0700
+++ bfd/elfcode.h	2004-07-03 10:04:17.156413654 -0700
@@ -742,6 +742,10 @@ elf_object_p (bfd *abfd)
 	  if (shindex == SHN_LORESERVE - 1)
 	    shindex += SHN_HIRESERVE + 1 - SHN_LORESERVE;
 	}
+
+      /* Set up group pointers.  */
+      if (! _bfd_elf_setup_group_pointers (abfd))
+	goto got_wrong_format_error;
     }
 
   /* Let the backend double check the format and override global
--- bfd/elflink.c.group	2004-07-03 10:04:17.000000000 -0700
+++ bfd/elflink.c	2004-07-03 16:37:59.786803309 -0700
@@ -6313,6 +6313,26 @@ elf_section_complain_discarded (asection
   return TRUE;
 }
 
+/* Find a match between a section and a member of a section group.  */
+
+static asection *
+match_group_member (asection *sec, asection *group)
+{
+  asection *first = elf_next_in_group (group);
+  asection *s = first;
+
+  while (s != NULL)
+    {
+      if (bfd_elf_match_symbols_in_sections (s, sec))
+	return s;
+
+      if (s == first)
+	break;
+    }
+
+  return NULL;
+}
+
 /* Link an input file into the linker output file.  This function
    handles all the sections and relocations of the input file at once.
    This is so that we only have to read the local symbols once, and
@@ -6639,11 +6659,25 @@ elf_link_input_bfd (struct elf_final_lin
 			     ought to define the same set of symbols, so
 			     it would seem that globals ought to always
 			     be defined in the kept section.  */
-			  if (sec->kept_section != NULL
-			      && sec->size == sec->kept_section->size)
+			  if (sec->kept_section != NULL)
 			    {
-			      *ps = sec->kept_section;
-			      continue;
+			      asection *member;
+
+			      /* Check if it is a linkonce section or
+				 member of a comdat group.  */
+			      if (elf_sec_group (sec) == NULL
+				  && sec->size == sec->kept_section->size)
+				{
+				  *ps = sec->kept_section;
+				  continue;
+				}
+			      else if (elf_sec_group (sec) != NULL
+				       && (member = match_group_member (sec, sec->kept_section))
+				       && sec->size == member->size)
+				{
+				  *ps = member;
+				  continue;
+				}
 			    }
 			}
 		      else if (complain)
@@ -9011,6 +9045,83 @@ bfd_elf_discard_info (bfd *output_bfd, s
   return ret;
 }
 
+struct already_linked_section
+{
+  asection *sec;
+  asection *linked;
+};
+
+/* Check if the member of a single member comdat group matches a
+   linkonce section and vice versa.  */
+static bfd_boolean
+try_match_symbols_in_sections
+  (struct bfd_section_already_linked_hash_entry *h, void *info)
+{
+  struct bfd_section_already_linked *l;
+  struct already_linked_section *s
+    = (struct already_linked_section *) info;
+
+  if (elf_sec_group (s->sec) == NULL)
+    {
+      /* It is a linkonce section. Try to match it with the member of a
+	 single member comdat group. */
+      for (l = h->entry; l != NULL; l = l->next)
+	if ((l->sec->flags & SEC_GROUP))
+	  {
+	    asection *first = elf_next_in_group (l->sec);
+
+	    if (first != NULL
+		&& elf_next_in_group (first) == first
+		&& bfd_elf_match_symbols_in_sections (first, s->sec))
+	      {
+		s->linked = first;
+		return FALSE;
+	      }
+	  }
+    }
+  else
+    {
+      /* It is the member of a single member comdat group. Try to match
+	 it with a linkonce section.  */
+      for (l = h->entry; l != NULL; l = l->next)
+	if ((l->sec->flags & SEC_GROUP) == 0
+	    && bfd_coff_get_comdat_section (l->sec->owner, l->sec) == NULL
+	    && bfd_elf_match_symbols_in_sections (l->sec, s->sec))
+	  {
+	    s->linked = l->sec;
+	    return FALSE;
+	  }
+    }
+
+  return TRUE;
+}
+
+static bfd_boolean
+already_linked (asection *sec, asection *group)
+{
+  struct already_linked_section result;
+
+  result.sec = sec;
+  result.linked = NULL;
+
+  bfd_section_already_linked_table_traverse
+    (try_match_symbols_in_sections, &result);
+
+  if (result.linked)
+    {
+      sec->output_section = bfd_abs_section_ptr;
+      sec->kept_section = result.linked;
+
+      /* Also discard the group section.  */
+      if (group)
+	group->output_section = bfd_abs_section_ptr;
+
+      return TRUE;
+    }
+
+  return FALSE;
+}
+
 void
 _bfd_elf_section_already_linked (bfd *abfd, struct bfd_section * sec)
 {
@@ -9018,11 +9129,34 @@ _bfd_elf_section_already_linked (bfd *ab
   const char *name;
   struct bfd_section_already_linked *l;
   struct bfd_section_already_linked_hash_entry *already_linked_list;
+  asection *group;
+
+  /* A single member comdat group section may be discarded by a
+     linkonce section. See below.  */
+  if (sec->output_section == bfd_abs_section_ptr)
+    return;
 
   flags = sec->flags;
-  if ((flags & SEC_LINK_ONCE) == 0)
+
+  /* Check if it belongs to a section group.  */
+  group = elf_sec_group (sec);
+
+  /* Return if it isn't a linkonce section nor a member of a group.  A
+     comdat group section also has SEC_LINK_ONCE set.  */
+  if ((flags & SEC_LINK_ONCE) == 0 && group == NULL)
     return;
 
+  if (group)
+    {
+      /* If this is the member of a single member comdat group, check if
+	 the group should be discarded.  */
+      if (elf_next_in_group (sec) == sec
+	  && (group->flags & SEC_LINK_ONCE) != 0)
+	sec = group;
+      else
+	return;
+    }
+
   /* FIXME: When doing a relocatable link, we may have trouble
      copying relocations in other sections that refer to local symbols
      in the section being discarded.  Those relocations will have to
@@ -9051,7 +9185,7 @@ _bfd_elf_section_already_linked (bfd *ab
 	 group section. We match a group section with a group section,
 	 a linkonce section with a linkonce section, and ignore comdat
 	 section.  */
-      if ((sec->flags & SEC_GROUP) == (l->sec->flags & SEC_GROUP)
+      if ((flags & SEC_GROUP) == (l->sec->flags & SEC_GROUP)
 	  && bfd_coff_get_comdat_section (l->sec->owner, l->sec) == NULL)
 	{
 	  /* The section has already been linked.  See if we should
@@ -9087,12 +9221,42 @@ _bfd_elf_section_already_linked (bfd *ab
 	  sec->kept_section = l->sec;
 	  
 	  if (flags & SEC_GROUP)
-	    bfd_elf_discard_group (abfd, sec);
+	    {
+	      asection *first = elf_next_in_group (sec);
+	      asection *s = first;
+
+	      while (s != NULL)
+		{
+		  s->output_section = bfd_abs_section_ptr;
+		  /* Record which group discards it.  */
+		  s->kept_section = l->sec;
+		  s = elf_next_in_group (s);
+		  /* These lists are circular.  */
+		  if (s == first)
+		    break;
+		}
+	    }
 
 	  return;
 	}
     }
 
+  if (group)
+    {
+      /* If this is the member of a single member comdat group and the
+	 group hasn't be discarded, we check if it matches a linkonce
+	 section. We only record the discarded comdat group. Otherwise
+	 the undiscarded group will be discarded incorrectly later since
+	 itself has been recorded.  */
+      if (! already_linked (elf_next_in_group (sec), group))
+	return;
+    }
+  else
+    /* There is no direct match. But for linkonce section, we should
+       check if there is a match with comdat group member. We always
+       record the linkonce section, discarded or not.  */
+    already_linked (sec, group);
+  
   /* This is the first section with this name.  Record it.  */
   bfd_section_already_linked_table_insert (already_linked_list, sec);
 }
--- bfd/libbfd-in.h.group	2004-07-03 10:04:17.000000000 -0700
+++ bfd/libbfd-in.h	2004-07-03 10:04:17.000000000 -0700
@@ -679,3 +679,7 @@ extern struct bfd_section_already_linked
   bfd_section_already_linked_table_lookup (const char *);
 extern void bfd_section_already_linked_table_insert
   (struct bfd_section_already_linked_hash_entry *, asection *);
+extern void bfd_section_already_linked_table_traverse
+  (bfd_boolean (*) (struct bfd_section_already_linked_hash_entry *,
+		    void *), void *);
+
--- bfd/linker.c.group	2004-07-03 10:04:17.000000000 -0700
+++ bfd/linker.c	2004-07-03 10:04:17.174411323 -0700
@@ -2858,8 +2858,19 @@ DESCRIPTION
 static struct bfd_hash_table _bfd_section_already_linked_table;
 
 /* Support routines for the hash table used by section_already_linked,
-   initialize the table, lookup, fill in an entry and remove the
-   table.  */
+   initialize the table, traverse, lookup, fill in an entry and remove
+   the table.  */
+
+void
+bfd_section_already_linked_table_traverse
+  (bfd_boolean (*func) (struct bfd_section_already_linked_hash_entry *,
+			void *), void *info)
+{
+  bfd_hash_traverse (&_bfd_section_already_linked_table,
+		     (bfd_boolean (*) (struct bfd_hash_entry *,
+				       void *)) func,
+		     info);
+}
 
 struct bfd_section_already_linked_hash_entry *
 bfd_section_already_linked_table_lookup (const char *name)


More information about the Binutils mailing list