[PATCH] gdb: replace msym_bunch with deque

simon.marchi
2025-12-17T04:32:22Z

From: Simon Marchi <simon.marchi@efficios.com>

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_reade::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 am 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++;
}

I am not sure when minsyms_read would already be true when reading
minimal symbols, and if we should do anything differently here.

Change-Id: I7d10c6aca42cc9dcf80b483394e1e56351a9465f
---
gdb/minsyms.c | 95 ++++++++-------------------------------------------
gdb/minsyms.h | 23 ++++---------
2 files changed, 21 insertions(+), 97 deletions(-)

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 70f5f61748a6..1b1e43845778 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 @@ class minimal_symbol_install_worker
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<minimal_symbol>
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 ed38044a38c6..3467d5dc8d95 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -20,6 +20,8 @@
#ifndef GDB_MINSYMS_H
#define GDB_MINSYMS_H

+#include <deque>
+
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 recod_full. */
+ std::deque<minimal_symbol> m_msyms;
};

\f

base-commit: 04a9afc1d81058275219b499fd75c983967a30c1
--
2.52.0