Re: [PATCH v3 4/5] Refactor: Move some generic code out of .gdb_index code

[Pedro Alves <palves@redhat.com>]

On 06/19/2017 09:55 PM, Jan Kratochvil wrote:
> Hi,
> 
> just for the next patch.
> 
> 
> Jan
> 
> 
> gdb/ChangeLog
> 2017-05-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* dwarf2read.c (create_cu_from_index_list): New from ...
> 	(create_cus_from_index_list): ... this function, use it.
> 	(dw_expand_symtabs_matching_file_matcher)
> 	(dw2_expand_symtabs_matching_one): New from ...
> 	(dw2_expand_symtabs_matching): ... this function, use them.

This code had unfortunately changed too much for the patch to
apply, but I managed to reconstruct it manually.

Here's what I pushed.

From 4b514bc80885fa91e57b9d80f74ea267df343c46 Mon Sep 17 00:00:00 2001
From: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Fri, 8 Dec 2017 22:44:11 +0000
Subject: [PATCH 4/9] Refactor: Move some generic code out of .gdb_index code

Preparation for the next patch.

gdb/ChangeLog
2017-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (create_cu_from_index_list): New from ...
	(create_cus_from_index_list): ... this function, use it.
	(dw_expand_symtabs_matching_file_matcher)
	(dw2_expand_symtabs_matching_one): New from ...
	(dw2_expand_symtabs_matching): ... this function, use them.
---
 gdb/ChangeLog    |   8 ++
 gdb/dwarf2read.c | 220 +++++++++++++++++++++++++++++++++----------------------
 2 files changed, 139 insertions(+), 89 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b6f3714..5d596f0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2017-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	* dwarf2read.c (create_cu_from_index_list): New from ...
+	(create_cus_from_index_list): ... this function, use it.
+	(dw_expand_symtabs_matching_file_matcher)
+	(dw2_expand_symtabs_matching_one): New from ...
+	(dw2_expand_symtabs_matching): ... this function, use them.
+
+2017-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* defs.h (elf_sym_fns_lazy_psyms, elf_sym_fns_gdb_index): Move here
 	declarations from elfread.c.
 	(dwarf2_initialize_objfile): Change return value.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 798af72..80d4857 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3061,6 +3061,29 @@ dw2_get_cu (int index)
   return dwarf2_per_objfile->all_comp_units[index];
 }
 
+/* Return a new dwarf2_per_cu_data allocated on OBJFILE's
+   objfile_obstack, and constructed with the specified field
+   values.  */
+
+static dwarf2_per_cu_data *
+create_cu_from_index_list (struct objfile *objfile,
+                          struct dwarf2_section_info *section,
+                          int is_dwz,
+                          sect_offset sect_off, ULONGEST length)
+{
+  dwarf2_per_cu_data *the_cu
+    = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+                     struct dwarf2_per_cu_data);
+  the_cu->sect_off = sect_off;
+  the_cu->length = length;
+  the_cu->objfile = objfile;
+  the_cu->section = section;
+  the_cu->v.quick = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+                                   struct dwarf2_per_cu_quick_data);
+  the_cu->is_dwz = is_dwz;
+  return the_cu;
+}
+
 /* A helper for create_cus_from_index that handles a given list of
    CUs.  */
 
@@ -3082,17 +3105,8 @@ create_cus_from_index_list (struct objfile *objfile,
       ULONGEST length = extract_unsigned_integer (cu_list + 8, 8, BFD_ENDIAN_LITTLE);
       cu_list += 2 * 8;
 
-      dwarf2_per_cu_data *the_cu
-	= OBSTACK_ZALLOC (&objfile->objfile_obstack,
-			  struct dwarf2_per_cu_data);
-      the_cu->sect_off = sect_off;
-      the_cu->length = length;
-      the_cu->objfile = objfile;
-      the_cu->section = section;
-      the_cu->v.quick = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-					struct dwarf2_per_cu_quick_data);
-      the_cu->is_dwz = is_dwz;
-      dwarf2_per_objfile->all_comp_units[base_offset + i / 2] = the_cu;
+      dwarf2_per_objfile->all_comp_units[base_offset + i / 2]
+	= create_cu_from_index_list (objfile, section, is_dwz, sect_off, length);
     }
 }
 
@@ -4939,6 +4953,31 @@ run_test ()
 
 #endif /* GDB_SELF_TEST */
 
+/* If FILE_MATCHER is NULL or if PER_CU has
+   dwarf2_per_cu_quick_data::MARK set (see
+   dw_expand_symtabs_matching_file_matcher), expand the CU and call
+   EXPANSION_NOTIFY on it.  */
+
+static void
+dw2_expand_symtabs_matching_one
+  (struct dwarf2_per_cu_data *per_cu,
+   gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
+   gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify)
+{
+  if (file_matcher == NULL || per_cu->v.quick->mark)
+    {
+      bool symtab_was_null
+	= (per_cu->v.quick->compunit_symtab == NULL);
+
+      dw2_instantiate_symtab (per_cu);
+
+      if (expansion_notify != NULL
+	  && symtab_was_null
+	  && per_cu->v.quick->compunit_symtab != NULL)
+	expansion_notify (per_cu->v.quick->compunit_symtab);
+    }
+}
+
 /* Helper for dw2_expand_matching symtabs.  Called on each symbol
    matched, to expand corresponding CUs that were marked.  IDX is the
    index of the symbol name that matched.  */
@@ -5016,109 +5055,112 @@ dw2_expand_marked_cus
 	}
 
       per_cu = dw2_get_cutu (cu_index);
-      if (file_matcher == NULL || per_cu->v.quick->mark)
-	{
-	  int symtab_was_null =
-	    (per_cu->v.quick->compunit_symtab == NULL);
-
-	  dw2_instantiate_symtab (per_cu);
-
-	  if (expansion_notify != NULL
-	      && symtab_was_null
-	      && per_cu->v.quick->compunit_symtab != NULL)
-	    expansion_notify (per_cu->v.quick->compunit_symtab);
-	}
+      dw2_expand_symtabs_matching_one (per_cu, file_matcher,
+				       expansion_notify);
     }
 }
 
+/* If FILE_MATCHER is non-NULL, set all the
+   dwarf2_per_cu_quick_data::MARK of the current DWARF2_PER_OBJFILE
+   that match FILE_MATCHER.  */
+
 static void
-dw2_expand_symtabs_matching
-  (struct objfile *objfile,
-   gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
-   const lookup_name_info &lookup_name,
-   gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
-   gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
-   enum search_domain kind)
+dw_expand_symtabs_matching_file_matcher
+  (gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher)
 {
-  int i;
-
-  dw2_setup (objfile);
-
-  /* index_table is NULL if OBJF_READNOW.  */
-  if (!dwarf2_per_objfile->index_table)
+  if (file_matcher == NULL)
     return;
 
-  if (file_matcher != NULL)
-    {
-      htab_up visited_found (htab_create_alloc (10, htab_hash_pointer,
+  objfile *const objfile = dwarf2_per_objfile->objfile;
+
+  htab_up visited_found (htab_create_alloc (10, htab_hash_pointer,
+					    htab_eq_pointer,
+					    NULL, xcalloc, xfree));
+  htab_up visited_not_found (htab_create_alloc (10, htab_hash_pointer,
 						htab_eq_pointer,
 						NULL, xcalloc, xfree));
-      htab_up visited_not_found (htab_create_alloc (10, htab_hash_pointer,
-						    htab_eq_pointer,
-						    NULL, xcalloc, xfree));
 
-      /* The rule is CUs specify all the files, including those used by
-	 any TU, so there's no need to scan TUs here.  */
+  /* The rule is CUs specify all the files, including those used by
+     any TU, so there's no need to scan TUs here.  */
 
-      for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
-	{
-	  int j;
-	  struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i);
-	  struct quick_file_names *file_data;
-	  void **slot;
+  for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
+    {
+      int j;
+      struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i);
+      struct quick_file_names *file_data;
+      void **slot;
 
-	  QUIT;
+      QUIT;
 
-	  per_cu->v.quick->mark = 0;
+      per_cu->v.quick->mark = 0;
 
-	  /* We only need to look at symtabs not already expanded.  */
-	  if (per_cu->v.quick->compunit_symtab)
-	    continue;
+      /* We only need to look at symtabs not already expanded.  */
+      if (per_cu->v.quick->compunit_symtab)
+	continue;
 
-	  file_data = dw2_get_file_names (per_cu);
-	  if (file_data == NULL)
-	    continue;
+      file_data = dw2_get_file_names (per_cu);
+      if (file_data == NULL)
+	continue;
 
-	  if (htab_find (visited_not_found.get (), file_data) != NULL)
-	    continue;
-	  else if (htab_find (visited_found.get (), file_data) != NULL)
+      if (htab_find (visited_not_found.get (), file_data) != NULL)
+	continue;
+      else if (htab_find (visited_found.get (), file_data) != NULL)
+	{
+	  per_cu->v.quick->mark = 1;
+	  continue;
+	}
+
+      for (j = 0; j < file_data->num_file_names; ++j)
+	{
+	  const char *this_real_name;
+
+	  if (file_matcher (file_data->file_names[j], false))
 	    {
 	      per_cu->v.quick->mark = 1;
-	      continue;
+	      break;
 	    }
 
-	  for (j = 0; j < file_data->num_file_names; ++j)
+	  /* Before we invoke realpath, which can get expensive when many
+	     files are involved, do a quick comparison of the basenames.  */
+	  if (!basenames_may_differ
+	      && !file_matcher (lbasename (file_data->file_names[j]),
+				true))
+	    continue;
+
+	  this_real_name = dw2_get_real_path (objfile, file_data, j);
+	  if (file_matcher (this_real_name, false))
 	    {
-	      const char *this_real_name;
+	      per_cu->v.quick->mark = 1;
+	      break;
+	    }
+	}
 
-	      if (file_matcher (file_data->file_names[j], false))
-		{
-		  per_cu->v.quick->mark = 1;
-		  break;
-		}
+      slot = htab_find_slot (per_cu->v.quick->mark
+			     ? visited_found.get ()
+			     : visited_not_found.get (),
+			     file_data, INSERT);
+      *slot = file_data;
+    }
+}
 
-	      /* Before we invoke realpath, which can get expensive when many
-		 files are involved, do a quick comparison of the basenames.  */
-	      if (!basenames_may_differ
-		  && !file_matcher (lbasename (file_data->file_names[j]),
-				    true))
-		continue;
+static void
+dw2_expand_symtabs_matching
+  (struct objfile *objfile,
+   gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
+   const lookup_name_info &lookup_name,
+   gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
+   gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
+   enum search_domain kind)
+{
+  int i;
 
-	      this_real_name = dw2_get_real_path (objfile, file_data, j);
-	      if (file_matcher (this_real_name, false))
-		{
-		  per_cu->v.quick->mark = 1;
-		  break;
-		}
-	    }
+  dw2_setup (objfile);
 
-	  slot = htab_find_slot (per_cu->v.quick->mark
-				 ? visited_found.get ()
-				 : visited_not_found.get (),
-				 file_data, INSERT);
-	  *slot = file_data;
-	}
-    }
+  /* index_table is NULL if OBJF_READNOW.  */
+  if (!dwarf2_per_objfile->index_table)
+    return;
+
+  dw_expand_symtabs_matching_file_matcher (file_matcher);
 
   mapped_index &index = *dwarf2_per_objfile->index_table;
 
-- 
2.5.5