PR25333, GAS is slow processing -fdebug-types-sections

Message ID ZdlwW0ELL5gKvR+F@squeak.grove.modra.org
State New
Headers
Series PR25333, GAS is slow processing -fdebug-types-sections |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Patch is already merged

Commit Message

Alan Modra Feb. 24, 2024, 4:28 a.m. UTC
  gas needs to build lists of sections for each group.  This arranges to
build the lists earlier, so they can be used when looking for sections
that belong to a group.  Using the section hash table to find sections
by name, then by group isn't efficient when there are numerous groups
with the same section names.  Using a hash table to quickly find a
group, then searching by section name on a list for the group results
in a 100-fold speed improvement assembling the testcase in this PR.

To reduce the number of times we traverse the section list, the patch
also moves some processing done in elf_adjust_symtab for linked-to
section, to elf_frob_file.  This requires a testsuite change because
processing will stop before elf_frob_file if there is a parse error in
section21.s, ie. you'll only get the "junk at end of line" error, not
the "undefined linked-to symbol" errors.

	PR 25333
	* config/obj-elf.c (struct group_list, groups): Move earlier.
	(match_section): New function, extracted from..
	(get_section_by_match): ..here.
	(free_section_idx): Move earlier.
	(group_section_find, group_section_insert): New functions.
	(change_section): Use the above.
	(elf_set_group_name): New function.
	(obj_elf_attach_to_group): Use elf_set_group_name.
	(set_additional_section_info): Handle linked_to_symbol_name and
	stabs code, extracted from..
	(adjust_stab_sections): ..here,..
	(build_additional_section_info): ..and here.
	(elf_adjust_symtab): Don't call build_additional_section_info.
	(elf_frob_file): Adjust.
	* config/obj-elf.h (elf_set_group_name): Declare.
	* config/tc-xtensa.c (cache_literal_section): Use elf_set_group_name.
	(xtensa_make_property_section): Likewise.
	* testsuite/gas/elf/attach-1.d: Stricter group section matching,
	and changed group section ordering.
	* testsuite/gas/elf/attach-2.d: Stricter group section matching.
	* testsuite/gas/elf/attach-2.s: Provide section bar type.
	* testsuite/gas/elf/elf.exp: Run attach-2.
	* testsuite/gas/elf/section21.l: Update.
	* testsuite/gas/elf/section21.s: Don't check for a parse error.
  

Patch

diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 00c6e38acf9..e28ba0a62d5 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -493,16 +493,19 @@  static struct section_stack *section_stack;
 /* ELF section flags for unique sections.  */
 #define SEC_ASSEMBLER_SHF_MASK SHF_GNU_RETAIN
 
-/* Return TRUE iff SEC matches the section info INF.  */
+struct group_list
+{
+  asection **head;		/* Section lists.  */
+  unsigned int num_group;	/* Number of lists.  */
+  htab_t indexes; /* Maps group name to index in head array.  */
+};
+
+static struct group_list groups;
 
 static bool
-get_section_by_match (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *inf)
+match_section (const asection *sec, const struct elf_section_match *match)
 {
-  struct elf_section_match *match = (struct elf_section_match *) inf;
-  const char *gname = match->group_name;
-  const char *group_name = elf_group_name (sec);
-  const char *linked_to_symbol_name
-    = sec->map_head.linked_to_symbol_name;
+  const char *linked_to_symbol_name = sec->map_head.linked_to_symbol_name;
   unsigned int sh_info = elf_section_data (sec)->this_hdr.sh_info;
   bfd_vma sh_flags = (elf_section_data (sec)->this_hdr.sh_flags
 		      & SEC_ASSEMBLER_SHF_MASK);
@@ -512,10 +515,6 @@  get_section_by_match (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *inf)
 	  && ((bfd_section_flags (sec) & SEC_ASSEMBLER_SECTION_ID)
 	       == (match->flags & SEC_ASSEMBLER_SECTION_ID))
 	  && sec->section_id == match->section_id
-	  && (group_name == gname
-	      || (group_name != NULL
-		  && gname != NULL
-		  && strcmp (group_name, gname) == 0))
 	  && (linked_to_symbol_name == match->linked_to_symbol_name
 	      || (linked_to_symbol_name != NULL
 		  && match->linked_to_symbol_name != NULL
@@ -523,6 +522,93 @@  get_section_by_match (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *inf)
 			     match->linked_to_symbol_name) == 0)));
 }
 
+/* Return TRUE iff SEC matches the section info INF.  */
+
+static bool
+get_section_by_match (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *inf)
+{
+  struct elf_section_match *match = (struct elf_section_match *) inf;
+  const char *gname = match->group_name;
+  const char *group_name = elf_group_name (sec);
+
+  return ((group_name == gname
+	   || (group_name != NULL
+	       && gname != NULL
+	       && strcmp (group_name, gname) == 0))
+	  && match_section (sec, match));
+}
+
+static void
+free_section_idx (void *ent)
+{
+  string_tuple_t *tuple = ent;
+  free ((char *) tuple->value);
+}
+
+/* Go look in section lists kept per group for SEC_NAME with
+   properties given by MATCH.  If info for the group named by
+   MATCH->GROUP_NAME has been initialised, set GROUP_IDX.  */
+
+static asection *
+group_section_find (const struct elf_section_match *match,
+		    const char *sec_name,
+		    unsigned int **group_idx)
+{
+  if (!groups.indexes)
+    {
+      groups.num_group = 0;
+      groups.head = NULL;
+      groups.indexes = htab_create_alloc (16, hash_string_tuple, eq_string_tuple,
+					  free_section_idx, notes_calloc, NULL);
+      *group_idx = NULL;
+      return NULL;
+    }
+
+  *group_idx = str_hash_find (groups.indexes, match->group_name);
+  if (*group_idx == NULL)
+    return NULL;
+
+  asection *s;
+  for (s = groups.head[**group_idx]; s != NULL; s = elf_next_in_group (s))
+    if ((s->name == sec_name
+	 || strcmp (s->name, sec_name) == 0)
+	&& match_section (s, match))
+      break;
+  return s;
+}
+
+/* Insert SEC into section lists kept per group.  MATCH and GROUP_IDX
+   must be from a prior call to group_section_find.  */
+
+static void
+group_section_insert (const struct elf_section_match *match,
+		      asection *sec,
+		      unsigned int **group_idx)
+{
+  if (*group_idx != NULL)
+    {
+      elf_next_in_group (sec) = groups.head[**group_idx];
+      groups.head[**group_idx] = sec;
+      return;
+    }
+
+  unsigned int i = groups.num_group;
+  if ((i & 127) == 0)
+    groups.head = XRESIZEVEC (asection *, groups.head, i + 128);
+  groups.head[i] = sec;
+  groups.num_group += 1;
+
+  /* We keep the index into groups.head rather than the entry address
+     because groups.head may be realloc'd, and because str_hash values
+     are a void* we make a copy of the index.  Strictly speaking there
+     is no guarantee that void* can represent any int value, so doing
+     without the indirection by casting an int or even uintptr_t may
+     for example lose lsbs of the value.  */
+  unsigned int *idx_ptr = XNEW (unsigned int);
+  *idx_ptr = i;
+  str_hash_insert (groups.indexes, match->group_name, idx_ptr, 0);
+}
+
 /* Handle the .section pseudo-op.  This code supports two different
    syntaxes.
 
@@ -582,15 +668,23 @@  change_section (const char *name,
 
   obj_elf_section_change_hook ();
 
-  old_sec = bfd_get_section_by_name_if (stdoutput, name, get_section_by_match,
-					(void *) match_p);
+  unsigned int *group_idx = NULL;
+  if (match_p->group_name)
+    old_sec = group_section_find (match_p, name, &group_idx);
+  else
+    old_sec = bfd_get_section_by_name_if (stdoutput, name, get_section_by_match,
+					  (void *) match_p);
   if (old_sec)
     {
       sec = old_sec;
       subseg_set (sec, new_subsection);
     }
   else
-    sec = subseg_force_new (name, new_subsection);
+    {
+      sec = subseg_force_new (name, new_subsection);
+      if (match_p->group_name)
+	group_section_insert (match_p, sec, &group_idx);
+    }
 
   bed = get_elf_backend_data (stdoutput);
   ssect = (*bed->get_sec_type_attr) (stdoutput, sec);
@@ -1084,6 +1178,29 @@  obj_elf_section_name (void)
   return name;
 }
 
+/* Arrange to put SEC, known to be in group GNAME into the per-group
+   section lists kept by gas.  */
+
+void
+elf_set_group_name (asection *sec, const char *gname)
+{
+  elf_group_name (sec) = gname;
+  elf_section_flags (sec) |= SHF_GROUP;
+
+  struct elf_section_match match;
+  match.group_name = gname;
+  match.linked_to_symbol_name = sec->map_head.linked_to_symbol_name;
+  match.section_id = sec->section_id;
+  match.sh_info = elf_section_data (sec)->this_hdr.sh_info;
+  match.sh_flags = (elf_section_data (sec)->this_hdr.sh_flags
+		    & SEC_ASSEMBLER_SHF_MASK);
+  match.flags = bfd_section_flags (sec) & SEC_ASSEMBLER_SECTION_ID;
+
+  unsigned int *group_idx;
+  if (!group_section_find (&match, sec->name, &group_idx))
+    group_section_insert (&match, sec, &group_idx);
+}
+
 static void
 obj_elf_attach_to_group (int dummy ATTRIBUTE_UNUSED)
 {
@@ -1102,9 +1219,7 @@  obj_elf_attach_to_group (int dummy ATTRIBUTE_UNUSED)
 		 bfd_section_name (now_seg), elf_group_name (now_seg));
       return;
     }
-
-  elf_group_name (now_seg) = gname;
-  elf_section_flags (now_seg) |= SHF_GROUP;
+  elf_set_group_name (now_seg, gname);
 }
 
 void
@@ -2588,11 +2703,27 @@  obj_elf_init_stab_section (segT stab, segT stabstr)
 
 #endif
 
-/* Fill in the counts in the first entry in a .stabs section.  */
+/* Called via bfd_map_over_sections.  If SEC's linked_to_symbol_name
+   isn't NULL, set up its linked-to section.
+   For .stabs section, fill in the counts in the first entry.  */
 
 static void
-adjust_stab_sections (bfd *abfd, asection *sec, void *xxx ATTRIBUTE_UNUSED)
+set_additional_section_info (bfd *abfd,
+			     asection *sec,
+			     void *inf ATTRIBUTE_UNUSED)
 {
+  if (sec->map_head.linked_to_symbol_name)
+    {
+      symbolS *linked_to_sym;
+      linked_to_sym = symbol_find (sec->map_head.linked_to_symbol_name);
+      if (!linked_to_sym || !S_IS_DEFINED (linked_to_sym))
+	as_bad (_("undefined linked-to symbol `%s' on section `%s'"),
+		sec->map_head.linked_to_symbol_name,
+		bfd_section_name (sec));
+      else
+	elf_linked_to_section (sec) = S_GET_SEGMENT (linked_to_sym);
+    }
+
   char *name;
   asection *strsec;
   char *p;
@@ -2811,80 +2942,6 @@  elf_fixup_removed_symbol (symbolS **sympp)
   *sympp = symp;
 }
 
-struct group_list
-{
-  asection **head;		/* Section lists.  */
-  unsigned int num_group;	/* Number of lists.  */
-  htab_t indexes; /* Maps group name to index in head array.  */
-};
-
-static struct group_list groups;
-
-/* Called via bfd_map_over_sections.  If SEC is a member of a group,
-   add it to a list of sections belonging to the group.  INF is a
-   pointer to a struct group_list, which is where we store the head of
-   each list.  If its link_to_symbol_name isn't NULL, set up its
-   linked-to section.  */
-
-static void
-build_additional_section_info (bfd *abfd ATTRIBUTE_UNUSED,
-				  asection *sec, void *inf)
-{
-  struct group_list *list = (struct group_list *) inf;
-  const char *group_name = elf_group_name (sec);
-  unsigned int i;
-  unsigned int *elem_idx;
-  unsigned int *idx_ptr;
-
-  if (sec->map_head.linked_to_symbol_name)
-    {
-      symbolS *linked_to_sym;
-      linked_to_sym = symbol_find (sec->map_head.linked_to_symbol_name);
-      if (!linked_to_sym || !S_IS_DEFINED (linked_to_sym))
-	as_bad (_("undefined linked-to symbol `%s' on section `%s'"),
-		sec->map_head.linked_to_symbol_name,
-		bfd_section_name (sec));
-      else
-	elf_linked_to_section (sec) = S_GET_SEGMENT (linked_to_sym);
-    }
-
-  if (group_name == NULL)
-    return;
-
-  /* If this group already has a list, add the section to the head of
-     the list.  */
-  elem_idx = (unsigned int *) str_hash_find (list->indexes, group_name);
-  if (elem_idx != NULL)
-    {
-      elf_next_in_group (sec) = list->head[*elem_idx];
-      list->head[*elem_idx] = sec;
-      return;
-    }
-
-  /* New group.  Make the arrays bigger in chunks to minimize calls to
-     realloc.  */
-  i = list->num_group;
-  if ((i & 127) == 0)
-    {
-      unsigned int newsize = i + 128;
-      list->head = XRESIZEVEC (asection *, list->head, newsize);
-    }
-  list->head[i] = sec;
-  list->num_group += 1;
-
-  /* Add index to hash.  */
-  idx_ptr = XNEW (unsigned int);
-  *idx_ptr = i;
-  str_hash_insert (list->indexes, group_name, idx_ptr, 0);
-}
-
-static void
-free_section_idx (void *ent)
-{
-  string_tuple_t *tuple = ent;
-  free ((char *) tuple->value);
-}
-
 /* Create symbols for group signature.  */
 
 void
@@ -2892,14 +2949,6 @@  elf_adjust_symtab (void)
 {
   unsigned int i;
 
-  /* Go find section groups.  */
-  groups.num_group = 0;
-  groups.head = NULL;
-  groups.indexes = htab_create_alloc (16, hash_string_tuple, eq_string_tuple,
-				      free_section_idx, notes_calloc, NULL);
-  bfd_map_over_sections (stdoutput, build_additional_section_info,
-			 &groups);
-
   /* Make the SHT_GROUP sections that describe each section group.  We
      can't set up the section contents here yet, because elf section
      indices have yet to be calculated.  elf.c:set_group_contents does
@@ -2966,7 +3015,7 @@  elf_adjust_symtab (void)
 void
 elf_frob_file (void)
 {
-  bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
+  bfd_map_over_sections (stdoutput, set_additional_section_info, NULL);
 
 #ifdef elf_tc_final_processing
   elf_tc_final_processing ();
diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
index 0187f6ebfad..7b5d2fe2b80 100644
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -297,4 +297,6 @@  extern asection *elf_com_section_ptr;
 extern symbolS * elf_common_parse (int ignore ATTRIBUTE_UNUSED, symbolS *symbolP,
 				   addressT size);
 
+extern void elf_set_group_name (asection *, const char *);
+
 #endif /* _OBJ_ELF_H */
diff --git a/gas/config/tc-xtensa.c b/gas/config/tc-xtensa.c
index 188715d9619..e051bb9265b 100644
--- a/gas/config/tc-xtensa.c
+++ b/gas/config/tc-xtensa.c
@@ -11718,10 +11718,11 @@  cache_literal_section (bool use_abs_literals)
 	       | (linkonce ? (SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD) : 0)
 	       | (use_abs_literals ? SEC_DATA : SEC_CODE));
 
-      elf_group_name (seg) = group_name;
-
       bfd_set_section_flags (seg, flags);
       bfd_set_section_alignment (seg, 2);
+
+      if (group_name)
+	elf_set_group_name (seg, group_name);
     }
 
   *pcached = seg;
@@ -11813,7 +11814,8 @@  xtensa_make_property_section (asection *sec, const char *base_name)
       if (! prop_sec)
 	return 0;
 
-      elf_group_name (prop_sec) = elf_group_name (sec);
+      if (elf_group_name (sec))
+	elf_set_group_name (prop_sec, elf_group_name (sec));
     }
 
   free (prop_sec_name);
diff --git a/gas/testsuite/gas/elf/attach-1.d b/gas/testsuite/gas/elf/attach-1.d
index 053783ea1a0..b6b76f5c234 100644
--- a/gas/testsuite/gas/elf/attach-1.d
+++ b/gas/testsuite/gas/elf/attach-1.d
@@ -3,9 +3,9 @@ 
 #source: attach-1.s
 
 #...
-group section \[    1\] `\.group' \[foo\.group\] contains . sections:
+group section \[    1\] `\.group' \[foo\.group\] contains [24] sections:
    \[Index\]    Name
-   \[    .\]   .*
-   \[    .\]   foo
-#pass
-
+.* foo
+.* (\.text|P)
+#?.* \.xt\.prop
+#?.* \.rela\.xt\.prop
diff --git a/gas/testsuite/gas/elf/attach-2.d b/gas/testsuite/gas/elf/attach-2.d
index 4aa52116357..68a1aab4c03 100644
--- a/gas/testsuite/gas/elf/attach-2.d
+++ b/gas/testsuite/gas/elf/attach-2.d
@@ -1,11 +1,11 @@ 
 #readelf: --section-groups
-#name: Attaching a section to a non-existant group
+#name: Attaching a section to group defined later
 #source: attach-2.s
 
 #...
-group section \[    1\] `\.group' \[foo\.group\] contains 2 sections:
+group section \[    1\] `\.group' \[foo\.group\] contains [24] sections:
    \[Index\]    Name
-   \[    .\]   bar
-   \[    .\]   foo
-#pass
-
+.* bar
+.* foo
+#?.* \.xt\.prop
+#?.* \.rela\.xt\.prop
diff --git a/gas/testsuite/gas/elf/attach-2.s b/gas/testsuite/gas/elf/attach-2.s
index 4a5663f3872..d62a81b25fe 100644
--- a/gas/testsuite/gas/elf/attach-2.s
+++ b/gas/testsuite/gas/elf/attach-2.s
@@ -1,9 +1,9 @@ 
-	.section bar
+	.section bar, "ax", %progbits
 	.nop
 	.attach_to_group foo.group
 	
 	.section foo, "G", %note , foo.group
 	.word 0
 
-	.section bar
+	.section bar, "ax", %progbits
 	.nop
diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
index e03dc2bf00c..c828c3af25c 100644
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -149,6 +149,7 @@  if { [is_elf_format] } then {
     run_dump_test "group3"
 
     run_dump_test "attach-1"
+    run_dump_test "attach-2"
     run_dump_test "attach-err"
 
     switch -glob $target_triplet {
diff --git a/gas/testsuite/gas/elf/section21.l b/gas/testsuite/gas/elf/section21.l
index 53f04151d33..b4d1283f312 100644
--- a/gas/testsuite/gas/elf/section21.l
+++ b/gas/testsuite/gas/elf/section21.l
@@ -1,5 +1,3 @@ 
 [^:]*: Assembler messages:
-[^:]*:11: Error: junk at end of line, first unrecognized character is `f'
-#...
 [^:]*: Error: undefined linked-to symbol `bar' on section `__patchable_function_entries'
 [^:]*: Error: undefined linked-to symbol `foo' on section `__patchable_function_entries'
diff --git a/gas/testsuite/gas/elf/section21.s b/gas/testsuite/gas/elf/section21.s
index ae5f848b299..e874154e54e 100644
--- a/gas/testsuite/gas/elf/section21.s
+++ b/gas/testsuite/gas/elf/section21.s
@@ -2,14 +2,9 @@ 
 	.dc.a	.LPFE1
 	.text
 .LPFE1:
-	.byte 0
+	.dc.a 0
 	.section __patchable_function_entries,"awo",%progbits,foo
 	.dc.a	.LPFE2
 	.text
 .LPFE2:
 	.dc.a foo
-	.section __patchable_function_entries,"awo",%progbits,1foo
-	.dc.a	.LPFE3
-	.text
-.LPFE3:
-	.byte 0