Allocate compunit_symtab on heap

This patch changes compunit_symtab to be allocated on the heap, using
'new'.  It also changes the container that holds these in the objfile.
I chose to use an intrusive_list to store compunit_symtab because I
think pointer stability is needed here.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33435
Approved-By: Simon Marchi <simon.marchi@efficios.com>

gdb/buildsym.c

@@ -57,13 +57,11 @@ buildsym_compunit::buildsym_compunit (struct objfile *objfile_,
   : m_objfile (objfile_),
     m_last_source_file (name == nullptr ? nullptr : xstrdup (name)),
     m_comp_dir (comp_dir_ == nullptr ? "" : comp_dir_),
+    m_owned_compunit_symtab (std::make_unique<compunit_symtab> (m_objfile, name)),
+    m_compunit_symtab (m_owned_compunit_symtab.get ()),
     m_language (language_),
     m_last_source_start_addr (last_addr)
 {
-  /* Allocate the compunit symtab now.  The caller needs it to allocate
-     non-primary symtabs.  It is also needed by get_macro_table.  */
-  m_compunit_symtab = allocate_compunit_symtab (m_objfile, name);
-
   /* Build the subfile for NAME (the main source file) so that we can record
      a pointer to it for later.
      IMPORTANT: Do not allocate a struct symtab for NAME here.
@@ -982,7 +980,7 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
       }
   }
 
-  add_compunit_symtab_to_objfile (cu);
+  add_compunit_symtab_to_objfile (std::move (m_owned_compunit_symtab));
 
   return cu;
 }

gdb/buildsym.h

@@ -384,8 +384,11 @@ private:
      the same lifetime as objfile.  */
   const char *m_debugformat = nullptr;
 
-  /* The compunit we are building.  */
-  struct compunit_symtab *m_compunit_symtab = nullptr;
+  /* The compunit we are building.  If the symtab is owned by this
+     object, both fields are set.  For a re-opened symtab, only
+     m_compunit_symtab is set.  */
+  std::unique_ptr<compunit_symtab> m_owned_compunit_symtab;
+  compunit_symtab *m_compunit_symtab;
 
   /* Language of this compunit_symtab.  */
   enum language m_language;

gdb/jit.c

@@ -517,7 +517,6 @@ jit_symtab_close_impl (struct gdb_symbol_callbacks *cb,
 static void
 finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile)
 {
-  struct compunit_symtab *cust;
   size_t blockvector_size;
   CORE_ADDR begin, end;
   struct blockvector *bv;
@@ -533,9 +532,11 @@ finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile)
       return a.end > b.end;
     });
 
-  cust = allocate_compunit_symtab (objfile, stab->file_name.c_str ());
+  auto cusymtab = std::make_unique<compunit_symtab> (objfile,
+						     stab->file_name.c_str ());
+  compunit_symtab *cust
+    = add_compunit_symtab_to_objfile (std::move (cusymtab));
   symtab *filetab = allocate_symtab (cust, stab->file_name.c_str ());
-  add_compunit_symtab_to_objfile (cust);
 
   /* JIT compilers compile in memory.  */
   cust->set_dirname (nullptr);

gdb/mdebugread.c

@@ -4623,12 +4623,13 @@ sort_blocks (struct symtab *s)
 static struct compunit_symtab *
 new_symtab (const char *name, int maxlines, struct objfile *objfile)
 {
-  struct compunit_symtab *cust = allocate_compunit_symtab (objfile, name);
+  auto cusymtab = std::make_unique<compunit_symtab> (objfile, name);
   struct symtab *symtab;
   struct blockvector *bv;
   enum language lang;
 
-  add_compunit_symtab_to_objfile (cust);
+  struct compunit_symtab *cust
+    = add_compunit_symtab_to_objfile (std::move (cusymtab));
   symtab = allocate_symtab (cust, name);
 
   symtab->set_linetable (new_linetable (maxlines));

gdb/objfiles.c

@@ -472,8 +472,6 @@ objfile::~objfile ()
   /* It still may reference data modules have associated with the objfile and
      the symbol file data.  */
   forget_cached_source_info ();
-  for (compunit_symtab *cu : compunits ())
-    cu->finalize ();
 
   breakpoint_free_objfile (this);
   btrace_free_objfile (this);
@@ -675,7 +673,7 @@ objfile_rebase (struct objfile *objfile, CORE_ADDR slide)
 bool
 objfile::has_full_symbols ()
 {
-  return this->compunit_symtabs != nullptr;
+  return !this->compunit_symtabs.empty ();
 }
 
 /* See objfiles.h.  */

gdb/objfiles.h

@@ -33,6 +33,8 @@
 #include "quick-symbol.h"
 #include <forward_list>
 #include "gdbsupport/unordered_map.h"
+#include "gdbsupport/owning_intrusive_list.h"
+#include "gdbsupport/reference-to-pointer-iterator.h"
 
 struct htab;
 struct objfile_data;
@@ -471,14 +473,18 @@ public:
   /* Return the program space associated with this objfile.  */
   program_space *pspace () { return m_pspace; }
 
+  using compunit_symtab_iterator
+    = reference_to_pointer_iterator<owning_intrusive_list<compunit_symtab>::iterator>;
+  using compunit_symtab_range = iterator_range<compunit_symtab_iterator>;
+
   /* A range adapter that makes it possible to iterate over all
      compunits in one objfile.  */
 
   compunit_symtab_range compunits ()
   {
-    next_iterator<compunit_symtab> begin (compunit_symtabs);
-
-    return compunit_symtab_range (std::move (begin));
+    auto begin = compunit_symtab_iterator (compunit_symtabs.begin ());
+    auto end = compunit_symtab_iterator (compunit_symtabs.end ());
+    return compunit_symtab_range (std::move (begin), std::move (end));
   }
 
   /* A range adapter that makes it possible to iterate over all
@@ -717,7 +723,7 @@ public:
   /* List of compunits.
      These are used to do symbol lookups and file/line-number lookups.  */
 
-  struct compunit_symtab *compunit_symtabs = nullptr;
+  owning_intrusive_list<compunit_symtab> compunit_symtabs;
 
   /* The object file's BFD.  Can be null if the objfile contains only
      minimal symbols (e.g. the run time common symbols for SunOS4) or

gdb/symfile.c

@@ -2590,7 +2590,7 @@ reread_symbols (int from_tty)
 	  objfile.sect_index_data = -1;
 	  objfile.sect_index_rodata = -1;
 	  objfile.sect_index_text = -1;
-	  objfile.compunit_symtabs = NULL;
+	  objfile.compunit_symtabs.clear ();
 	  objfile.template_symbols = NULL;
 	  objfile.static_links.clear ();
 
@@ -2820,8 +2820,7 @@ deduce_language_from_filename (const char *filename)
   return language_unknown;
 }
 
-/* Allocate and initialize a new symbol table.
-   CUST is from the result of allocate_compunit_symtab.  */
+/* Allocate and initialize a new symbol table.  */
 
 struct symtab *
 allocate_symtab (struct compunit_symtab *cust, const char *filename,
@@ -2865,41 +2864,15 @@ allocate_symtab (struct compunit_symtab *cust, const char *filename,
   return symtab;
 }
 
-/* Allocate and initialize a new compunit.
-   NAME is the name of the main source file, if there is one, or some
-   descriptive text if there are no source files.  */
+/* See symfile.h.  */
 
-struct compunit_symtab *
-allocate_compunit_symtab (struct objfile *objfile, const char *name)
+compunit_symtab *
+add_compunit_symtab_to_objfile (std::unique_ptr<compunit_symtab> cu)
 {
-  struct compunit_symtab *cu = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-					       struct compunit_symtab);
-  const char *saved_name;
-
-  cu->set_objfile (objfile);
-
-  /* The name we record here is only for display/debugging purposes.
-     Just save the basename to avoid path issues (too long for display,
-     relative vs absolute, etc.).  */
-  saved_name = lbasename (name);
-  cu->name = obstack_strdup (&objfile->objfile_obstack, saved_name);
-
-  cu->set_debugformat ("unknown");
-
-  symtab_create_debug_printf_v ("created compunit symtab %s for %s",
-				host_address_to_string (cu),
-				cu->name);
-
-  return cu;
-}
-
-/* Hook CU to the objfile it comes from.  */
-
-void
-add_compunit_symtab_to_objfile (struct compunit_symtab *cu)
-{
-  cu->next = cu->objfile ()->compunit_symtabs;
-  cu->objfile ()->compunit_symtabs = cu;
+  compunit_symtab *result = cu.get ();
+  struct objfile *objfile = result->objfile ();
+  objfile->compunit_symtabs.push_back (std::move (cu));
+  return result;
 }
 
 

gdb/symfile.h

@@ -212,11 +212,10 @@ allocate_symtab (struct compunit_symtab *cust, const char *filename)
   return allocate_symtab (cust, filename, filename);
 }
 
-extern struct compunit_symtab *allocate_compunit_symtab (struct objfile *,
-							 const char *)
-  ATTRIBUTE_NONNULL (1);
-
-extern void add_compunit_symtab_to_objfile (struct compunit_symtab *cu);
+/* Add CU to its objfile, transferring ownership to the objfile.
+   Returns a pointer to the compunit symtab.  */
+extern compunit_symtab *add_compunit_symtab_to_objfile
+     (std::unique_ptr<compunit_symtab> cu);
 
 extern void add_symtab_fns (enum bfd_flavour flavour, const struct sym_fns *);
 

gdb/symmisc.c

@@ -122,23 +122,29 @@ dump_objfile (struct objfile *objfile)
 
   objfile->dump ();
 
-  if (objfile->compunit_symtabs != NULL)
+  bool symtabs_printed = false;
+  for (compunit_symtab *cu : objfile->compunits ())
     {
-      gdb_printf ("Symtabs:\n");
-      for (compunit_symtab *cu : objfile->compunits ())
+      if (!symtabs_printed)
 	{
-	  for (symtab *symtab : cu->filetabs ())
-	    {
-	      gdb_printf ("%s at %s",
-			  symtab_to_filename_for_display (symtab),
-			  host_address_to_string (symtab));
-	      if (symtab->compunit ()->objfile () != objfile)
-		gdb_printf (", NOT ON CHAIN!");
-	      gdb_printf ("\n");
-	    }
+	  gdb_printf ("Symtabs:\n");
+	  symtabs_printed = true;
+	}
+
+      for (symtab *symtab : cu->filetabs ())
+	{
+	  gdb_printf ("%s at %s",
+		      symtab_to_filename_for_display (symtab),
+		      host_address_to_string (symtab));
+	  if (symtab->compunit ()->objfile () != objfile)
+	    gdb_printf (", NOT ON CHAIN!");
+	  gdb_printf ("\n");
 	}
-      gdb_printf ("\n\n");
     }
+
+  /* If we printed any symtabs, print some newlines.  */
+  if (symtabs_printed)
+    gdb_printf ("\n\n");
 }
 
 /* Print minimal symbols from this objfile.  */

gdb/symtab.c

@@ -493,8 +493,24 @@ compunit_symtab::forget_cached_source_info ()
 
 /* See symtab.h.  */
 
-void
-compunit_symtab::finalize ()
+compunit_symtab::compunit_symtab (struct objfile *objfile,
+				  const char *name_)
+  : m_objfile (objfile),
+    /* The name we record here is only for display/debugging purposes.
+       Just save the basename to avoid path issues (too long for
+       display, relative vs absolute, etc.).  */
+    name (obstack_strdup (&objfile->objfile_obstack, lbasename (name_))),
+    m_locations_valid (false),
+    m_epilogue_unwind_valid (false)
+{
+  symtab_create_debug_printf_v ("created compunit symtab %s for %s",
+				host_address_to_string (this),
+				name);
+}
+
+/* See symtab.h.  */
+
+compunit_symtab::~compunit_symtab ()
 {
   this->forget_cached_source_info ();
   delete m_call_site_htab;

gdb/symtab.h

@@ -1830,18 +1830,18 @@ using symtab_range = next_range<symtab>;
    where "foo.c(cu)" and "bar.c(cu)" are struct compunit_symtab objects,
    and the files foo.c, etc. are struct symtab objects.  */
 
-struct compunit_symtab
+struct compunit_symtab : intrusive_list_node<compunit_symtab>
 {
+  compunit_symtab (struct objfile *objfile, const char *name);
+  ~compunit_symtab ();
+
+  DISABLE_COPY_AND_ASSIGN (compunit_symtab);
+
   struct objfile *objfile () const
   {
     return m_objfile;
   }
 
-  void set_objfile (struct objfile *objfile)
-  {
-    m_objfile = objfile;
-  }
-
   symtab_range filetabs () const
   {
     next_iterator<symtab> begin (m_filetabs);
@@ -1959,15 +1959,6 @@ struct compunit_symtab
   /* Clear any cached source file names.  */
   void forget_cached_source_info ();
 
-  /* This is called when an objfile is being destroyed and will free
-     any resources used by this compunit_symtab.  Normally a
-     destructor would be used instead, but at the moment
-     compunit_symtab objects are allocated on an obstack.  */
-  void finalize ();
-
-  /* Unordered chain of all compunit symtabs of this objfile.  */
-  struct compunit_symtab *next;
-
   /* Object file from which this symtab information was read.  */
   struct objfile *m_objfile;
 
@@ -1980,30 +1971,30 @@ struct compunit_symtab
      source file (e.g., .c, .cc) is guaranteed to be first.
      Each symtab is a file, either the "main" source file (e.g., .c, .cc)
      or header (e.g., .h).  */
-  symtab *m_filetabs;
+  symtab *m_filetabs = nullptr;
 
   /* Last entry in FILETABS list.
      Subfiles are added to the end of the list so they accumulate in order,
      with the main source subfile living at the front.
      The main reason is so that the main source file symtab is at the head
      of the list, and the rest appear in order for debugging convenience.  */
-  symtab *m_last_filetab;
+  symtab *m_last_filetab = nullptr;
 
   /* Non-NULL string that identifies the format of the debugging information,
      such as "stabs", "dwarf 1", "dwarf 2", "coff", etc.  This is mostly useful
      for automated testing of gdb but may also be information that is
      useful to the user.  */
-  const char *m_debugformat;
+  const char *m_debugformat = "unknown";
 
   /* String of producer version information, or NULL if we don't know.  */
-  const char *m_producer;
+  const char *m_producer = nullptr;
 
   /* Directory in which it was compiled, or NULL if we don't know.  */
-  const char *m_dirname;
+  const char *m_dirname = nullptr;
 
   /* List of all symbol scope blocks for this symtab.  It is shared among
      all symtabs in a given compilation unit.  */
-  struct blockvector *m_blockvector;
+  struct blockvector *m_blockvector = nullptr;
 
   /* Symtab has been compiled with both optimizations and debug info so that
      GDB may stop skipping prologues as variables locations are valid already
@@ -2015,13 +2006,13 @@ struct compunit_symtab
   unsigned int m_epilogue_unwind_valid : 1;
 
   /* struct call_site entries for this compilation unit or NULL.  */
-  call_site_htab_t *m_call_site_htab;
+  call_site_htab_t *m_call_site_htab = nullptr;
 
   /* The macro table for this symtab.  Like the blockvector, this
      is shared between different symtabs in a given compilation unit.
      It's debatable whether it *should* be shared among all the symtabs in
      the given compilation unit, but it currently is.  */
-  struct macro_table *m_macro_table;
+  struct macro_table *m_macro_table = nullptr;
 
   /* If non-NULL, then this points to a NULL-terminated vector of
      included compunits.  When searching the static or global
@@ -2030,17 +2021,15 @@ struct compunit_symtab
      list must be flattened -- the symbol reader is responsible for
      ensuring that this vector contains the transitive closure of all
      included compunits.  */
-  struct compunit_symtab **includes;
+  struct compunit_symtab **includes = nullptr;
 
   /* If this is an included compunit, this points to one includer
      of the table.  This user is considered the canonical compunit
      containing this one.  An included compunit may itself be
      included by another.  */
-  struct compunit_symtab *user;
+  struct compunit_symtab *user = nullptr;
 };
 
-using compunit_symtab_range = next_range<compunit_symtab>;
-
 /* Return true if this symtab is the "main" symtab of its compunit_symtab.  */
 
 static inline bool