This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Remove make_cleanup_free_section_addr_info


This removes make_cleanup_free_section_addr_info, instead introducing
a new section_addr_info_up type and changing all the uses.  While
doing this, I noticed that addrs_section_sort could be changed to
return a std::vector, allowing the removal of even more cleanups.

Regression tested by the buildbot.

gdb/ChangeLog
2018-03-13  Tom Tromey  <tom@tromey.com>

	* utils.h (make_cleanup_free_section_addr_info): Don't declare.
	* utils.c (do_free_section_addr_info)
	(make_cleanup_free_section_addr_info): Remove.
	* symfile.h (struct section_addr_info_deleter): New.
	(section_addr_info_up): New typedef.
	(build_section_addr_info_from_objfile, alloc_section_addr_info)
	(build_section_addr_info_from_section_table): Return
	section_addr_info_up.
	(free_section_addr_info): Don't declare.
	* symfile.c (alloc_section_addr_info)
	(build_section_addr_info_from_section_table)
	(build_section_addr_info_from_bfd)
	(build_section_addr_info_from_objfile): Return a
	section_addr_info_up.
	(section_addr_info_deleter::operator ()): Rename from
	free_section_addr_info.
	(addrs_section_compar): Change type; now usable by std::sort.
	(addrs_section_sort): Return a std::vector.  Don't add a final
	NULL.
	(addr_info_make_relative, syms_from_objfile_1)
	(symbol_file_add_separate, add_symbol_file_command): Update.
	* symfile-mem.c (symbol_file_add_from_memory): Update.
	* solib.c (solib_read_symbols): Update.
	* objfiles.c (objfile_relocate): Update.
	* jit.c (jit_bfd_try_read_symtab): Update.
---
 gdb/ChangeLog     |  28 ++++++++++++
 gdb/jit.c         |  10 ++---
 gdb/objfiles.c    |  13 ++----
 gdb/solib.c       |  10 ++---
 gdb/symfile-mem.c |   8 +---
 gdb/symfile.c     | 126 ++++++++++++++++++++++--------------------------------
 gdb/symfile.h     |  27 +++++++-----
 gdb/utils.c       |  12 ------
 gdb/utils.h       |   4 --
 9 files changed, 111 insertions(+), 127 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 62d6634541..9a46db3917 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -888,10 +888,8 @@ jit_bfd_try_read_symtab (struct jit_code_entry *code_entry,
                          CORE_ADDR entry_addr,
                          struct gdbarch *gdbarch)
 {
-  struct section_addr_info *sai;
   struct bfd_section *sec;
   struct objfile *objfile;
-  struct cleanup *old_cleanups;
   int i;
   const struct bfd_arch_info *b;
 
@@ -931,8 +929,8 @@ JITed symbol file is not an object file, ignoring it.\n"));
   /* Read the section address information out of the symbol file.  Since the
      file is generated by the JIT at runtime, it should all of the absolute
      addresses that we care about.  */
-  sai = alloc_section_addr_info (bfd_count_sections (nbfd.get ()));
-  old_cleanups = make_cleanup_free_section_addr_info (sai);
+  section_addr_info_up sai
+    = alloc_section_addr_info (bfd_count_sections (nbfd.get ()));
   i = 0;
   for (sec = nbfd->sections; sec != NULL; sec = sec->next)
     if ((bfd_get_section_flags (nbfd.get (), sec) & (SEC_ALLOC|SEC_LOAD)) != 0)
@@ -948,10 +946,10 @@ JITed symbol file is not an object file, ignoring it.\n"));
 
   /* This call does not take ownership of SAI.  */
   objfile = symbol_file_add_from_bfd (nbfd.get (),
-				      bfd_get_filename (nbfd.get ()), 0, sai,
+				      bfd_get_filename (nbfd.get ()), 0,
+				      sai.get (),
 				      OBJF_SHARED | OBJF_NOT_FILENAME, NULL);
 
-  do_cleanups (old_cleanups);
   add_objfile_entry (objfile, entry_addr);
 }
 
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index a9aaf89540..22b20fa12a 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -906,16 +906,13 @@ objfile_relocate (struct objfile *objfile,
        debug_objfile;
        debug_objfile = objfile_separate_debug_iterate (objfile, debug_objfile))
     {
-      struct section_addr_info *objfile_addrs;
-      struct cleanup *my_cleanups;
-
-      objfile_addrs = build_section_addr_info_from_objfile (objfile);
-      my_cleanups = make_cleanup (xfree, objfile_addrs);
+      section_addr_info_up objfile_addrs
+	= build_section_addr_info_from_objfile (objfile);
 
       /* Here OBJFILE_ADDRS contain the correct absolute addresses, the
 	 relative ones must be already created according to debug_objfile.  */
 
-      addr_info_make_relative (objfile_addrs, debug_objfile->obfd);
+      addr_info_make_relative (objfile_addrs.get (), debug_objfile->obfd);
 
       gdb_assert (debug_objfile->num_sections
 		  == gdb_bfd_count_sections (debug_objfile->obfd));
@@ -923,11 +920,9 @@ objfile_relocate (struct objfile *objfile,
 	new_debug_offsets (SIZEOF_N_SECTION_OFFSETS (debug_objfile->num_sections));
       relative_addr_info_to_section_offsets (new_debug_offsets.data (),
 					     debug_objfile->num_sections,
-					     objfile_addrs);
+					     objfile_addrs.get ());
 
       changed |= objfile_relocate1 (debug_objfile, new_debug_offsets.data ());
-
-      do_cleanups (my_cleanups);
     }
 
   /* Relocate breakpoints as necessary, after things are relocated.  */
diff --git a/gdb/solib.c b/gdb/solib.c
index 1c78845938..ccbc8609b0 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -687,13 +687,13 @@ solib_read_symbols (struct so_list *so, symfile_add_flags flags)
 	    }
 	  if (so->objfile == NULL)
 	    {
-	      sap = build_section_addr_info_from_section_table (so->sections,
-								so->sections_end);
+	      section_addr_info_up sap
+		= build_section_addr_info_from_section_table (so->sections,
+							      so->sections_end);
 	      so->objfile = symbol_file_add_from_bfd (so->abfd, so->so_name,
-						      flags, sap, OBJF_SHARED,
-						      NULL);
+						      flags, sap.get (),
+						      OBJF_SHARED, NULL);
 	      so->objfile->addr_low = so->addr_low;
-	      free_section_addr_info (sap);
 	    }
 
 	  so->symbols_loaded = 1;
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 8d91c729a5..90eacbe61c 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -88,9 +88,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
   struct bfd *nbfd;
   struct bfd_section *sec;
   bfd_vma loadbase;
-  struct section_addr_info *sai;
   unsigned int i;
-  struct cleanup *cleanup;
   symfile_add_flags add_flags = 0;
 
   if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
@@ -114,8 +112,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
     error (_("Got object file from memory but can't read symbols: %s."),
 	   bfd_errmsg (bfd_get_error ()));
 
-  sai = alloc_section_addr_info (bfd_count_sections (nbfd));
-  cleanup = make_cleanup (xfree, sai);
+  section_addr_info_up sai = alloc_section_addr_info (bfd_count_sections (nbfd));
   i = 0;
   for (sec = nbfd->sections; sec != NULL; sec = sec->next)
     if ((bfd_get_section_flags (nbfd, sec) & (SEC_ALLOC|SEC_LOAD)) != 0)
@@ -131,14 +128,13 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
     add_flags |= SYMFILE_VERBOSE;
 
   objf = symbol_file_add_from_bfd (nbfd, bfd_get_filename (nbfd),
-				   add_flags, sai, OBJF_SHARED, NULL);
+				   add_flags, sai.get (), OBJF_SHARED, NULL);
 
   add_target_sections_of_objfile (objf);
 
   /* This might change our ideas about frames already looked at.  */
   reinit_frame_cache ();
 
-  do_cleanups (cleanup);
   return objf;
 }
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 58747d545b..0a514b34fc 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -64,6 +64,7 @@
 #include <sys/stat.h>
 #include <ctype.h>
 #include <chrono>
+#include <algorithm>
 
 #include "psymtab.h"
 
@@ -219,7 +220,7 @@ find_lowest_section (bfd *abfd, asection *sect, void *obj)
    new object's 'num_sections' field is set to 0; it must be updated
    by the caller.  */
 
-struct section_addr_info *
+section_addr_info_up
 alloc_section_addr_info (size_t num_sections)
 {
   struct section_addr_info *sap;
@@ -230,21 +231,20 @@ alloc_section_addr_info (size_t num_sections)
   sap = (struct section_addr_info *) xmalloc (size);
   memset (sap, 0, size);
 
-  return sap;
+  return section_addr_info_up (sap);
 }
 
 /* Build (allocate and populate) a section_addr_info struct from
    an existing section table.  */
 
-extern struct section_addr_info *
+section_addr_info_up
 build_section_addr_info_from_section_table (const struct target_section *start,
                                             const struct target_section *end)
 {
-  struct section_addr_info *sap;
   const struct target_section *stp;
   int oidx;
 
-  sap = alloc_section_addr_info (end - start);
+  section_addr_info_up sap = alloc_section_addr_info (end - start);
 
   for (stp = start, oidx = 0; stp != end; stp++)
     {
@@ -268,14 +268,14 @@ build_section_addr_info_from_section_table (const struct target_section *start,
 
 /* Create a section_addr_info from section offsets in ABFD.  */
 
-static struct section_addr_info *
+static section_addr_info_up
 build_section_addr_info_from_bfd (bfd *abfd)
 {
-  struct section_addr_info *sap;
   int i;
   struct bfd_section *sec;
 
-  sap = alloc_section_addr_info (bfd_count_sections (abfd));
+  section_addr_info_up sap
+    = alloc_section_addr_info (bfd_count_sections (abfd));
   for (i = 0, sec = abfd->sections; sec != NULL; sec = sec->next)
     if (bfd_get_section_flags (abfd, sec) & (SEC_ALLOC | SEC_LOAD))
       {
@@ -292,16 +292,15 @@ build_section_addr_info_from_bfd (bfd *abfd)
 
 /* Create a section_addr_info from section offsets in OBJFILE.  */
 
-struct section_addr_info *
+section_addr_info_up
 build_section_addr_info_from_objfile (const struct objfile *objfile)
 {
-  struct section_addr_info *sap;
   int i;
 
   /* Before reread_symbols gets rewritten it is not safe to call:
      gdb_assert (objfile->num_sections == bfd_count_sections (objfile->obfd));
      */
-  sap = build_section_addr_info_from_bfd (objfile->obfd);
+  section_addr_info_up sap = build_section_addr_info_from_bfd (objfile->obfd);
   for (i = 0; i < sap->num_sections; i++)
     {
       int sectindex = sap->other[i].sectindex;
@@ -313,8 +312,8 @@ build_section_addr_info_from_objfile (const struct objfile *objfile)
 
 /* Free all memory allocated by build_section_addr_info_from_section_table.  */
 
-extern void
-free_section_addr_info (struct section_addr_info *sap)
+void
+section_addr_info_deleter::operator () (struct section_addr_info *sap)
 {
   int idx;
 
@@ -502,39 +501,35 @@ addr_section_name (const char *s)
   return s;
 }
 
-/* qsort comparator for addrs_section_sort.  Sort entries in ascending order by
-   their (name, sectindex) pair.  sectindex makes the sort by name stable.  */
+/* std::sort comparator for addrs_section_sort.  Sort entries in
+   ascending order by their (name, sectindex) pair.  sectindex makes
+   the sort by name stable.  */
 
-static int
-addrs_section_compar (const void *ap, const void *bp)
+static bool
+addrs_section_compar (const struct other_sections *a,
+		      const struct other_sections *b)
 {
-  const struct other_sections *a = *((struct other_sections **) ap);
-  const struct other_sections *b = *((struct other_sections **) bp);
   int retval;
 
   retval = strcmp (addr_section_name (a->name), addr_section_name (b->name));
-  if (retval)
-    return retval;
+  if (retval != 0)
+    return retval < 0;
 
-  return a->sectindex - b->sectindex;
+  return a->sectindex < b->sectindex;
 }
 
-/* Provide sorted array of pointers to sections of ADDRS.  The array is
-   terminated by NULL.  Caller is responsible to call xfree for it.  */
+/* Provide sorted array of pointers to sections of ADDRS.  */
 
-static struct other_sections **
+static std::vector<struct other_sections *>
 addrs_section_sort (struct section_addr_info *addrs)
 {
-  struct other_sections **array;
   int i;
 
-  /* `+ 1' for the NULL terminator.  */
-  array = XNEWVEC (struct other_sections *, addrs->num_sections + 1);
+  std::vector<struct other_sections *> array (addrs->num_sections);
   for (i = 0; i < addrs->num_sections; i++)
     array[i] = &addrs->other[i];
-  array[i] = NULL;
 
-  qsort (array, i, sizeof (*array), addrs_section_compar);
+  std::sort (array.begin (), array.end (), addrs_section_compar);
 
   return array;
 }
@@ -549,10 +544,6 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
   asection *lower_sect;
   CORE_ADDR lower_offset;
   int i;
-  struct cleanup *my_cleanup;
-  struct section_addr_info *abfd_addrs;
-  struct other_sections **addrs_sorted, **abfd_addrs_sorted;
-  struct other_sections **addrs_to_abfd_addrs;
 
   /* Find lowest loadable section to be used as starting point for
      continguous sections.  */
@@ -577,45 +568,44 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
      Use stable sort by name for the sections in both files.  Then linearly
      scan both lists matching as most of the entries as possible.  */
 
-  addrs_sorted = addrs_section_sort (addrs);
-  my_cleanup = make_cleanup (xfree, addrs_sorted);
+  std::vector<struct other_sections *> addrs_sorted
+    = addrs_section_sort (addrs);
 
-  abfd_addrs = build_section_addr_info_from_bfd (abfd);
-  make_cleanup_free_section_addr_info (abfd_addrs);
-  abfd_addrs_sorted = addrs_section_sort (abfd_addrs);
-  make_cleanup (xfree, abfd_addrs_sorted);
+  section_addr_info_up abfd_addrs = build_section_addr_info_from_bfd (abfd);
+  std::vector<struct other_sections *> abfd_addrs_sorted
+    = addrs_section_sort (abfd_addrs.get ());
 
   /* Now create ADDRS_TO_ABFD_ADDRS from ADDRS_SORTED and
      ABFD_ADDRS_SORTED.  */
 
-  addrs_to_abfd_addrs = XCNEWVEC (struct other_sections *, addrs->num_sections);
-  make_cleanup (xfree, addrs_to_abfd_addrs);
+  std::vector<struct other_sections *>
+    addrs_to_abfd_addrs (addrs->num_sections, nullptr);
 
-  while (*addrs_sorted)
+  std::vector<struct other_sections *>::iterator abfd_sorted_iter
+    = abfd_addrs_sorted.begin ();
+  for (struct other_sections *sect : addrs_sorted)
     {
-      const char *sect_name = addr_section_name ((*addrs_sorted)->name);
+      const char *sect_name = addr_section_name (sect->name);
 
-      while (*abfd_addrs_sorted
-	     && strcmp (addr_section_name ((*abfd_addrs_sorted)->name),
+      while (abfd_sorted_iter != abfd_addrs_sorted.end ()
+	     && strcmp (addr_section_name ((*abfd_sorted_iter)->name),
 			sect_name) < 0)
-	abfd_addrs_sorted++;
+	abfd_sorted_iter++;
 
-      if (*abfd_addrs_sorted
-	  && strcmp (addr_section_name ((*abfd_addrs_sorted)->name),
+      if (abfd_sorted_iter != abfd_addrs_sorted.end ()
+	  && strcmp (addr_section_name ((*abfd_sorted_iter)->name),
 		     sect_name) == 0)
 	{
 	  int index_in_addrs;
 
 	  /* Make the found item directly addressable from ADDRS.  */
-	  index_in_addrs = *addrs_sorted - addrs->other;
+	  index_in_addrs = sect - addrs->other;
 	  gdb_assert (addrs_to_abfd_addrs[index_in_addrs] == NULL);
-	  addrs_to_abfd_addrs[index_in_addrs] = *abfd_addrs_sorted;
+	  addrs_to_abfd_addrs[index_in_addrs] = *abfd_sorted_iter;
 
 	  /* Never use the same ABFD entry twice.  */
-	  abfd_addrs_sorted++;
+	  abfd_sorted_iter++;
 	}
-
-      addrs_sorted++;
     }
 
   /* Calculate offsets for the loadable sections.
@@ -681,8 +671,6 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
 	  addrs->other[i].sectindex = -1;
 	}
     }
-
-  do_cleanups (my_cleanup);
 }
 
 /* Parse the user's idea of an offset for dynamic linking, into our idea
@@ -971,7 +959,7 @@ syms_from_objfile_1 (struct objfile *objfile,
 		     struct section_addr_info *addrs,
 		     symfile_add_flags add_flags)
 {
-  struct section_addr_info *local_addr = NULL;
+  section_addr_info_up local_addr;
   struct cleanup *old_chain;
   const int mainline = add_flags & SYMFILE_MAINLINE;
 
@@ -1003,8 +991,7 @@ syms_from_objfile_1 (struct objfile *objfile,
   if (! addrs)
     {
       local_addr = alloc_section_addr_info (1);
-      make_cleanup (xfree, local_addr);
-      addrs = local_addr;
+      addrs = local_addr.get ();
     }
 
   if (mainline)
@@ -1053,7 +1040,6 @@ syms_from_objfile_1 (struct objfile *objfile,
 
   objfile_holder.release ();
   discard_cleanups (old_chain);
-  xfree (local_addr);
 }
 
 /* Same as syms_from_objfile_1, but also initializes the objfile
@@ -1231,22 +1217,16 @@ symbol_file_add_separate (bfd *bfd, const char *name,
 			  symfile_add_flags symfile_flags,
 			  struct objfile *objfile)
 {
-  struct section_addr_info *sap;
-  struct cleanup *my_cleanup;
-
   /* Create section_addr_info.  We can't directly use offsets from OBJFILE
      because sections of BFD may not match sections of OBJFILE and because
      vma may have been modified by tools such as prelink.  */
-  sap = build_section_addr_info_from_objfile (objfile);
-  my_cleanup = make_cleanup_free_section_addr_info (sap);
+  section_addr_info_up sap = build_section_addr_info_from_objfile (objfile);
 
   symbol_file_add_with_addrs
-    (bfd, name, symfile_flags, sap,
+    (bfd, name, symfile_flags, sap.get (),
      objfile->flags & (OBJF_REORDERED | OBJF_SHARED | OBJF_READNOW
 		       | OBJF_USERLOADED),
      objfile);
-
-  do_cleanups (my_cleanup);
 }
 
 /* Process the symbol file ABFD, as either the main file or as a
@@ -2172,10 +2152,8 @@ add_symbol_file_command (const char *args, int from_tty)
     const char *value;
   };
 
-  struct section_addr_info *section_addrs;
   std::vector<sect_opt> sect_opts = { { ".text", NULL } };
   bool stop_processing_options = false;
-  struct cleanup *my_cleanups = make_cleanup (null_cleanup, NULL);
 
   dont_repeat ();
 
@@ -2247,8 +2225,8 @@ add_symbol_file_command (const char *args, int from_tty)
 
   printf_unfiltered (_("add symbol table from file \"%s\" at\n"),
 		     filename.get ());
-  section_addrs = alloc_section_addr_info (sect_opts.size ());
-  make_cleanup (xfree, section_addrs);
+  section_addr_info_up section_addrs
+    = alloc_section_addr_info (sect_opts.size ());
   for (sect_opt &sect : sect_opts)
     {
       CORE_ADDR addr;
@@ -2276,14 +2254,14 @@ add_symbol_file_command (const char *args, int from_tty)
   if (from_tty && (!query ("%s", "")))
     error (_("Not confirmed."));
 
-  objf = symbol_file_add (filename.get (), add_flags, section_addrs, flags);
+  objf = symbol_file_add (filename.get (), add_flags, section_addrs.get (),
+			  flags);
 
   add_target_sections_of_objfile (objf);
 
   /* Getting new symbols may change our opinion about what is
      frameless.  */
   reinit_frame_cache ();
-  do_cleanups (my_cleanups);
 }
 
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 8cd47d8811..0e03f3115e 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -369,7 +369,19 @@ struct sym_fns
   const struct quick_symbol_functions *qf;
 };
 
-extern struct section_addr_info *
+/* A deleter that frees a struct section_addr_info.  */
+
+struct section_addr_info_deleter
+{
+  void operator() (struct section_addr_info *handle);
+};
+
+/* A unique pointer that holds a section_addr_info.  */
+
+typedef std::unique_ptr<struct section_addr_info> section_addr_info_up;
+
+
+extern section_addr_info_up
   build_section_addr_info_from_objfile (const struct objfile *objfile);
 
 extern void relative_addr_info_to_section_offsets
@@ -430,24 +442,17 @@ extern std::string find_separate_debug_file_by_debuglink (struct objfile *);
 
 /* Create a new section_addr_info, with room for NUM_SECTIONS.  */
 
-extern struct section_addr_info *alloc_section_addr_info (size_t
-							  num_sections);
+extern section_addr_info_up alloc_section_addr_info (size_t num_sections);
 
 /* Build (allocate and populate) a section_addr_info struct from an
    existing section table.  */
 
-extern struct section_addr_info
-  *build_section_addr_info_from_section_table (const struct target_section
+extern section_addr_info_up
+   build_section_addr_info_from_section_table (const struct target_section
 					       *start,
 					       const struct target_section
 					       *end);
 
-/* Free all memory allocated by
-   build_section_addr_info_from_section_table.  */
-
-extern void free_section_addr_info (struct section_addr_info *);
-
-
 			/*   Variables   */
 
 /* If non-zero, shared library symbols will be added automatically
diff --git a/gdb/utils.c b/gdb/utils.c
index b99d444a6e..3886efd840 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -141,18 +141,6 @@ show_pagination_enabled (struct ui_file *file, int from_tty,
    because while they use the "cleanup API" they are not part of the
    "cleanup API".  */
 
-static void
-do_free_section_addr_info (void *arg)
-{
-  free_section_addr_info ((struct section_addr_info *) arg);
-}
-
-struct cleanup *
-make_cleanup_free_section_addr_info (struct section_addr_info *addrs)
-{
-  return make_cleanup (do_free_section_addr_info, addrs);
-}
-
 /* Helper for make_cleanup_unpush_target.  */
 
 static void
diff --git a/gdb/utils.h b/gdb/utils.h
index 8ca3eb0369..6ff18568fe 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -242,10 +242,6 @@ private:
 
 /* Cleanup utilities.  */
 
-struct section_addr_info;
-extern struct cleanup *make_cleanup_free_section_addr_info
-                       (struct section_addr_info *);
-
 /* For make_cleanup_close see common/filestuff.h.  */
 
 struct target_ops;
-- 
2.13.6


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]