Simplify DWARF symtab inclusion handling

Message ID 20240119201334.536259-1-tom@tromey.com
State New
Headers
Series Simplify DWARF symtab inclusion handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

Tom Tromey Jan. 19, 2024, 8:13 p.m. UTC
  In the past, dwarf2_per_cu_data was allocated using malloc, so special
handling was needed for the vector used for symtab handling.  We
changed this to use 'new' a while back, so this code can now be
greatly simplified.

Regression tested on x86-64 Fedora 38.
---
 gdb/dwarf2/read.c | 34 +++++++++++++---------------------
 gdb/dwarf2/read.h | 46 +++-------------------------------------------
 2 files changed, 16 insertions(+), 64 deletions(-)
  

Comments

Simon Marchi Jan. 19, 2024, 8:40 p.m. UTC | #1
On 1/19/24 15:13, Tom Tromey wrote:
> In the past, dwarf2_per_cu_data was allocated using malloc, so special
> handling was needed for the vector used for symtab handling.  We
> changed this to use 'new' a while back, so this code can now be
> greatly simplified.

LGTM, but I noted two formatting fixes you could do while at it.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

> @@ -20702,7 +20694,7 @@ follow_die_sig_1 (struct die_info *src_die, struct signatured_type *sig_type,
>        if (per_objfile->per_bfd->index_table != NULL
>  	  && !per_objfile->per_bfd->index_table->version_check ())
>  	{
> -	  (*ref_cu)->per_cu->imported_symtabs_push (sig_cu->per_cu);
> +	  (*ref_cu)->per_cu->imported_symtabs.push_back (sig_cu->per_cu);
>  	}

You could remove the unnecessary braces here.

> @@ -244,45 +241,8 @@ struct dwarf2_per_cu_data
>       .gdb_index version <=7 this also records the TUs that the CU referred
>       to.  Concurrently with this change gdb was modified to emit version 8
>       indices so we only pay a price for gold generated indices.
> -     http://sourceware.org/bugzilla/show_bug.cgi?id=15021.
> -
> -     This currently needs to be a public member due to how
> -     dwarf2_per_cu_data is allocated and used.  Ideally in future things
> -     could be refactored to make this private.  Until then please try to
> -     avoid direct access to this member, and instead use the helper
> -     functions above.  */
> -  std::vector <dwarf2_per_cu_data *> *imported_symtabs = nullptr;
> -
> -  /* Return true of IMPORTED_SYMTABS is empty or not yet allocated.  */
> -  bool imported_symtabs_empty () const
> -  {
> -    return (imported_symtabs == nullptr || imported_symtabs->empty ());
> -  }
> -
> -  /* Push P to the back of IMPORTED_SYMTABS, allocated IMPORTED_SYMTABS
> -     first if required.  */
> -  void imported_symtabs_push (dwarf2_per_cu_data *p)
> -  {
> -    if (imported_symtabs == nullptr)
> -      imported_symtabs = new std::vector <dwarf2_per_cu_data *>;
> -    imported_symtabs->push_back (p);
> -  }
> -
> -  /* Return the size of IMPORTED_SYMTABS if it is allocated, otherwise
> -     return 0.  */
> -  size_t imported_symtabs_size () const
> -  {
> -    if (imported_symtabs == nullptr)
> -      return 0;
> -    return imported_symtabs->size ();
> -  }
> -
> -  /* Delete IMPORTED_SYMTABS and set the pointer back to nullptr.  */
> -  void imported_symtabs_free ()
> -  {
> -    delete imported_symtabs;
> -    imported_symtabs = nullptr;
> -  }
> +     http://sourceware.org/bugzilla/show_bug.cgi?id=15021.  */
> +  std::vector <dwarf2_per_cu_data *> imported_symtabs;

You could remove the space after `vector`.

Simon
  
Tom Tromey Jan. 19, 2024, 11:40 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 1/19/24 15:13, Tom Tromey wrote:
>> In the past, dwarf2_per_cu_data was allocated using malloc, so special
>> handling was needed for the vector used for symtab handling.  We
>> changed this to use 'new' a while back, so this code can now be
>> greatly simplified.

Simon> LGTM, but I noted two formatting fixes you could do while at it.

I made those changes & I'm checking it in.

Tom
  

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7691fe0050b..5230358c4a7 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1280,10 +1280,7 @@  dwarf2_per_bfd::~dwarf2_per_bfd ()
     index_table->wait_completely ();
 
   for (auto &per_cu : all_units)
-    {
-      per_cu->imported_symtabs_free ();
-      per_cu->free_cached_file_names ();
-    }
+    per_cu->free_cached_file_names ();
 
   /* Everything else should be on this->obstack.  */
 }
@@ -6103,13 +6100,10 @@  recursively_compute_inclusions (std::vector<compunit_symtab *> *result,
 	}
     }
 
-  if (!per_cu->imported_symtabs_empty ())
-    for (dwarf2_per_cu_data *ptr : *per_cu->imported_symtabs)
-      {
-	recursively_compute_inclusions (result, all_children,
-					all_type_symtabs, ptr, per_objfile,
-					cust);
-      }
+  for (dwarf2_per_cu_data *ptr : per_cu->imported_symtabs)
+    recursively_compute_inclusions (result, all_children,
+				    all_type_symtabs, ptr, per_objfile,
+				    cust);
 }
 
 /* Compute the compunit_symtab 'includes' fields for the compunit_symtab of
@@ -6121,7 +6115,7 @@  compute_compunit_symtab_includes (dwarf2_per_cu_data *per_cu,
 {
   gdb_assert (! per_cu->is_debug_types);
 
-  if (!per_cu->imported_symtabs_empty ())
+  if (!per_cu->imported_symtabs.empty ())
     {
       int len;
       std::vector<compunit_symtab *> result_symtabs;
@@ -6138,12 +6132,10 @@  compute_compunit_symtab_includes (dwarf2_per_cu_data *per_cu,
 						   htab_eq_pointer,
 						   NULL, xcalloc, xfree));
 
-      for (dwarf2_per_cu_data *ptr : *per_cu->imported_symtabs)
-	{
-	  recursively_compute_inclusions (&result_symtabs, all_children.get (),
-					  all_type_symtabs.get (), ptr,
-					  per_objfile, cust);
-	}
+      for (dwarf2_per_cu_data *ptr : per_cu->imported_symtabs)
+	recursively_compute_inclusions (&result_symtabs, all_children.get (),
+					all_type_symtabs.get (), ptr,
+					per_objfile, cust);
 
       /* Now we have a transitive closure of all the included symtabs.  */
       len = result_symtabs.size ();
@@ -6391,7 +6383,7 @@  process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu)
 	load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu),
 			     false, cu->lang ());
 
-      cu->per_cu->imported_symtabs_push (per_cu);
+      cu->per_cu->imported_symtabs.push_back (per_cu);
     }
 }
 
@@ -9651,7 +9643,7 @@  queue_and_load_dwo_tu (void **slot, void *info)
       if (maybe_queue_comp_unit (NULL, sig_type, cu->per_objfile,
 				 cu->lang ()))
 	load_full_type_unit (sig_type, cu->per_objfile);
-      cu->per_cu->imported_symtabs_push (sig_type);
+      cu->per_cu->imported_symtabs.push_back (sig_type);
     }
 
   return 1;
@@ -20702,7 +20694,7 @@  follow_die_sig_1 (struct die_info *src_die, struct signatured_type *sig_type,
       if (per_objfile->per_bfd->index_table != NULL
 	  && !per_objfile->per_bfd->index_table->version_check ())
 	{
-	  (*ref_cu)->per_cu->imported_symtabs_push (sig_cu->per_cu);
+	  (*ref_cu)->per_cu->imported_symtabs.push_back (sig_cu->per_cu);
 	}
 
       *ref_cu = sig_cu;
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 66c6fe3fe87..ea60218e494 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -227,10 +227,7 @@  struct dwarf2_per_cu_data
      NOTE: This points into dwarf2_per_objfile->per_bfd->quick_file_names_table.  */
   struct quick_file_names *file_names = nullptr;
 
-  /* The CUs we import using DW_TAG_imported_unit.  This is filled in
-     while reading psymtabs, used to compute the psymtab dependencies,
-     and then cleared.  Then it is filled in again while reading full
-     symbols, and only deleted when the objfile is destroyed.
+  /* The CUs we import using DW_TAG_imported_unit.
 
      This is also used to work around a difference between the way gold
      generates .gdb_index version <=7 and the way gdb does.  Arguably this
@@ -244,45 +241,8 @@  struct dwarf2_per_cu_data
      .gdb_index version <=7 this also records the TUs that the CU referred
      to.  Concurrently with this change gdb was modified to emit version 8
      indices so we only pay a price for gold generated indices.
-     http://sourceware.org/bugzilla/show_bug.cgi?id=15021.
-
-     This currently needs to be a public member due to how
-     dwarf2_per_cu_data is allocated and used.  Ideally in future things
-     could be refactored to make this private.  Until then please try to
-     avoid direct access to this member, and instead use the helper
-     functions above.  */
-  std::vector <dwarf2_per_cu_data *> *imported_symtabs = nullptr;
-
-  /* Return true of IMPORTED_SYMTABS is empty or not yet allocated.  */
-  bool imported_symtabs_empty () const
-  {
-    return (imported_symtabs == nullptr || imported_symtabs->empty ());
-  }
-
-  /* Push P to the back of IMPORTED_SYMTABS, allocated IMPORTED_SYMTABS
-     first if required.  */
-  void imported_symtabs_push (dwarf2_per_cu_data *p)
-  {
-    if (imported_symtabs == nullptr)
-      imported_symtabs = new std::vector <dwarf2_per_cu_data *>;
-    imported_symtabs->push_back (p);
-  }
-
-  /* Return the size of IMPORTED_SYMTABS if it is allocated, otherwise
-     return 0.  */
-  size_t imported_symtabs_size () const
-  {
-    if (imported_symtabs == nullptr)
-      return 0;
-    return imported_symtabs->size ();
-  }
-
-  /* Delete IMPORTED_SYMTABS and set the pointer back to nullptr.  */
-  void imported_symtabs_free ()
-  {
-    delete imported_symtabs;
-    imported_symtabs = nullptr;
-  }
+     http://sourceware.org/bugzilla/show_bug.cgi?id=15021.  */
+  std::vector <dwarf2_per_cu_data *> imported_symtabs;
 
   /* Get the header of this per_cu, reading it if necessary.  */
   const comp_unit_head *get_header () const;