[binutils-gdb] Allocate minimal symbols with malloc

[Tom Tromey <tromey at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=042d75e42c5572f333e0e06dabd3c5c4afab486c

commit 042d75e42c5572f333e0e06dabd3c5c4afab486c
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Mar 2 12:29:48 2019 -0700

    Allocate minimal symbols with malloc
    
    Currently, minimal symbols are allocated on the per-BFD obstack.
    However, it is also possible for multiple symbol readers to create
    minimal symbols for a given objfile.  In this case, the minimal
    symbols will be reallocated on the obstack, leading to some waste of
    storage.
    
    This is a memory leak, but I think it won't be caught by tools like
    valgrind, because valgrind doesn't know about obstacks.
    
    This patch fixes the problem by using malloc to allocate the storage
    for minimal symbols.
    
    gdb/ChangeLog
    2019-03-15  Tom Tromey  <tom@tromey.com>
    
    	* objfiles.h (struct objfile_per_bfd_storage) <msymbols>: Now a
    	unique_xmalloc_ptr.
    	(objfile::msymbols_range::begin, objfile::msymbols_range::end):
    	Update.
    	* minsyms.c (lookup_minimal_symbol_by_pc_section)
    	(build_minimal_symbol_hash_tables)
    	(minimal_symbol_reader::install): Update.

Diff:
---
 gdb/ChangeLog  | 10 ++++++++++
 gdb/minsyms.c  | 25 ++++++++++++-------------
 gdb/objfiles.h | 10 ++++------
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3a95320..581a4c6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2019-03-15  Tom Tromey  <tom@tromey.com>
 
+	* objfiles.h (struct objfile_per_bfd_storage) <msymbols>: Now a
+	unique_xmalloc_ptr.
+	(objfile::msymbols_range::begin, objfile::msymbols_range::end):
+	Update.
+	* minsyms.c (lookup_minimal_symbol_by_pc_section)
+	(build_minimal_symbol_hash_tables)
+	(minimal_symbol_reader::install): Update.
+
+2019-03-15  Tom Tromey  <tom@tromey.com>
+
 	* symtab.c (create_demangled_names_hash): Update.
 	(symbol_set_names): Update.
 	* objfiles.h (struct objfile_per_bfd_storage)
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index efeaf2a..88ff259 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -742,7 +742,7 @@ lookup_minimal_symbol_by_pc_section (CORE_ADDR pc_in, struct obj_section *sectio
 	{
 	  int best_zero_sized = -1;
 
-          msymbol = objfile->per_bfd->msymbols;
+          msymbol = objfile->per_bfd->msymbols.get ();
 	  lo = 0;
 	  hi = objfile->per_bfd->minimal_symbol_count - 1;
 
@@ -1298,7 +1298,7 @@ build_minimal_symbol_hash_tables (struct objfile *objfile)
 
   /* Now, (re)insert the actual entries.  */
   for ((i = objfile->per_bfd->minimal_symbol_count,
-	msym = objfile->per_bfd->msymbols);
+	msym = objfile->per_bfd->msymbols.get ());
        i > 0;
        i--, msym++)
     {
@@ -1363,14 +1363,16 @@ minimal_symbol_reader::install ()
       alloc_count = m_msym_count + m_objfile->per_bfd->minimal_symbol_count;
       obstack_blank (&m_objfile->per_bfd->storage_obstack,
 		     alloc_count * sizeof (struct minimal_symbol));
-      msymbols = (struct minimal_symbol *)
-	obstack_base (&m_objfile->per_bfd->storage_obstack);
+      gdb::unique_xmalloc_ptr<minimal_symbol>
+	msym_holder (XNEWVEC (minimal_symbol, alloc_count));
+      msymbols = msym_holder.get ();
 
       /* Copy in the existing minimal symbols, if there are any.  */
 
       if (m_objfile->per_bfd->minimal_symbol_count)
-	memcpy ((char *) msymbols, (char *) m_objfile->per_bfd->msymbols,
-	    m_objfile->per_bfd->minimal_symbol_count * sizeof (struct minimal_symbol));
+	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
@@ -1396,19 +1398,16 @@ minimal_symbol_reader::install ()
          no longer using.  */
 
       mcount = compact_minimal_symbols (msymbols, mcount, m_objfile);
-
-      ssize_t shrink_bytes
-	= (mcount + 1 - alloc_count) * sizeof (struct minimal_symbol);
-      obstack_blank_fast (&m_objfile->per_bfd->storage_obstack, shrink_bytes);
-      msymbols = (struct minimal_symbol *)
-	obstack_finish (&m_objfile->per_bfd->storage_obstack);
+      msym_holder.reset (XRESIZEVEC (struct minimal_symbol,
+				     msym_holder.release (),
+				     mcount));
 
       /* Attach the minimal symbol table to the specified objfile.
          The strings themselves are also located in the storage_obstack
          of this objfile.  */
 
       m_objfile->per_bfd->minimal_symbol_count = mcount;
-      m_objfile->per_bfd->msymbols = msymbols;
+      m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
       /* Now build the hash tables; we can't do this incrementally
          at an earlier point since we weren't finished with the obstack
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 1fa6f3c..b07ddfd 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -282,11 +282,9 @@ struct objfile_per_bfd_storage
      name and a zero value for the address.  This makes it easy to walk
      through the array when passed a pointer to somewhere in the middle
      of it.  There is also a count of the number of symbols, which does
-     not include the terminating null symbol.  The array itself, as well
-     as all the data that it points to, should be allocated on the
-     objfile_obstack for this file.  */
+     not include the terminating null symbol.  */
 
-  minimal_symbol *msymbols = NULL;
+  gdb::unique_xmalloc_ptr<minimal_symbol> msymbols;
   int minimal_symbol_count = 0;
 
   /* The number of minimal symbols read, before any minimal symbol
@@ -375,13 +373,13 @@ struct objfile
 
     minimal_symbol_iterator begin () const
     {
-      return minimal_symbol_iterator (m_objfile->per_bfd->msymbols);
+      return minimal_symbol_iterator (m_objfile->per_bfd->msymbols.get ());
     }
 
     minimal_symbol_iterator end () const
     {
       return minimal_symbol_iterator
-	(m_objfile->per_bfd->msymbols
+	(m_objfile->per_bfd->msymbols.get ()
 	 + m_objfile->per_bfd->minimal_symbol_count);
     }