[3/3] gdb: pass signature to allocate_signatured_type and signatured_type constructor

Message ID 20210531143525.3981077-3-simon.marchi@polymtl.ca
State New
Headers show
Series
  • [1/3] gdb: move dwarf2_per_cu_data and signatured_type up
Related show

Commit Message

Philippe Waroquiers via Gdb-patches May 31, 2021, 2:35 p.m.
All signatured_type constucted (even those used only for lookups in hash
maps) need a signature.  Enforce that by passing the signature all the
way to the signatured_type constructor.

gdb/ChangeLog:

	* dwarf2/read.h (struct structured_type) <signatured_type>: New.
	Update all callers.
	(struct dwarf2_per_bfd) <allocate_signatured_type>: Add
	signature parameter, update all callers.
	* dwar2/read.c (dwarf2_per_bfd::allocate_signatured_type): Add
	signature parameter.

Change-Id: I99bc1f88f54127666aa133ddbbabb7f7668fa14a
---
 gdb/dwarf2/read.c | 45 +++++++++++++++++++--------------------------
 gdb/dwarf2/read.h |  8 ++++++--
 2 files changed, 25 insertions(+), 28 deletions(-)

-- 
2.31.1

Comments

Tom Tromey May 31, 2021, 3:30 p.m. | #1
Simon> All signatured_type constucted (even those used only for lookups in hash

Typo, "constructed".

Simon>  signatured_type_up
Simon> -dwarf2_per_bfd::allocate_signatured_type ()
Simon> +dwarf2_per_bfd::allocate_signatured_type (ULONGEST signature)
Simon>  {
Simon> -  signatured_type_up result (new signatured_type);
Simon> +  signatured_type_up result (new signatured_type (signature));
Simon>    result->per_bfd = this;
Simon>    result->index = all_comp_units.size ();
Simon>    result->is_debug_types = true;

This looks good, though I wonder why not go further and pass the other
things via the constructor as well.

The other patches also look good.  I think it would be fine to commit,
or extend this one and commit, either way.

Tom
Philippe Waroquiers via Gdb-patches May 31, 2021, 4:33 p.m. | #2
On 2021-05-31 11:30 a.m., Tom Tromey wrote:
> Simon> All signatured_type constucted (even those used only for lookups in hash

> 

> Typo, "constructed".

> 

> Simon>  signatured_type_up

> Simon> -dwarf2_per_bfd::allocate_signatured_type ()

> Simon> +dwarf2_per_bfd::allocate_signatured_type (ULONGEST signature)

> Simon>  {

> Simon> -  signatured_type_up result (new signatured_type);

> Simon> +  signatured_type_up result (new signatured_type (signature));

> Simon>    result->per_bfd = this;

> Simon>    result->index = all_comp_units.size ();

> Simon>    result->is_debug_types = true;

> 

> This looks good, though I wonder why not go further and pass the other

> things via the constructor as well.


I started by doing that, trying to pass more things, but it slowly grew
a bit too big and some things weren't as obvious, so I came back to just
the original goal.

> 

> The other patches also look good.  I think it would be fine to commit,

> or extend this one and commit, either way.


Thanks, I'll push them as-is, and we can build on top of that.

Simon

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 24247defebb8..338003590dce 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2295,9 +2295,9 @@  dwarf2_per_bfd::allocate_per_cu ()
 /* See read.h.  */
 
 signatured_type_up
-dwarf2_per_bfd::allocate_signatured_type ()
+dwarf2_per_bfd::allocate_signatured_type (ULONGEST signature)
 {
-  signatured_type_up result (new signatured_type);
+  signatured_type_up result (new signatured_type (signature));
   result->per_bfd = this;
   result->index = all_comp_units.size ();
   result->is_debug_types = true;
@@ -2396,8 +2396,7 @@  create_signatured_type_table_from_index
       signature = extract_unsigned_integer (bytes + 16, 8, BFD_ENDIAN_LITTLE);
       bytes += 3 * 8;
 
-      sig_type = per_bfd->allocate_signatured_type ();
-      sig_type->signature = signature;
+      sig_type = per_bfd->allocate_signatured_type (signature);
       sig_type->type_offset_in_tu = type_offset_in_tu;
       sig_type->section = section;
       sig_type->sect_off = sect_off;
@@ -2447,8 +2446,8 @@  create_signatured_type_table_from_debug_names
 				     section->buffer + to_underlying (sect_off),
 				     rcuh_kind::TYPE);
 
-      sig_type = per_objfile->per_bfd->allocate_signatured_type ();
-      sig_type->signature = cu_header.signature;
+      sig_type = per_objfile->per_bfd->allocate_signatured_type
+	(cu_header.signature);
       sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
       sig_type->section = section;
       sig_type->sect_off = sect_off;
@@ -5887,14 +5886,13 @@  add_type_unit (dwarf2_per_objfile *per_objfile, ULONGEST sig, void **slot)
     ++per_objfile->per_bfd->tu_stats.nr_all_type_units_reallocs;
 
   signatured_type_up sig_type_holder
-    = per_objfile->per_bfd->allocate_signatured_type ();
+    = per_objfile->per_bfd->allocate_signatured_type (sig);
   signatured_type *sig_type = sig_type_holder.get ();
 
   per_objfile->resize_symtabs ();
 
   per_objfile->per_bfd->all_comp_units.emplace_back
     (sig_type_holder.release ());
-  sig_type->signature = sig;
   if (per_objfile->per_bfd->using_index)
     {
       sig_type->v.quick =
@@ -5965,7 +5963,6 @@  lookup_dwo_signatured_type (struct dwarf2_cu *cu, ULONGEST sig)
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
   struct dwo_file *dwo_file;
   struct dwo_unit find_dwo_entry, *dwo_entry;
-  struct signatured_type find_sig_entry, *sig_entry;
   void **slot;
 
   gdb_assert (cu->dwo_unit && per_objfile->per_bfd->using_index);
@@ -5981,10 +5978,10 @@  lookup_dwo_signatured_type (struct dwarf2_cu *cu, ULONGEST sig)
      the TU has an entry in .gdb_index, replace the recorded data from
      .gdb_index with this TU.  */
 
-  find_sig_entry.signature = sig;
+  signatured_type find_sig_entry (sig);
   slot = htab_find_slot (per_objfile->per_bfd->signatured_types.get (),
 			 &find_sig_entry, INSERT);
-  sig_entry = (struct signatured_type *) *slot;
+  signatured_type *sig_entry = (struct signatured_type *) *slot;
 
   /* We can get here with the TU already read, *or* in the process of being
      read.  Don't reassign the global entry to point to this DWO if that's
@@ -6031,7 +6028,6 @@  lookup_dwp_signatured_type (struct dwarf2_cu *cu, ULONGEST sig)
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
   struct dwp_file *dwp_file = get_dwp_file (per_objfile);
   struct dwo_unit *dwo_entry;
-  struct signatured_type find_sig_entry, *sig_entry;
   void **slot;
 
   gdb_assert (cu->dwo_unit && per_objfile->per_bfd->using_index);
@@ -6042,10 +6038,10 @@  lookup_dwp_signatured_type (struct dwarf2_cu *cu, ULONGEST sig)
   if (per_objfile->per_bfd->signatured_types == NULL)
     per_objfile->per_bfd->signatured_types = allocate_signatured_type_table ();
 
-  find_sig_entry.signature = sig;
+  signatured_type find_sig_entry (sig);
   slot = htab_find_slot (per_objfile->per_bfd->signatured_types.get (),
 			 &find_sig_entry, INSERT);
-  sig_entry = (struct signatured_type *) *slot;
+  signatured_type *sig_entry = (struct signatured_type *) *slot;
 
   /* Have we already tried to read this TU?
      Note: sig_entry can be NULL if the skeleton TU was removed (thus it
@@ -6086,15 +6082,12 @@  lookup_signatured_type (struct dwarf2_cu *cu, ULONGEST sig)
     }
   else
     {
-      struct signatured_type find_entry, *entry;
-
       if (per_objfile->per_bfd->signatured_types == NULL)
 	return NULL;
-      find_entry.signature = sig;
-      entry = ((struct signatured_type *)
-	       htab_find (per_objfile->per_bfd->signatured_types.get (),
-			  &find_entry));
-      return entry;
+      signatured_type find_entry (sig);
+      return ((struct signatured_type *)
+	      htab_find (per_objfile->per_bfd->signatured_types.get (),
+			 &find_entry));
     }
 }
 
@@ -7271,14 +7264,13 @@  process_skeletonless_type_unit (void **slot, void *info)
 {
   struct dwo_unit *dwo_unit = (struct dwo_unit *) *slot;
   dwarf2_per_objfile *per_objfile = (dwarf2_per_objfile *) info;
-  struct signatured_type find_entry, *entry;
 
   /* If this TU doesn't exist in the global table, add it and read it in.  */
 
   if (per_objfile->per_bfd->signatured_types == NULL)
     per_objfile->per_bfd->signatured_types = allocate_signatured_type_table ();
 
-  find_entry.signature = dwo_unit->signature;
+  signatured_type find_entry (dwo_unit->signature);
   slot = htab_find_slot (per_objfile->per_bfd->signatured_types.get (),
 			 &find_entry, INSERT);
   /* If we've already seen this type there's nothing to do.  What's happening
@@ -7288,7 +7280,8 @@  process_skeletonless_type_unit (void **slot, void *info)
 
   /* This does the job that create_all_comp_units would have done for
      this TU.  */
-  entry = add_type_unit (per_objfile, dwo_unit->signature, slot);
+  signatured_type *entry
+    = add_type_unit (per_objfile, dwo_unit->signature, slot);
   fill_in_sig_entry_from_dwo_entry (per_objfile, entry, dwo_unit);
   *slot = entry;
 
@@ -7482,9 +7475,9 @@  read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
 	  if (types_htab == nullptr)
 	    types_htab = allocate_signatured_type_table ();
 
-	  auto sig_type = per_objfile->per_bfd->allocate_signatured_type ();
+	  auto sig_type = per_objfile->per_bfd->allocate_signatured_type
+	    (cu_header.signature);
 	  signatured_type *sig_ptr = sig_type.get ();
-	  sig_type->signature = cu_header.signature;
 	  sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
 	  this_cu.reset (sig_type.release ());
 
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 98937915d27e..6f45eea1268b 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -281,8 +281,12 @@  struct dwarf2_per_cu_data
 
 struct signatured_type : public dwarf2_per_cu_data
 {
+  signatured_type (ULONGEST signature)
+    : signature (signature)
+  {}
+
   /* The type's signature.  */
-  ULONGEST signature = 0;
+  ULONGEST signature;
 
   /* Offset in the TU of the type's DIE, as read from the TU header.
      If this TU is a DWO stub and the definition lives in a DWO file
@@ -341,7 +345,7 @@  struct dwarf2_per_bfd
   /* A convenience function to allocate a signatured_type.  The
      returned object has its "index" field set properly.  The object
      is allocated on the dwarf2_per_bfd obstack.  */
-  signatured_type_up allocate_signatured_type ();
+  signatured_type_up allocate_signatured_type (ULONGEST signature);
 
 private:
   /* This function is mapped across the sections and remembers the