[2/5] Replace dw2_get_cu/dw2_get_cutu with methods of dwarf2_per_objfile

Message ID 20180401000444.13490-3-simon.marchi@polymtl.ca
State New
Headers show
Series
  • Random dwarf2read.c improvements
Related show

Commit Message

Simon Marchi April 1, 2018, 12:04 a.m.
Those two functions look like good candidates to become methods of
dwarf2_per_objfile.  I did that, and added get_tu as well.  When
replacing usages of dw2_get_cutu, I changed some instances to get_cutu
and others to get_cu, when appropriate (when we know we want a CU and
not a TU).

gdb/ChangeLog:

	* dwarf2read.h (struct signatured_type): Forward declare.
	(struct dwarf2_per_objfile) <get_cutu, get_cu, get_tu>:
	New methods.
	* dwarf2read.c (dwarf2_per_objfile::get_cutu): Rename from...
	(dw2_get_cutu): ...this.
	(dwarf2_per_objfile::get_cu): Rename from...
	(dw2_get_cu): ...this.
	(dwarf2_per_objfile::get_tu): New.
	(create_addrmap_from_index): Adjust.
	(create_addrmap_from_aranges): Adjust.
	(dw2_find_last_source_symtab): Adjust.
	(dw2_map_symtabs_matching_filename): Adjust.
	(dw2_symtab_iter_next): Adjust.
	(dw2_print_stats): Adjust.
	(dw2_expand_all_symtabs): Adjust.
	(dw2_expand_symtabs_with_fullname): Adjust.
	(dw2_expand_marked_cus): Adjust.
	(dw_expand_symtabs_matching_file_matcher): Adjust.
	(dw2_map_symbol_filenames): Adjust.
	(dw2_debug_names_iterator::next): Adjust.
	(dwarf2_initialize_objfile): Adjust.
	(set_partial_user): Adjust.
	(dwarf2_build_psymtabs_hard): Adjust.
---
 gdb/dwarf2read.c | 86 +++++++++++++++++++++++++-------------------------------
 gdb/dwarf2read.h | 25 ++++++++++++++++
 2 files changed, 64 insertions(+), 47 deletions(-)

-- 
2.16.3

Comments

Joel Brobecker April 2, 2018, 4:17 p.m. | #1
Hi Simon,

On Sat, Mar 31, 2018 at 08:04:41PM -0400, Simon Marchi wrote:
> Those two functions look like good candidates to become methods of

> dwarf2_per_objfile.  I did that, and added get_tu as well.  When

> replacing usages of dw2_get_cutu, I changed some instances to get_cutu

> and others to get_cu, when appropriate (when we know we want a CU and

> not a TU).

> 

> gdb/ChangeLog:

> 

> 	* dwarf2read.h (struct signatured_type): Forward declare.

> 	(struct dwarf2_per_objfile) <get_cutu, get_cu, get_tu>:

> 	New methods.

> 	* dwarf2read.c (dwarf2_per_objfile::get_cutu): Rename from...

> 	(dw2_get_cutu): ...this.

> 	(dwarf2_per_objfile::get_cu): Rename from...

> 	(dw2_get_cu): ...this.

> 	(dwarf2_per_objfile::get_tu): New.

> 	(create_addrmap_from_index): Adjust.

> 	(create_addrmap_from_aranges): Adjust.

> 	(dw2_find_last_source_symtab): Adjust.

> 	(dw2_map_symtabs_matching_filename): Adjust.

> 	(dw2_symtab_iter_next): Adjust.

> 	(dw2_print_stats): Adjust.

> 	(dw2_expand_all_symtabs): Adjust.

> 	(dw2_expand_symtabs_with_fullname): Adjust.

> 	(dw2_expand_marked_cus): Adjust.

> 	(dw_expand_symtabs_matching_file_matcher): Adjust.

> 	(dw2_map_symbol_filenames): Adjust.

> 	(dw2_debug_names_iterator::next): Adjust.

> 	(dwarf2_initialize_objfile): Adjust.

> 	(set_partial_user): Adjust.

> 	(dwarf2_build_psymtabs_hard): Adjust.


Got time to look at this one to (and discover the dw2_get_cutu function).
The patch looks good to me. Not only that, I see that you changed some
of the "get_cutu" calls to either ::get_cu or ::get_tu when there was
enough context to know which one it was that we were handling. This is
a very nice enhancement, in my opinion.

That's all the time I have to review this patch series for now.
I may or may not be able to review the rest, so please anyone feel
free to take a look.

-- 
Joel
Simon Marchi April 2, 2018, 5:21 p.m. | #2
On 2018-04-02 12:17, Joel Brobecker wrote:
> Hi Simon,

> 

> On Sat, Mar 31, 2018 at 08:04:41PM -0400, Simon Marchi wrote:

>> Those two functions look like good candidates to become methods of

>> dwarf2_per_objfile.  I did that, and added get_tu as well.  When

>> replacing usages of dw2_get_cutu, I changed some instances to get_cutu

>> and others to get_cu, when appropriate (when we know we want a CU and

>> not a TU).

>> 

>> gdb/ChangeLog:

>> 

>> 	* dwarf2read.h (struct signatured_type): Forward declare.

>> 	(struct dwarf2_per_objfile) <get_cutu, get_cu, get_tu>:

>> 	New methods.

>> 	* dwarf2read.c (dwarf2_per_objfile::get_cutu): Rename from...

>> 	(dw2_get_cutu): ...this.

>> 	(dwarf2_per_objfile::get_cu): Rename from...

>> 	(dw2_get_cu): ...this.

>> 	(dwarf2_per_objfile::get_tu): New.

>> 	(create_addrmap_from_index): Adjust.

>> 	(create_addrmap_from_aranges): Adjust.

>> 	(dw2_find_last_source_symtab): Adjust.

>> 	(dw2_map_symtabs_matching_filename): Adjust.

>> 	(dw2_symtab_iter_next): Adjust.

>> 	(dw2_print_stats): Adjust.

>> 	(dw2_expand_all_symtabs): Adjust.

>> 	(dw2_expand_symtabs_with_fullname): Adjust.

>> 	(dw2_expand_marked_cus): Adjust.

>> 	(dw_expand_symtabs_matching_file_matcher): Adjust.

>> 	(dw2_map_symbol_filenames): Adjust.

>> 	(dw2_debug_names_iterator::next): Adjust.

>> 	(dwarf2_initialize_objfile): Adjust.

>> 	(set_partial_user): Adjust.

>> 	(dwarf2_build_psymtabs_hard): Adjust.

> 

> Got time to look at this one to (and discover the dw2_get_cutu 

> function).

> The patch looks good to me. Not only that, I see that you changed some

> of the "get_cutu" calls to either ::get_cu or ::get_tu when there was

> enough context to know which one it was that we were handling. This is

> a very nice enhancement, in my opinion.

> 

> That's all the time I have to review this patch series for now.

> I may or may not be able to review the rest, so please anyone feel

> free to take a look.


Thanks, your input is appreciated!

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index c2fed7f60109..695799e96f4f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2915,43 +2915,39 @@  dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
   return per_cu->v.quick->compunit_symtab;
 }
 
-/* Return the CU/TU given its index.
+/* See declaration.  */
 
-   This is intended for loops like:
+dwarf2_per_cu_data *
+dwarf2_per_objfile::get_cutu (int index)
+{
+  if (index >= this->n_comp_units)
+    {
+      index -= this->n_comp_units;
+      gdb_assert (index < this->n_type_units);
+      return &this->all_type_units[index]->per_cu;
+    }
 
-   for (i = 0; i < (dwarf2_per_objfile->n_comp_units
-		    + dwarf2_per_objfile->n_type_units); ++i)
-     {
-       struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (i);
+  return this->all_comp_units[index];
+}
 
-       ...;
-     }
-*/
+/* See declaration.  */
 
-static struct dwarf2_per_cu_data *
-dw2_get_cutu (struct dwarf2_per_objfile *dwarf2_per_objfile,
-	      int index)
+dwarf2_per_cu_data *
+dwarf2_per_objfile::get_cu (int index)
 {
-  if (index >= dwarf2_per_objfile->n_comp_units)
-    {
-      index -= dwarf2_per_objfile->n_comp_units;
-      gdb_assert (index < dwarf2_per_objfile->n_type_units);
-      return &dwarf2_per_objfile->all_type_units[index]->per_cu;
-    }
+  gdb_assert (index >= 0 && index < this->n_comp_units);
 
-  return dwarf2_per_objfile->all_comp_units[index];
+  return this->all_comp_units[index];
 }
 
-/* Return the CU given its index.
-   This differs from dw2_get_cutu in that it's for when you know INDEX
-   refers to a CU.  */
+/* See declaration.  */
 
-static struct dwarf2_per_cu_data *
-dw2_get_cu (struct dwarf2_per_objfile *dwarf2_per_objfile, int index)
+signatured_type *
+dwarf2_per_objfile::get_tu (int index)
 {
-  gdb_assert (index >= 0 && index < dwarf2_per_objfile->n_comp_units);
+  gdb_assert (index >= 0 && index < this->n_type_units);
 
-  return dwarf2_per_objfile->all_comp_units[index];
+  return this->all_type_units[index];
 }
 
 /* Return a new dwarf2_per_cu_data allocated on OBJFILE's
@@ -3204,7 +3200,7 @@  create_addrmap_from_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
       lo = gdbarch_adjust_dwarf2_addr (gdbarch, lo + baseaddr);
       hi = gdbarch_adjust_dwarf2_addr (gdbarch, hi + baseaddr);
       addrmap_set_empty (mutable_map, lo, hi - 1,
-			 dw2_get_cutu (dwarf2_per_objfile, cu_index));
+			 dwarf2_per_objfile->get_cu (cu_index));
     }
 
   objfile->psymtabs_addrmap = addrmap_create_fixed (mutable_map,
@@ -3233,7 +3229,7 @@  create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
     debug_info_offset_to_per_cu;
   for (int cui = 0; cui < dwarf2_per_objfile->n_comp_units; ++cui)
     {
-      dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, cui);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (cui);
       const auto insertpair
 	= debug_info_offset_to_per_cu.emplace (per_cu->sect_off, per_cu);
       if (!insertpair.second)
@@ -3766,7 +3762,7 @@  dw2_find_last_source_symtab (struct objfile *objfile)
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   int index = dwarf2_per_objfile->n_comp_units - 1;
-  dwarf2_per_cu_data *dwarf_cu = dw2_get_cutu (dwarf2_per_objfile, index);
+  dwarf2_per_cu_data *dwarf_cu = dwarf2_per_objfile->get_cu (index);
   compunit_symtab *cust = dw2_instantiate_symtab (dwarf_cu);
 
   if (cust == NULL)
@@ -3846,7 +3842,7 @@  dw2_map_symtabs_matching_filename
   for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
     {
       int j;
-      struct dwarf2_per_cu_data *per_cu = dw2_get_cu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
       struct quick_file_names *file_data;
 
       /* We only need to look at symtabs not already expanded.  */
@@ -3974,7 +3970,6 @@  dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
       offset_type cu_index_and_attrs =
 	MAYBE_SWAP (iter->vec[iter->next + 1]);
       offset_type cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs);
-      struct dwarf2_per_cu_data *per_cu;
       int want_static = iter->block_index != GLOBAL_BLOCK;
       /* This value is only valid for index versions >= 7.  */
       int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs);
@@ -3999,7 +3994,7 @@  dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
 	  continue;
 	}
 
-      per_cu = dw2_get_cutu (dwarf2_per_objfile, cu_index);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (cu_index);
 
       /* Skip if already read in.  */
       if (per_cu->v.quick->compunit_symtab)
@@ -4103,7 +4098,7 @@  dw2_print_stats (struct objfile *objfile)
 
   for (int i = 0; i < total; ++i)
     {
-      struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
 
       if (!per_cu->v.quick->compunit_symtab)
 	++count;
@@ -4172,8 +4167,7 @@  dw2_expand_all_symtabs (struct objfile *objfile)
 
   for (int i = 0; i < total_units; ++i)
     {
-      struct dwarf2_per_cu_data *per_cu
-	= dw2_get_cutu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
 
       dw2_instantiate_symtab (per_cu);
     }
@@ -4194,7 +4188,7 @@  dw2_expand_symtabs_with_fullname (struct objfile *objfile,
   for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
     {
       int j;
-      struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
       struct quick_file_names *file_data;
 
       /* We only need to look at symtabs not already expanded.  */
@@ -5064,7 +5058,6 @@  dw2_expand_marked_cus
   vec_len = MAYBE_SWAP (vec[0]);
   for (vec_idx = 0; vec_idx < vec_len; ++vec_idx)
     {
-      struct dwarf2_per_cu_data *per_cu;
       offset_type cu_index_and_attrs = MAYBE_SWAP (vec[vec_idx + 1]);
       /* This value is only valid for index versions >= 7.  */
       int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs);
@@ -5121,7 +5114,7 @@  dw2_expand_marked_cus
 	  continue;
 	}
 
-      per_cu = dw2_get_cutu (dwarf2_per_objfile, cu_index);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (cu_index);
       dw2_expand_symtabs_matching_one (per_cu, file_matcher,
 				       expansion_notify);
     }
@@ -5154,7 +5147,7 @@  dw_expand_symtabs_matching_file_matcher
   for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
     {
       int j;
-      struct dwarf2_per_cu_data *per_cu = dw2_get_cu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
       struct quick_file_names *file_data;
       void **slot;
 
@@ -5318,7 +5311,7 @@  dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
 
       for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
 	{
-	  dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, i);
+	  dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
 
 	  if (per_cu->v.quick->compunit_symtab)
 	    {
@@ -5332,7 +5325,7 @@  dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
 
       for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
 	{
-	  dwarf2_per_cu_data *per_cu = dw2_get_cu (dwarf2_per_objfile, i);
+	  dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
 	  struct quick_file_names *file_data;
 	  void **slot;
 
@@ -5955,7 +5948,7 @@  dw2_debug_names_iterator::next ()
 			 objfile_name (dwarf2_per_objfile->objfile));
 	      continue;
 	    }
-	  per_cu = dw2_get_cutu (dwarf2_per_objfile, ull);
+	  per_cu = dwarf2_per_objfile->get_cutu (ull);
 	  break;
 	case DW_IDX_type_unit:
 	  /* Don't crash on bad data.  */
@@ -5968,8 +5961,7 @@  dw2_debug_names_iterator::next ()
 			 objfile_name (dwarf2_per_objfile->objfile));
 	      continue;
 	    }
-	  per_cu = dw2_get_cutu (dwarf2_per_objfile,
-				 dwarf2_per_objfile->n_comp_units + ull);
+	  per_cu = &dwarf2_per_objfile->get_tu (ull)->per_cu;
 	  break;
 	case DW_IDX_GNU_internal:
 	  if (!m_map.augmentation_is_gdb)
@@ -6248,7 +6240,7 @@  dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
       for (i = 0; i < (dwarf2_per_objfile->n_comp_units
 		       + dwarf2_per_objfile->n_type_units); ++i)
 	{
-	  dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, i);
+	  dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
 
 	  per_cu->v.quick = OBSTACK_ZALLOC (&objfile->objfile_obstack,
 					    struct dwarf2_per_cu_quick_data);
@@ -8437,7 +8429,7 @@  set_partial_user (struct dwarf2_per_objfile *dwarf2_per_objfile)
 
   for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
     {
-      struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
       struct partial_symtab *pst = per_cu->v.psymtab;
       int j;
 
@@ -8490,7 +8482,7 @@  dwarf2_build_psymtabs_hard (struct dwarf2_per_objfile *dwarf2_per_objfile)
 
   for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
     {
-      struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (dwarf2_per_objfile, i);
+      dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cu (i);
 
       process_psymtab_comp_unit (per_cu, 0, language_minimal);
     }
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 46a10520ec2a..24b5ff47e7eb 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -89,6 +89,7 @@  struct tu_stats
 struct dwarf2_debug_sections;
 struct mapped_index;
 struct mapped_debug_names;
+struct signatured_type;
 
 /* Collection of data recorded per objfile.
    This hangs off of dwarf2_objfile_data_key.  */
@@ -105,6 +106,30 @@  struct dwarf2_per_objfile : public allocate_on_obstack
 
   DISABLE_COPY_AND_ASSIGN (dwarf2_per_objfile);
 
+  /* Return the CU/TU given its index.
+
+     This is intended for loops like:
+
+     for (i = 0; i < (dwarf2_per_objfile->n_comp_units
+		      + dwarf2_per_objfile->n_type_units); ++i)
+       {
+         dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
+
+         ...;
+       }
+  */
+  dwarf2_per_cu_data *get_cutu (int index);
+
+  /* Return the CU given its index.
+     This differs from get_cutu in that it's for when you know INDEX refers to a
+     CU.  */
+  dwarf2_per_cu_data *get_cu (int index);
+
+  /* Return the TU given its index.
+     This differs from get_cutu in that it's for when you know INDEX refers to a
+     TU.  */
+  signatured_type *get_tu (int index);
+
   /* Free all cached compilation units.  */
   void free_cached_comp_units ();
 private: