[RFA,1/2] Allocate dwarf2_per_cu_data and signatured_type with new

Message ID 20180519203701.31211-2-tom@tromey.com
State New
Headers show
Series
  • Replace one use of VEC in dwarf2read.c
Related show

Commit Message

Tom Tromey May 19, 2018, 8:37 p.m.
This changes dwarf2_per_cu_data and signatured_type to be allocated
with new, rather than on the obstack.  The main idea here is to move
destructors into the objects that actually own the data, to make
dwarf2read a bit simpler to understand, and to enable the next patch.

This particular change required one little hack to make "delete" work
properly.  I did not think it would be good to add a virtual
destructor to dwarf2_per_cu_data just for this.

gdb/ChangeLog
2018-05-18  Tom Tromey  <tom@tromey.com>

	* dwarf2read.c (dwarf2_per_cu_data::dwarf2_per_cu_data)
	(dwarf2_per_cu_data::~dwarf2_per_cu_data): Define.
	(~dwarf2_per_objfile): Update.
	(create_cu_from_index_list): Use new.
	(create_signatured_type_table_from_index)
	(create_signatured_type_table_from_debug_names, add_type_unit)
	(read_comp_units_from_section): Use new.
	(create_cus_hash_table): Update.
	* dwarf2read.h (struct dwarf2_per_cu_data): Add constructor,
	destructor, initializers.
	(struct signatured_type): Likewise.
	(struct dwarf2_per_cu_data) <is_sig_type>: New member.
---
 gdb/ChangeLog    | 15 +++++++++++++++
 gdb/dwarf2read.c | 53 +++++++++++++++++++++++++++++++++--------------------
 gdb/dwarf2read.h | 47 +++++++++++++++++++++++++++++++++--------------
 3 files changed, 81 insertions(+), 34 deletions(-)

-- 
2.13.6

Comments

Simon Marchi May 21, 2018, 3:38 a.m. | #1
On 2018-05-19 04:37 PM, Tom Tromey wrote:
> This changes dwarf2_per_cu_data and signatured_type to be allocated

> with new, rather than on the obstack.  The main idea here is to move

> destructors into the objects that actually own the data, to make

> dwarf2read a bit simpler to understand, and to enable the next patch.

> 

> This particular change required one little hack to make "delete" work

> properly.  I did not think it would be good to add a virtual

> destructor to dwarf2_per_cu_data just for this.


Hi Tom,

It would only be useful to make the destructor of dwarf2_per_cu_data virtual
if other classes (such as signatured_type) inherited from it instead of
including it as their first data member.  But I think it would be preferrable
to do that (inheritance + virtual destructor) instead of hacking our way around
like this.

I have a patch in a branch that does that:
https://github.com/simark/binutils-gdb/commit/bbbd2c0b10b6e78965cdd70a157e7f475522009f

Though I would get rid of the allocation on the obstack like you did.  Do you think it
would be a good way to go?  Or does the extra space used by the vtable for each CU
worries you?

In any case, I pushed my obstack_new stuff, because I presumed there would be
some problem here.  This allocation in create_type_unit_group is problematic:

  tu_group = OBSTACK_ZALLOC (&objfile->objfile_obstack,
			     struct type_unit_group);

because type_unit_group includes a dwarf2_per_cu_data, which now needs to be
constructed.  That class would also need to be new'ed.

Thanks,

Simon

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0690785c5e..e53c015932 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2117,6 +2117,22 @@  attr_value_as_address (struct attribute *attr)
   return addr;
 }
 
+dwarf2_per_cu_data::dwarf2_per_cu_data ()
+  : queued (0),
+    load_all_dies (0),
+    is_debug_types (0),
+    is_dwz (0),
+    reading_dwo_directly (0),
+    tu_read (0),
+    is_sig_type (0)
+{
+}
+
+dwarf2_per_cu_data::~dwarf2_per_cu_data ()
+{
+  VEC_free (dwarf2_per_cu_ptr, imported_symtabs);
+}
+
 /* See declaration.  */
 
 dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
@@ -2146,10 +2162,15 @@  dwarf2_per_objfile::~dwarf2_per_objfile ()
     htab_delete (line_header_hash);
 
   for (dwarf2_per_cu_data *per_cu : all_comp_units)
-    VEC_free (dwarf2_per_cu_ptr, per_cu->imported_symtabs);
+    {
+      if (per_cu->is_sig_type)
+	delete (struct signatured_type *) per_cu;
+      else
+	delete per_cu;
+    }
 
   for (signatured_type *sig_type : all_type_units)
-    VEC_free (dwarf2_per_cu_ptr, sig_type->per_cu.imported_symtabs);
+    delete sig_type;
 
   VEC_free (dwarf2_section_info_def, types);
 
@@ -2959,9 +2980,7 @@  create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
                           sect_offset sect_off, ULONGEST length)
 {
   struct objfile *objfile = dwarf2_per_objfile->objfile;
-  dwarf2_per_cu_data *the_cu
-    = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-                     struct dwarf2_per_cu_data);
+  dwarf2_per_cu_data *the_cu = new struct dwarf2_per_cu_data;
   the_cu->sect_off = sect_off;
   the_cu->length = length;
   the_cu->dwarf2_per_objfile = dwarf2_per_objfile;
@@ -3052,10 +3071,10 @@  create_signatured_type_table_from_index
       signature = extract_unsigned_integer (bytes + 16, 8, BFD_ENDIAN_LITTLE);
       bytes += 3 * 8;
 
-      sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-				 struct signatured_type);
+      sig_type = new struct signatured_type;
       sig_type->signature = signature;
       sig_type->type_offset_in_tu = type_offset_in_tu;
+      sig_type->per_cu.is_sig_type = 1;
       sig_type->per_cu.is_debug_types = 1;
       sig_type->per_cu.section = section;
       sig_type->per_cu.sect_off = sect_off;
@@ -3109,10 +3128,10 @@  create_signatured_type_table_from_debug_names
 				     section->buffer + to_underlying (sect_off),
 				     rcuh_kind::TYPE);
 
-      sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-				 struct signatured_type);
+      sig_type = new struct signatured_type;
       sig_type->signature = cu_header.signature;
       sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
+      sig_type->per_cu.is_sig_type = 1;
       sig_type->per_cu.is_debug_types = 1;
       sig_type->per_cu.section = section;
       sig_type->per_cu.sect_off = sect_off;
@@ -6831,11 +6850,11 @@  add_type_unit (struct dwarf2_per_objfile *dwarf2_per_objfile, ULONGEST sig,
       == dwarf2_per_objfile->all_type_units.capacity ())
     ++dwarf2_per_objfile->tu_stats.nr_all_type_units_reallocs;
 
-  signatured_type *sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-					      struct signatured_type);
+  signatured_type *sig_type = new struct signatured_type;
 
   dwarf2_per_objfile->all_type_units.push_back (sig_type);
   sig_type->signature = sig;
+  sig_type->per_cu.is_sig_type = 1;
   sig_type->per_cu.is_debug_types = 1;
   if (dwarf2_per_objfile->using_index)
     {
@@ -8504,18 +8523,13 @@  read_comp_units_from_section (struct dwarf2_per_objfile *dwarf2_per_objfile,
 
       /* Save the compilation unit for later lookup.  */
       if (cu_header.unit_type != DW_UT_type)
-	{
-	  this_cu = XOBNEW (&objfile->objfile_obstack,
-			    struct dwarf2_per_cu_data);
-	  memset (this_cu, 0, sizeof (*this_cu));
-	}
+	this_cu = new struct dwarf2_per_cu_data;
       else
 	{
-	  auto sig_type = XOBNEW (&objfile->objfile_obstack,
-				  struct signatured_type);
-	  memset (sig_type, 0, sizeof (*sig_type));
+	  auto sig_type = new struct signatured_type;
 	  sig_type->signature = cu_header.signature;
 	  sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
+	  sig_type->per_cu.is_sig_type = 1;
 	  this_cu = &sig_type->per_cu;
 	}
       this_cu->is_debug_types = (cu_header.unit_type == DW_UT_type);
@@ -11865,7 +11879,6 @@  create_cus_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
 
       memset (&create_dwo_cu_data.dwo_unit, 0,
 	      sizeof (create_dwo_cu_data.dwo_unit));
-      memset (&per_cu, 0, sizeof (per_cu));
       per_cu.dwarf2_per_objfile = dwarf2_per_objfile;
       per_cu.is_debug_types = 0;
       per_cu.sect_off = sect_offset (info_ptr - section.buffer);
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index fbac7171de..369461f965 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -253,16 +253,21 @@  dwarf2_per_objfile *get_dwarf2_per_objfile (struct objfile *objfile);
 
 struct dwarf2_per_cu_data
 {
+  dwarf2_per_cu_data ();
+  ~dwarf2_per_cu_data ();
+
+  DISABLE_COPY_AND_ASSIGN (dwarf2_per_cu_data);
+
   /* The start offset and length of this compilation unit.
      NOTE: Unlike comp_unit_head.length, this length includes
      initial_length_size.
      If the DIE refers to a DWO file, this is always of the original die,
      not the DWO file.  */
-  sect_offset sect_off;
-  unsigned int length;
+  sect_offset sect_off {};
+  unsigned int length = 0;
 
   /* DWARF standard version this data has been read from (such as 4 or 5).  */
-  short dwarf_version;
+  short dwarf_version = 0;
 
   /* Flag indicating this compilation unit will be read in before
      any of the current compilation units are processed.  */
@@ -298,18 +303,22 @@  struct dwarf2_per_cu_data
      This flag is only valid if is_debug_types is true.  */
   unsigned int tu_read : 1;
 
+  /* Non-zero if this is actually a signatured_type, below.  This
+     allows for safe downcasting.  */
+  unsigned int is_sig_type : 1;
+
   /* The section this CU/TU lives in.
      If the DIE refers to a DWO file, this is always the original die,
      not the DWO file.  */
-  struct dwarf2_section_info *section;
+  struct dwarf2_section_info *section = nullptr;
 
   /* Set to non-NULL iff this CU is currently loaded.  When it gets freed out
      of the CU cache it gets reset to NULL again.  This is left as NULL for
      dummy CUs (a CU header, but nothing else).  */
-  struct dwarf2_cu *cu;
+  struct dwarf2_cu *cu = nullptr;
 
   /* The corresponding dwarf2_per_objfile.  */
-  struct dwarf2_per_objfile *dwarf2_per_objfile;
+  struct dwarf2_per_objfile *dwarf2_per_objfile = nullptr;
 
   /* When dwarf2_per_objfile->using_index is true, the 'quick' field
      is active.  Otherwise, the 'psymtab' field is active.  */
@@ -321,7 +330,7 @@  struct dwarf2_per_cu_data
 
     /* Data needed by the "quick" functions.  */
     struct dwarf2_per_cu_quick_data *quick;
-  } v;
+  } v {};
 
   /* The CUs we import using DW_TAG_imported_unit.  This is filled in
      while reading psymtabs, used to compute the psymtab dependencies,
@@ -341,13 +350,23 @@  struct dwarf2_per_cu_data
      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.  */
-  VEC (dwarf2_per_cu_ptr) *imported_symtabs;
+  VEC (dwarf2_per_cu_ptr) *imported_symtabs = nullptr;
 };
 
 /* Entry in the signatured_types hash table.  */
 
 struct signatured_type
 {
+  signatured_type ()
+  {
+  }
+
+  ~signatured_type ()
+  {
+  }
+
+  DISABLE_COPY_AND_ASSIGN (signatured_type);
+
   /* The "per_cu" object of this type.
      This struct is used iff per_cu.is_debug_types.
      N.B.: This is the first member so that it's easy to convert pointers
@@ -355,32 +374,32 @@  struct signatured_type
   struct dwarf2_per_cu_data per_cu;
 
   /* The type's signature.  */
-  ULONGEST signature;
+  ULONGEST signature = 0;
 
   /* 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
      (specified by DW_AT_GNU_dwo_name), this value is unusable.  */
-  cu_offset type_offset_in_tu;
+  cu_offset type_offset_in_tu {};
 
   /* Offset in the section of the type's DIE.
      If the definition lives in a DWO file, this is the offset in the
      .debug_types.dwo section.
      The value is zero until the actual value is known.
      Zero is otherwise not a valid section offset.  */
-  sect_offset type_offset_in_section;
+  sect_offset type_offset_in_section {};
 
   /* Type units are grouped by their DW_AT_stmt_list entry so that they
      can share them.  This points to the containing symtab.  */
-  struct type_unit_group *type_unit_group;
+  struct type_unit_group *type_unit_group = nullptr;
 
   /* The type.
      The first time we encounter this type we fully read it in and install it
      in the symbol tables.  Subsequent times we only need the type.  */
-  struct type *type;
+  struct type *type = nullptr;
 
   /* Containing DWO unit.
      This field is valid iff per_cu.reading_dwo_directly.  */
-  struct dwo_unit *dwo_unit;
+  struct dwo_unit *dwo_unit = nullptr;
 };
 
 typedef struct signatured_type *sig_type_ptr;