Re: PR 161/251 patch causing massive link time regression

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Oct 13, 2004 at 05:21:07PM -0700, H. J. Lu wrote:
> It should help. We can also require the single member comdat group,
> which may match a linkonce section, should have the group sigature
> the same as the section name of the linkonce section. We don't have
> to check symbols between 2 sections.
> 
> BTW, the problem my patch tries to fix is
> 
> 	.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"axG",@progbits,__i686.get_pc_thunk.bx,comdat
> 
> vs.
> 
>         .section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"ax",@progbits
> 
> linker should treat them the same. If gcc can generate
> 
> 	.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"axG",@progbits,.gnu.linkonce.t.__i686.get_pc_thunk.bx,comdat
> 
> it will help linker a lot.

Would the following patch be enough?

E.g. following combinations would work:
.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"axG",@progbits,__i686.get_pc_thunk.bx,comdat
.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"ax",@progbits

.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"axG",@progbits,.gnu.linkonce.t.__i686.get_pc_thunk.bx,comdat
.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"ax",@progbits

.section .text,"axG",@progbits,__i686.get_pc_thunk.bx,comdat
.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"ax",@progbits

.section .text,"axG",@progbits,.gnu.linkonce.t.__i686.get_pc_thunk.bx,comdat
.section .gnu.linkonce.t.__i686.get_pc_thunk.bx,"ax",@progbits

though your testcase in PR 161 will not work (the comdat group is there
.__i686.get_pc_thunk.bx, not sure why the dot at the begining).
At least the intent of the patch (unless I goofed up) is doing exactly what
linker did without this patch, but only try to match sections that have the
same base section name (resp. comdat group name), where base section name
is section name with ^\.gnu\.linkonce\.[^.]*\. prefix if present stripped.

2004-10-14  Jakub Jelinek  <jakub@redhat.com>

	* elflink.c (struct already_linked_section): Removed.
	(try_match_symbols_in_sections, already_linked): Removed.
	(_bfd_elf_section_already_linked): Skip ^\.gnu\.linkonce\.[^.]*\.
	prefix of section names when finding already_linked_table
	chain.  Compare section names.  Instead of calling already_linked,
	do it inline and only for sections in the same already_linked_list.

--- bfd/elflink.c.jj	2004-10-13 10:02:09.000000000 +0200
+++ bfd/elflink.c	2004-10-14 14:29:17.000000000 +0200
@@ -9268,88 +9268,11 @@ 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)
 {
   flagword flags;
-  const char *name;
+  const char *name, *p;
   struct bfd_section_already_linked *l;
   struct bfd_section_already_linked_hash_entry *already_linked_list;
   asection *group;
@@ -9399,7 +9322,13 @@ _bfd_elf_section_already_linked (bfd *ab
 
   name = bfd_get_section_name (abfd, sec);
 
-  already_linked_list = bfd_section_already_linked_table_lookup (name);
+  if (strncmp (name, ".gnu.linkonce.", sizeof (".gnu.linkonce.") - 1) == 0
+      && (p = strchr (name + sizeof (".gnu.linkonce.") - 1, '.')) != NULL)
+    p++;
+  else
+    p = name;
+
+  already_linked_list = bfd_section_already_linked_table_lookup (p);
 
   for (l = already_linked_list->entry; l != NULL; l = l->next)
     {
@@ -9409,10 +9338,11 @@ _bfd_elf_section_already_linked (bfd *ab
 	 a linkonce section with a linkonce section, and ignore comdat
 	 section.  */
       if ((flags & SEC_GROUP) == (l->sec->flags & SEC_GROUP)
+	  && strcmp (name, l->sec->name) == 0
 	  && bfd_coff_get_comdat_section (l->sec->owner, l->sec) == NULL)
 	{
 	  /* The section has already been linked.  See if we should
-             issue a warning.  */
+	     issue a warning.  */
 	  switch (flags & SEC_LINK_DUPLICATES)
 	    {
 	    default:
@@ -9501,15 +9431,39 @@ _bfd_elf_section_already_linked (bfd *ab
 	 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))
+      for (l = already_linked_list->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,
+						  elf_next_in_group (sec)))
+	  {
+	    elf_next_in_group (sec)->output_section = bfd_abs_section_ptr;
+	    elf_next_in_group (sec)->kept_section = l->sec;
+	    group->output_section = bfd_abs_section_ptr;
+	    break;
+	  }
+      if (l == NULL)
 	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);
-  
+    for (l = already_linked_list->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, sec))
+	    {
+	      sec->output_section = bfd_abs_section_ptr;
+	      sec->kept_section = l->sec;
+	      break;
+	    }
+	}
+
   /* This is the first section with this name.  Record it.  */
   bfd_section_already_linked_table_insert (already_linked_list, sec);
 }


	Jakub