[PATCH][gdb/symtab] Add all_comp_units_cu/tu

[PATCH][gdb/symtab] Add all_comp_units_cu/tu

[Tom de Vries]

Hi,

Add all_comp_units_cu and all_comp_units_tu arrayviews on all_comp_units,
which allows us to:
- easily get the number of CUs or TUs in all_comp_units, and
- easily access the nth CU or TU.

This minimizes the use of tu_stats.nr_tus.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/symtab] Add all_comp_units_cu/tu

---
 gdb/dwarf2/index-write.c |  5 ++---
 gdb/dwarf2/read.c        | 33 +++++++++++++++++++++++++--------
 gdb/dwarf2/read.h        |  5 +++++
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index b6d8dddafd3..baaed474c39 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1286,9 +1286,8 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
     }
 
    /* Verify that all units are represented.  */
-  gdb_assert (counter == (per_objfile->per_bfd->all_comp_units.size ()
-			  - per_objfile->per_bfd->tu_stats.nr_tus));
-  gdb_assert (types_counter == per_objfile->per_bfd->tu_stats.nr_tus);
+  gdb_assert (counter == per_objfile->per_bfd->all_comp_units_cu.size ());
+  gdb_assert (types_counter == per_objfile->per_bfd->all_comp_units_tu.size ());
 
   for (const cooked_index_entry *entry : table->all_entries ())
     nametable.insert (entry);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 29ccd00a1da..94c369188c1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2631,6 +2631,8 @@ to use the section anyway."),
   return 1;
 }
 
+static void finalize_all_comp_units (dwarf2_per_bfd *per_bfd);
+
 /* Callback types for dwarf2_read_gdb_index.  */
 
 typedef gdb::function_view
@@ -2722,6 +2724,8 @@ dwarf2_read_gdb_index
 					       types_list_elements);
     }
 
+  finalize_all_comp_units (per_bfd);
+
   create_addrmap_from_index (per_objfile, map.get ());
 
   per_bfd->index_table = std::move (map);
@@ -4754,6 +4758,8 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 	(per_objfile, *map, section, &per_bfd->abbrev);
     }
 
+  finalize_all_comp_units (per_bfd);
+
   create_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges);
 
   per_bfd->index_table = std::move (map);
@@ -5017,9 +5023,7 @@ dw2_debug_names_iterator::next ()
 	case DW_IDX_compile_unit:
 	  {
 	    /* Don't crash on bad data.  */
-	    int nr_cus = (per_bfd->all_comp_units.size ()
-			  - per_bfd->tu_stats.nr_tus);
-	    if (ull >= nr_cus)
+	    if (ull >= per_bfd->all_comp_units_cu.size ())
 	      {
 		complaint (_(".debug_names entry has bad CU index %s"
 			     " [in module %s]"),
@@ -5032,7 +5036,7 @@ dw2_debug_names_iterator::next ()
 	  break;
 	case DW_IDX_type_unit:
 	  /* Don't crash on bad data.  */
-	  if (ull >= per_bfd->tu_stats.nr_tus)
+	  if (ull >= per_bfd->all_comp_units_tu.size ())
 	    {
 	      complaint (_(".debug_names entry has bad TU index %s"
 			   " [in module %s]"),
@@ -5041,8 +5045,7 @@ dw2_debug_names_iterator::next ()
 	      continue;
 	    }
 	  {
-	    int nr_cus = (per_bfd->all_comp_units.size ()
-			  - per_bfd->tu_stats.nr_tus);
+	    int nr_cus = per_bfd->all_comp_units_cu.size ();
 	    per_cu = per_bfd->get_cu (nr_cus + ull);
 	  }
 	  break;
@@ -6918,7 +6921,7 @@ build_type_psymtabs (dwarf2_per_objfile *per_objfile,
   /* It's up to the caller to not call us multiple times.  */
   gdb_assert (per_objfile->per_bfd->type_unit_groups == NULL);
 
-  if (per_objfile->per_bfd->tu_stats.nr_tus == 0)
+  if (per_objfile->per_bfd->all_comp_units_tu.size () == 0)
     return;
 
   /* TUs typically share abbrev tables, and there can be way more TUs than
@@ -6945,7 +6948,7 @@ build_type_psymtabs (dwarf2_per_objfile *per_objfile,
   /* Sort in a separate table to maintain the order of all_comp_units
      for .gdb_index: TU indices directly index all_type_units.  */
   std::vector<tu_abbrev_offset> sorted_by_abbrev;
-  sorted_by_abbrev.reserve (per_objfile->per_bfd->tu_stats.nr_tus);
+  sorted_by_abbrev.reserve (per_objfile->per_bfd->all_comp_units_tu.size ());
 
   for (const auto &cu : per_objfile->per_bfd->all_comp_units)
     {
@@ -7259,6 +7262,18 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
     }
 }
 
+/* Initialize the views on all_comp_units.  */
+
+static void
+finalize_all_comp_units (dwarf2_per_bfd *per_bfd)
+{
+  size_t nr_tus = per_bfd->tu_stats.nr_tus;
+  size_t nr_cus = per_bfd->all_comp_units.size () - nr_tus;
+  gdb::array_view<dwarf2_per_cu_data_up> tmp = per_bfd->all_comp_units;
+  per_bfd->all_comp_units_cu = tmp.slice (0, nr_cus);
+  per_bfd->all_comp_units_tu = tmp.slice (nr_cus, nr_tus);
+}
+
 /* Create a list of all compilation units in OBJFILE.
    This is only done for -readnow and building partial symtabs.  */
 
@@ -7290,6 +7305,8 @@ create_all_comp_units (dwarf2_per_objfile *per_objfile)
     }
 
   per_objfile->per_bfd->signatured_types = std::move (types_htab);
+
+  finalize_all_comp_units (per_objfile->per_bfd);
 }
 
 /* Return the initial uleb128 in the die at INFO_PTR.  */
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index c2f86a9d367..d79d2b779db 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -491,6 +491,11 @@ struct dwarf2_per_bfd
      the target compilation unit of a particular reference.  */
   std::vector<dwarf2_per_cu_data_up> all_comp_units;
 
+  /* The all_comp_units vector contains both CUs and TUs.  Provide views on
+     the vector that are limited to either the CU part or the TU part.  */
+  gdb::array_view<dwarf2_per_cu_data_up> all_comp_units_cu;
+  gdb::array_view<dwarf2_per_cu_data_up> all_comp_units_tu;
+
   /* Table of struct type_unit_group objects.
      The hash key is the DW_AT_stmt_list value.  */
   htab_up type_unit_groups;

Re: [PATCH][gdb/symtab] Add all_comp_units_cu/tu

[Simon Marchi]

On 9/16/22 08:42, Tom de Vries via Gdb-patches wrote:
> Hi,
> 
> Add all_comp_units_cu and all_comp_units_tu arrayviews on all_comp_units,
> which allows us to:
> - easily get the number of CUs or TUs in all_comp_units, and
> - easily access the nth CU or TU.

I think it's a good idea, but I would suggest reworking the naming a
bit.  I find it unclear to comp_units_cu and comp_units_tu, given that
"cu" means comp_units.  I would suggest renaming all_comp_units to
all_units (since it contains units of different kinds), and then you can
have all_comp_units and all_type_units for the array views.

Simon

Re: [PATCH][gdb/symtab] Add all_comp_units_cu/tu

[Tom de Vries]

On 9/17/22 11:59, Simon Marchi wrote:
> On 9/16/22 08:42, Tom de Vries via Gdb-patches wrote:
>> Hi,
>>
>> Add all_comp_units_cu and all_comp_units_tu arrayviews on all_comp_units,
>> which allows us to:
>> - easily get the number of CUs or TUs in all_comp_units, and
>> - easily access the nth CU or TU.
> 
> I think it's a good idea, but I would suggest reworking the naming a
> bit.  I find it unclear to comp_units_cu and comp_units_tu, given that
> "cu" means comp_units.  I would suggest renaming all_comp_units to
> all_units (since it contains units of different kinds), and then you can
> have all_comp_units and all_type_units for the array views.

I've done that now.

I initially had two patches that I merged and tested, but I reverted 
back to two patches because I think it'll be easier to review.

Sumitted here:
- https://sourceware.org/pipermail/gdb-patches/2022-September/191908.html
- https://sourceware.org/pipermail/gdb-patches/2022-September/191907.html

Thanks,
- Tom