Patchwork [RFA,4/6] Change dwarf2_cu::method_info to be a std::vector

login
register
mail settings
Submitter Tom Tromey
Date Jan. 6, 2018, 12:26 a.m.
Message ID <20180106002621.21099-5-tom@tromey.com>
Download mbox | patch
Permalink /patch/25240/
State New
Headers show

Comments

Tom Tromey - Jan. 6, 2018, 12:26 a.m.
This changes the type of dwarf2_cu::method_info and fixes up the uses.
In order to remove cleanups from process_full_comp_unit and
process_full_type_unit, psymtab_include_file_name also had to be
changed to avoid leaving dangling cleanups.

2018-01-05  Tom Tromey  <tom@tromey.com>

	* dwarf2read.c (delayed_method_info): Remove typedef.
	(dwarf2_cu::method_info): Now a std::vector.
	(add_to_method_list): Update.
	(free_delayed_list): Remove.
	(compute_delayed_physnames): Update.
	(process_full_comp_unit, process_full_type_unit): Clear the method
	list.  Remove cleanups.
	(psymtab_include_file_name): Add name_holder parameter.  Use
	unique_xmalloc_ptr.
	(dwarf_decode_lines): Update.
---
 gdb/ChangeLog    | 13 ++++++++
 gdb/dwarf2read.c | 93 ++++++++++++++++++++++----------------------------------
 2 files changed, 49 insertions(+), 57 deletions(-)
Simon Marchi - Jan. 16, 2018, 3:29 p.m.
On 2018-01-05 07:26 PM, Tom Tromey wrote:
> This changes the type of dwarf2_cu::method_info and fixes up the uses.
> In order to remove cleanups from process_full_comp_unit and
> process_full_type_unit, psymtab_include_file_name also had to be
> changed to avoid leaving dangling cleanups.

That seems ok, but I don't completely understand what's happening.

Can you clarify how process_full_comp_unit/process_full_type_unit are
related to psymtab_include_file_name?  I don't see how the later gets
called by the formers.

Simon
Tom Tromey - Jan. 17, 2018, 5:15 p.m.
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> On 2018-01-05 07:26 PM, Tom Tromey wrote:
>> This changes the type of dwarf2_cu::method_info and fixes up the uses.
>> In order to remove cleanups from process_full_comp_unit and
>> process_full_type_unit, psymtab_include_file_name also had to be
>> changed to avoid leaving dangling cleanups.

Simon> That seems ok, but I don't completely understand what's happening.

Simon> Can you clarify how process_full_comp_unit/process_full_type_unit are
Simon> related to psymtab_include_file_name?  I don't see how the later gets
Simon> called by the formers.

The call chain looks like this:

process_full_comp_unit
-> process_die
-> read_file_scope
-> handle_DW_AT_stmt_list
-> dwarf_decode_lines
-> psymtab_include_file_name

What happened formerly is that process_full_comp_unit installed an outer
cleanup, and then psymtab_include_file_name took advantage of that by
leaving some dangling cleanups.

Tom
Simon Marchi - Jan. 17, 2018, 5:29 p.m.
On 2018-01-17 12:15, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> On 2018-01-05 07:26 PM, Tom Tromey wrote:
>>> This changes the type of dwarf2_cu::method_info and fixes up the 
>>> uses.
>>> In order to remove cleanups from process_full_comp_unit and
>>> process_full_type_unit, psymtab_include_file_name also had to be
>>> changed to avoid leaving dangling cleanups.
> 
> Simon> That seems ok, but I don't completely understand what's 
> happening.
> 
> Simon> Can you clarify how 
> process_full_comp_unit/process_full_type_unit are
> Simon> related to psymtab_include_file_name?  I don't see how the later 
> gets
> Simon> called by the formers.
> 
> The call chain looks like this:
> 
> process_full_comp_unit
> -> process_die
> -> read_file_scope
> -> handle_DW_AT_stmt_list
> -> dwarf_decode_lines
> -> psymtab_include_file_name
> 
> What happened formerly is that process_full_comp_unit installed an 
> outer
> cleanup, and then psymtab_include_file_name took advantage of that by
> leaving some dangling cleanups.

Ok, thanks for the clarification.  The patch LGTM.

Simon

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 04000eacae..0c878d9aba 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@ 
 2018-01-05  Tom Tromey  <tom@tromey.com>
 
+	* dwarf2read.c (delayed_method_info): Remove typedef.
+	(dwarf2_cu::method_info): Now a std::vector.
+	(add_to_method_list): Update.
+	(free_delayed_list): Remove.
+	(compute_delayed_physnames): Update.
+	(process_full_comp_unit, process_full_type_unit): Clear the method
+	list.  Remove cleanups.
+	(psymtab_include_file_name): Add name_holder parameter.  Use
+	unique_xmalloc_ptr.
+	(dwarf_decode_lines): Update.
+
+2018-01-05  Tom Tromey  <tom@tromey.com>
+
 	* dwarf2read.c (struct dwarf2_cu): Add constructor, destructor.
 	<free_abbrev_table>: Declare method.
 	(~auto_free_abbrev_table): Update.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 400080b208..998c8479ea 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -633,9 +633,6 @@  struct delayed_method_info
   struct die_info *die;
 };
 
-typedef struct delayed_method_info delayed_method_info;
-DEF_VEC_O (delayed_method_info);
-
 /* Internal state when decoding a particular compilation unit.  */
 struct dwarf2_cu
 {
@@ -724,7 +721,7 @@  struct dwarf2_cu
 
   /* A list of methods which need to have physnames computed
      after all type information has been read.  */
-  VEC (delayed_method_info) *method_list = nullptr;
+  std::vector<delayed_method_info> method_list;
 
   /* To be copied to symtab->call_site_htab.  */
   htab_t call_site_htab = nullptr;
@@ -10051,20 +10048,7 @@  add_to_method_list (struct type *type, int fnfield_index, int index,
   mi.index = index;
   mi.name = name;
   mi.die = die;
-  VEC_safe_push (delayed_method_info, cu->method_list, &mi);
-}
-
-/* A cleanup for freeing the delayed method list.  */
-
-static void
-free_delayed_list (void *ptr)
-{
-  struct dwarf2_cu *cu = (struct dwarf2_cu *) ptr;
-  if (cu->method_list != NULL)
-    {
-      VEC_free (delayed_method_info, cu->method_list);
-      cu->method_list = NULL;
-    }
+  cu->method_list.push_back (mi);
 }
 
 /* Check whether [PHYSNAME, PHYSNAME+LEN) ends with a modifier like
@@ -10093,21 +10077,18 @@  check_modifier (const char *physname, size_t &len, const char (&mod)[N])
 static void
 compute_delayed_physnames (struct dwarf2_cu *cu)
 {
-  int i;
-  struct delayed_method_info *mi;
-
   /* Only C++ delays computing physnames.  */
-  if (VEC_empty (delayed_method_info, cu->method_list))
+  if (cu->method_list.empty ())
     return;
   gdb_assert (cu->language == language_cplus);
 
-  for (i = 0; VEC_iterate (delayed_method_info, cu->method_list, i, mi) ; ++i)
+  for (struct delayed_method_info &mi : cu->method_list)
     {
       const char *physname;
       struct fn_fieldlist *fn_flp
-	= &TYPE_FN_FIELDLIST (mi->type, mi->fnfield_index);
-      physname = dwarf2_physname (mi->name, mi->die, cu);
-      TYPE_FN_FIELD_PHYSNAME (fn_flp->fn_fields, mi->index)
+	= &TYPE_FN_FIELDLIST (mi.type, mi.fnfield_index);
+      physname = dwarf2_physname (mi.name, mi.die, cu);
+      TYPE_FN_FIELD_PHYSNAME (fn_flp->fn_fields, mi.index)
 	= physname ? physname : "";
 
       /* Since there's no tag to indicate whether a method is a
@@ -10122,14 +10103,17 @@  compute_delayed_physnames (struct dwarf2_cu *cu)
 	      if (physname[len] == ')') /* shortcut */
 		break;
 	      else if (check_modifier (physname, len, " const"))
-		TYPE_FN_FIELD_CONST (fn_flp->fn_fields, mi->index) = 1;
+		TYPE_FN_FIELD_CONST (fn_flp->fn_fields, mi.index) = 1;
 	      else if (check_modifier (physname, len, " volatile"))
-		TYPE_FN_FIELD_VOLATILE (fn_flp->fn_fields, mi->index) = 1;
+		TYPE_FN_FIELD_VOLATILE (fn_flp->fn_fields, mi.index) = 1;
 	      else
 		break;
 	    }
 	}
     }
+
+  /* The list is no longer needed.  */
+  cu->method_list.clear ();
 }
 
 /* Go objects should be embedded in a DW_TAG_module DIE,
@@ -10364,7 +10348,6 @@  process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   CORE_ADDR lowpc, highpc;
   struct compunit_symtab *cust;
-  struct cleanup *delayed_list_cleanup;
   CORE_ADDR baseaddr;
   struct block *static_block;
   CORE_ADDR addr;
@@ -10373,7 +10356,9 @@  process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
 
   buildsym_init ();
   scoped_free_pendings free_pending;
-  delayed_list_cleanup = make_cleanup (free_delayed_list, cu);
+
+  /* Clear the list here in case something was left over.  */
+  cu->method_list.clear ();
 
   cu->list_in_scope = &file_symbols;
 
@@ -10391,7 +10376,6 @@  process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
      should be complete, and it should now be safe to compute all of the
      physnames.  */
   compute_delayed_physnames (cu);
-  do_cleanups (delayed_list_cleanup);
 
   /* Some compilers don't define a DW_AT_high_pc attribute for the
      compilation unit.  If the DW_AT_high_pc is missing, synthesize
@@ -10466,7 +10450,6 @@  process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
   struct dwarf2_cu *cu = per_cu->cu;
   struct objfile *objfile = per_cu->objfile;
   struct compunit_symtab *cust;
-  struct cleanup *delayed_list_cleanup;
   struct signatured_type *sig_type;
 
   gdb_assert (per_cu->is_debug_types);
@@ -10474,7 +10457,9 @@  process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
 
   buildsym_init ();
   scoped_free_pendings free_pending;
-  delayed_list_cleanup = make_cleanup (free_delayed_list, cu);
+
+  /* Clear the list here in case something was left over.  */
+  cu->method_list.clear ();
 
   cu->list_in_scope = &file_symbols;
 
@@ -10492,7 +10477,6 @@  process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
      should be complete, and it should now be safe to compute all of the
      physnames.  */
   compute_delayed_physnames (cu);
-  do_cleanups (delayed_list_cleanup);
 
   /* TUs share symbol tables.
      If this is the first TU to use this symtab, complete the construction
@@ -20219,25 +20203,24 @@  dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
    Return the file name of the psymtab for included file FILE_INDEX
    in line header LH of PST.
    COMP_DIR is the compilation directory (DW_AT_comp_dir) or NULL if unknown.
-   If space for the result is malloc'd, it will be freed by a cleanup.
-   Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.
-
-   The function creates dangling cleanup registration.  */
+   If space for the result is malloc'd, *NAME_HOLDER will be set.
+   Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
 
 static const char *
 psymtab_include_file_name (const struct line_header *lh, int file_index,
 			   const struct partial_symtab *pst,
-			   const char *comp_dir)
+			   const char *comp_dir,
+			   gdb::unique_xmalloc_ptr<char> *name_holder)
 {
   const file_entry &fe = lh->file_names[file_index];
   const char *include_name = fe.name;
   const char *include_name_to_compare = include_name;
   const char *pst_filename;
-  char *copied_name = NULL;
   int file_is_pst;
 
   const char *dir_name = fe.include_dir (lh);
 
+  gdb::unique_xmalloc_ptr<char> hold_compare;
   if (!IS_ABSOLUTE_PATH (include_name)
       && (dir_name != NULL || comp_dir != NULL))
     {
@@ -20264,36 +20247,30 @@  psymtab_include_file_name (const struct line_header *lh, int file_index,
 
       if (dir_name != NULL)
 	{
-	  char *tem = concat (dir_name, SLASH_STRING,
-			      include_name, (char *)NULL);
-
-	  make_cleanup (xfree, tem);
-	  include_name = tem;
+	  name_holder->reset (concat (dir_name, SLASH_STRING,
+				      include_name, (char *) NULL));
+	  include_name = name_holder->get ();
 	  include_name_to_compare = include_name;
 	}
       if (!IS_ABSOLUTE_PATH (include_name) && comp_dir != NULL)
 	{
-	  char *tem = concat (comp_dir, SLASH_STRING,
-			      include_name, (char *)NULL);
-
-	  make_cleanup (xfree, tem);
-	  include_name_to_compare = tem;
+	  hold_compare.reset (concat (comp_dir, SLASH_STRING,
+				      include_name, (char *) NULL));
+	  include_name_to_compare = hold_compare.get ();
 	}
     }
 
   pst_filename = pst->filename;
+  gdb::unique_xmalloc_ptr<char> copied_name;
   if (!IS_ABSOLUTE_PATH (pst_filename) && pst->dirname != NULL)
     {
-      copied_name = concat (pst->dirname, SLASH_STRING,
-			    pst_filename, (char *)NULL);
-      pst_filename = copied_name;
+      copied_name.reset (concat (pst->dirname, SLASH_STRING,
+				 pst_filename, (char *) NULL));
+      pst_filename = copied_name.get ();
     }
 
   file_is_pst = FILENAME_CMP (include_name_to_compare, pst_filename) == 0;
 
-  if (copied_name != NULL)
-    xfree (copied_name);
-
   if (file_is_pst)
     return NULL;
   return include_name;
@@ -20951,8 +20928,10 @@  dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
       for (file_index = 0; file_index < lh->file_names.size (); file_index++)
         if (lh->file_names[file_index].included_p == 1)
           {
+	    gdb::unique_xmalloc_ptr<char> name_holder;
 	    const char *include_name =
-	      psymtab_include_file_name (lh, file_index, pst, comp_dir);
+	      psymtab_include_file_name (lh, file_index, pst, comp_dir,
+					 &name_holder);
 	    if (include_name != NULL)
               dwarf2_create_include_psymtab (include_name, pst, objfile);
           }