From: Simon Marchi Date: Wed, 17 Dec 2025 04:31:39 +0000 (-0500) Subject: gdb: replace msym_bunch with deque X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dac3dbbf5e244c01d45bd1aa14e1e5cea7d5a63e;p=thirdparty%2Fbinutils-gdb.git gdb: replace msym_bunch with deque This patch replaces the msym_bunch implementation with an std::deque. I initially tried to replace it with a vector. However, that doesn't work, because minimal_symbol references need to stay valid across calls to minimal_symbol_reader::record_full in at least one spot. elf_symtab_read calls minimal_symbol_reader::record_full (through function record_minimal_symbol) to record a minimal symbol for a PLT entry and then uses a previously added minimal symbol to set the size of the PLT minimal symbol. If we used a vector, a re-allocation could happen which would invalidate the reference to the previous minimal symbol (luckily this was caught by ASan). An std::deque implementation typically uses a sequence of fixed-sized arrays, much like our current msym_bunch implementation. So adding an item at the end will not invalidate existing references. But unlike our msym_bunch, we don't have to worry about memory management. I was a bit puzzled about this code in minimal_symbol_reader::record_full: /* If we already read minimal symbols for this objfile, then don't ever allocate a new one. */ if (!m_objfile->per_bfd->minsyms_read) { m_msym_bunch_index++; m_objfile->per_bfd->n_minsyms++; } From what I understand, this "feature" gets used when you have ECOFF debug info in an ELF executable. We first parse the ELF minimal symbols in elf_symfile_read, then go into elfmdebug_build_psymtabs. elfmdebug_build_psymtabs has the capability to generate minimal symbols (useful when you use ECOFF debug info in an ECOFF executable I guess), but in this case we already have the ELF ones, so we don't want to record the ECOFF minimal symbols. Hence this mechanism to suppress new minimal symbols. The consequence of this patch, I believe, is that minimal_symbol_reader::record_full will unnecessarily allocate minimal symbols in this case. These minimal symbols won't be installed, because of the check in minimal_symbol_reader::install. The minimal symbols will be freed when the minimal_symbol_reader gets destroyed. That means a higher temporary memory usage when reading an ECOFF-in-ELF file, but no change in behavior. See discussion at [1]. [1] https://inbox.sourceware.org/gdb-patches/85958fad-17e1-4593-b842-d60a6610f149@polymtl.ca/T/#meaf9b53da296e7f6872b441ec97038d172ca907f Change-Id: I7d10c6aca42cc9dcf80b483394e1e56351a9465f Approved-By: Tom Tromey --- diff --git a/gdb/minsyms.c b/gdb/minsyms.c index 70f5f61748a..1b1e4384577 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -143,17 +143,6 @@ msymbol_is_function (struct objfile *objfile, minimal_symbol *minsym, } } -/* Accumulate the minimal symbols for each objfile in bunches of BUNCH_SIZE. - At the end, copy them all into one newly allocated array. */ - -#define BUNCH_SIZE 127 - -struct msym_bunch - { - struct msym_bunch *next; - struct minimal_symbol contents[BUNCH_SIZE]; - }; - /* See minsyms.h. */ unsigned int @@ -1087,32 +1076,10 @@ get_symbol_leading_char (program_space *pspace, bfd *abfd) /* See minsyms.h. */ minimal_symbol_reader::minimal_symbol_reader (struct objfile *obj) -: m_objfile (obj), - m_msym_bunch (NULL), - /* Note that presetting m_msym_bunch_index to BUNCH_SIZE causes the - first call to save a minimal symbol to allocate the memory for - the first bunch. */ - m_msym_bunch_index (BUNCH_SIZE), - m_msym_count (0) + : m_objfile (obj) { } -/* Discard the currently collected minimal symbols, if any. If we wish - to save them for later use, we must have already copied them somewhere - else before calling this function. */ - -minimal_symbol_reader::~minimal_symbol_reader () -{ - struct msym_bunch *next; - - while (m_msym_bunch != NULL) - { - next = m_msym_bunch->next; - xfree (m_msym_bunch); - m_msym_bunch = next; - } -} - /* See minsyms.h. */ void @@ -1180,9 +1147,6 @@ minimal_symbol_reader::record_full (std::string_view name, enum minimal_symbol_type ms_type, int section) { - struct msym_bunch *newobj; - struct minimal_symbol *msymbol; - /* Don't put gcc_compiled, __gnu_compiled_cplus, and friends into the minimal symbols, because if there is also another symbol at the same address (e.g. the first function of the file), @@ -1207,14 +1171,7 @@ minimal_symbol_reader::record_full (std::string_view name, hex_string (LONGEST (address)), section, (int) name.size (), name.data ()); - if (m_msym_bunch_index == BUNCH_SIZE) - { - newobj = XCNEW (struct msym_bunch); - m_msym_bunch_index = 0; - newobj->next = m_msym_bunch; - m_msym_bunch = newobj; - } - msymbol = &m_msym_bunch->contents[m_msym_bunch_index]; + minimal_symbol *msymbol = &m_msyms.emplace_back (); msymbol->set_language (language_unknown, &m_objfile->per_bfd->storage_obstack); @@ -1226,17 +1183,13 @@ minimal_symbol_reader::record_full (std::string_view name, msymbol->set_unrelocated_address (address); msymbol->set_section_index (section); - msymbol->set_type (ms_type); /* If we already read minimal symbols for this objfile, then don't ever allocate a new one. */ if (!m_objfile->per_bfd->minsyms_read) - { - m_msym_bunch_index++; - m_objfile->per_bfd->n_minsyms++; - } - m_msym_count++; + m_objfile->per_bfd->n_minsyms++; + return msymbol; } @@ -1465,59 +1418,41 @@ private: void minimal_symbol_reader::install () { - int mcount; - struct msym_bunch *bunch; - struct minimal_symbol *msymbols; - int alloc_count; - if (m_objfile->per_bfd->minsyms_read) return; - if (m_msym_count > 0) + if (!m_msyms.empty ()) { - symtab_create_debug_printf ("installing %d minimal symbols of objfile %s", - m_msym_count, objfile_name (m_objfile)); + symtab_create_debug_printf ("installing %zu minimal symbols of objfile %s", + m_msyms.size (), objfile_name (m_objfile)); /* Allocate enough space, into which we will gather the bunches of new and existing minimal symbols, sort them, and then compact out the duplicate entries. Once we have a final table, we will give back the excess space. */ - - alloc_count = m_msym_count + m_objfile->per_bfd->minimal_symbol_count; + int alloc_count + = m_msyms.size () + m_objfile->per_bfd->minimal_symbol_count; gdb::unique_xmalloc_ptr msym_holder (XNEWVEC (minimal_symbol, alloc_count)); - msymbols = msym_holder.get (); + minimal_symbol *msymbols = msym_holder.get (); /* Copy in the existing minimal symbols, if there are any. */ - if (m_objfile->per_bfd->minimal_symbol_count) memcpy (msymbols, m_objfile->per_bfd->msymbols.get (), m_objfile->per_bfd->minimal_symbol_count * sizeof (struct minimal_symbol)); - /* Walk through the list of minimal symbol bunches, adding each symbol - to the new contiguous array of symbols. Note that we start with the - current, possibly partially filled bunch (thus we use the current - msym_bunch_index for the first bunch we copy over), and thereafter - each bunch is full. */ - - mcount = m_objfile->per_bfd->minimal_symbol_count; - - for (bunch = m_msym_bunch; bunch != NULL; bunch = bunch->next) - { - memcpy (&msymbols[mcount], &bunch->contents[0], - m_msym_bunch_index * sizeof (struct minimal_symbol)); - mcount += m_msym_bunch_index; - m_msym_bunch_index = BUNCH_SIZE; - } + /* Walk through the list of recorded minimal symbol, adding each symbol + to the new contiguous array of symbols. */ + int mcount = m_objfile->per_bfd->minimal_symbol_count; + std::copy (m_msyms.begin (), m_msyms.end (), &msymbols[mcount]); + mcount += m_msyms.size (); /* Sort the minimal symbols by address. */ - std::sort (msymbols, msymbols + mcount, minimal_symbol_is_less_than); /* Compact out any duplicates, and free up whatever space we are no longer using. */ - mcount = compact_minimal_symbols (msymbols, mcount, m_objfile); msym_holder.reset (XRESIZEVEC (struct minimal_symbol, msym_holder.release (), diff --git a/gdb/minsyms.h b/gdb/minsyms.h index ed38044a38c..97be03953d5 100644 --- a/gdb/minsyms.h +++ b/gdb/minsyms.h @@ -20,6 +20,8 @@ #ifndef GDB_MINSYMS_H #define GDB_MINSYMS_H +#include + struct program_space; struct type; @@ -78,8 +80,6 @@ struct bound_minimal_symbol as opaque and use functions provided by minsyms.c to inspect them. */ -struct msym_bunch; - /* An RAII-based object that is used to record minimal symbols while they are being read. */ class minimal_symbol_reader @@ -92,8 +92,6 @@ class minimal_symbol_reader explicit minimal_symbol_reader (struct objfile *); - ~minimal_symbol_reader (); - /* Install the minimal symbols that have been collected into the given objfile. */ @@ -154,19 +152,10 @@ class minimal_symbol_reader struct objfile *m_objfile; - /* Bunch currently being filled up. - The next field points to chain of filled bunches. */ - - struct msym_bunch *m_msym_bunch; - - /* Number of slots filled in current bunch. */ - - int m_msym_bunch_index; - - /* Total number of minimal symbols recorded so far for the - objfile. */ - - int m_msym_count; + /* The minimal symbols recorded so far. This uses a deque instead of e.g. a + vector, because references to minimal symbols need to stay valid across + calls to record_full. */ + std::deque m_msyms; };