From: Tom Tromey Date: Mon, 15 Sep 2025 22:46:25 +0000 (-0600) Subject: Allocate compunit_symtab on heap X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=23e3008c3eadd2825ebbd7953a894cfd58f89e12;p=thirdparty%2Fbinutils-gdb.git 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 --- diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 6dc079f29b1..2a8e95e078b 100644 --- a/gdb/buildsym.c +++ b/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 (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; } diff --git a/gdb/buildsym.h b/gdb/buildsym.h index 8f38131ec55..5d4b582349a 100644 --- a/gdb/buildsym.h +++ b/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 m_owned_compunit_symtab; + compunit_symtab *m_compunit_symtab; /* Language of this compunit_symtab. */ enum language m_language; diff --git a/gdb/jit.c b/gdb/jit.c index a1e41fc2ac7..ca817e85c71 100644 --- a/gdb/jit.c +++ b/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 (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); diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c index a7d26c3422c..9504e5e1670 100644 --- a/gdb/mdebugread.c +++ b/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 (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)); diff --git a/gdb/objfiles.c b/gdb/objfiles.c index 18189efabb5..482e12e717f 100644 --- a/gdb/objfiles.c +++ b/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. */ diff --git a/gdb/objfiles.h b/gdb/objfiles.h index 9dbb46ab610..0f9c7709d06 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -33,6 +33,8 @@ #include "quick-symbol.h" #include #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::iterator>; + using compunit_symtab_range = iterator_range; + /* A range adapter that makes it possible to iterate over all compunits in one objfile. */ compunit_symtab_range compunits () { - next_iterator 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_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 diff --git a/gdb/symfile.c b/gdb/symfile.c index b15782ea9de..eb969249f3f 100644 --- a/gdb/symfile.c +++ b/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. */ - -struct compunit_symtab * -allocate_compunit_symtab (struct objfile *objfile, const char *name) -{ - 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. */ +/* See symfile.h. */ -void -add_compunit_symtab_to_objfile (struct compunit_symtab *cu) +compunit_symtab * +add_compunit_symtab_to_objfile (std::unique_ptr 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; } diff --git a/gdb/symfile.h b/gdb/symfile.h index 7e7de3800d8..418e0847df4 100644 --- a/gdb/symfile.h +++ b/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 cu); extern void add_symtab_fns (enum bfd_flavour flavour, const struct sym_fns *); diff --git a/gdb/symmisc.c b/gdb/symmisc.c index a74891bcb3e..f031d00a77e 100644 --- a/gdb/symmisc.c +++ b/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. */ diff --git a/gdb/symtab.c b/gdb/symtab.c index 216a4c18c0c..3d8583cf76f 100644 --- a/gdb/symtab.c +++ b/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; diff --git a/gdb/symtab.h b/gdb/symtab.h index 49daac5112c..20131ffd9e6 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1830,18 +1830,18 @@ using symtab_range = next_range; 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 (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 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; - /* Return true if this symtab is the "main" symtab of its compunit_symtab. */ static inline bool