[binutils-gdb] Use malloc for mutable addrmaps

[binutils-gdb] Use malloc for mutable addrmaps

[Tom Tromey]

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

commit 93b527ef787b7c98611abd21bdd77de6c41769f1
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Apr 16 10:12:49 2022 -0600

    Use malloc for mutable addrmaps
    
    Mutable addrmaps currently require an obstack.  This was probably done
    to avoid having to call splay_tree_delete, but examination of the code
    shows that all mutable obstacks have a limited lifetime -- now it's
    simple to treat them as ordinary C++ objects, in some cases
    stack-allocating them, and have a destructor to make the needed call.
    This patch implements this change.

Diff:
---
 gdb/addrmap.c     | 65 +++++++++++++++----------------------------------------
 gdb/addrmap.h     | 23 +++++---------------
 gdb/buildsym.c    | 12 +++-------
 gdb/buildsym.h    |  8 ++-----
 gdb/dwarf2/read.c | 43 +++++++++++++-----------------------
 5 files changed, 43 insertions(+), 108 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 51a6c676ec6..06f3a831df9 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -102,11 +102,11 @@ addrmap_fixed::foreach (addrmap_foreach_fn fn)
 \f
 /* Mutable address maps.  */
 
-/* Allocate a copy of CORE_ADDR in the obstack.  */
+/* Allocate a copy of CORE_ADDR.  */
 splay_tree_key
 addrmap_mutable::allocate_key (CORE_ADDR addr)
 {
-  CORE_ADDR *key = XOBNEW (obstack, CORE_ADDR);
+  CORE_ADDR *key = XNEW (CORE_ADDR);
 
   *key = addr;
   return (splay_tree_key) key;
@@ -325,42 +325,6 @@ addrmap_mutable::foreach (addrmap_foreach_fn fn)
 }
 
 
-void *
-addrmap_mutable::splay_obstack_alloc (int size, void *closure)
-{
-  struct addrmap_mutable *map = (struct addrmap_mutable *) closure;
-  splay_tree_node n;
-
-  /* We should only be asked to allocate nodes and larger things.
-     (If, at some point in the future, this is no longer true, we can
-     just round up the size to sizeof (*n).)  */
-  gdb_assert (size >= sizeof (*n));
-
-  if (map->free_nodes)
-    {
-      n = map->free_nodes;
-      map->free_nodes = n->right;
-      return n;
-    }
-  else
-    return obstack_alloc (map->obstack, size);
-}
-
-
-void
-addrmap_mutable::splay_obstack_free (void *obj, void *closure)
-{
-  struct addrmap_mutable *map = (struct addrmap_mutable *) closure;
-  splay_tree_node n = (splay_tree_node) obj;
-
-  /* We've asserted in the allocation function that we only allocate
-     nodes or larger things, so it should be safe to put whatever
-     we get passed back on the free list.  */
-  n->right = map->free_nodes;
-  map->free_nodes = n;
-}
-
-
 /* Compare keys as CORE_ADDR * values.  */
 static int
 splay_compare_CORE_ADDR_ptr (splay_tree_key ak, splay_tree_key bk)
@@ -378,15 +342,21 @@ splay_compare_CORE_ADDR_ptr (splay_tree_key ak, splay_tree_key bk)
 }
 
 
-addrmap_mutable::addrmap_mutable (struct obstack *obs)
-  : obstack (obs),
-    tree (splay_tree_new_with_allocator (splay_compare_CORE_ADDR_ptr,
-					 NULL, /* no delete key */
-					 NULL, /* no delete value */
-					 splay_obstack_alloc,
-					 splay_obstack_free,
-					 this))
+static void
+xfree_wrapper (splay_tree_key key)
+{
+  xfree ((void *) key);
+}
+
+addrmap_mutable::addrmap_mutable ()
+  : tree (splay_tree_new (splay_compare_CORE_ADDR_ptr, xfree_wrapper,
+			  nullptr /* no delete value */))
+{
+}
+
+addrmap_mutable::~addrmap_mutable ()
 {
+  splay_tree_delete (tree);
 }
 
 
@@ -461,8 +431,7 @@ test_addrmap ()
   /* Create mutable addrmap.  */
   struct obstack temp_obstack;
   obstack_init (&temp_obstack);
-  struct addrmap_mutable *map
-    = new (&temp_obstack) addrmap_mutable (&temp_obstack);
+  struct addrmap_mutable *map = new (&temp_obstack) addrmap_mutable;
   SELF_CHECK (map != nullptr);
 
   /* Check initial state.  */
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index a3b1d6ad4a8..bdf1199481f 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -32,7 +32,7 @@
    Address maps come in two flavors: fixed, and mutable.  Mutable
    address maps consume more memory, but can be changed and extended.
    A fixed address map, once constructed (from a mutable address map),
-   can't be edited.  Both kinds of map are allocated in obstacks.  */
+   can't be edited.  */
 
 /* The type of a function used to iterate over the map.
    OBJ is NULL for unmapped regions.  */
@@ -40,7 +40,7 @@ typedef gdb::function_view<int (CORE_ADDR start_addr, void *obj)>
      addrmap_foreach_fn;
 
 /* The base class for addrmaps.  */
-struct addrmap : public allocate_on_obstack
+struct addrmap
 {
   virtual ~addrmap () = default;
 
@@ -101,7 +101,8 @@ struct addrmap : public allocate_on_obstack
 struct addrmap_mutable;
 
 /* Fixed address maps.  */
-struct addrmap_fixed : public addrmap
+struct addrmap_fixed : public addrmap,
+		       public allocate_on_obstack
 {
 public:
 
@@ -142,7 +143,8 @@ struct addrmap_mutable : public addrmap
 {
 public:
 
-  explicit addrmap_mutable (struct obstack *obs);
+  addrmap_mutable ();
+  ~addrmap_mutable ();
   DISABLE_COPY_AND_ASSIGN (addrmap_mutable);
 
   void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
@@ -153,16 +155,6 @@ public:
 
 private:
 
-  /* The obstack to use for allocations for this map.  */
-  struct obstack *obstack;
-
-  /* A freelist for splay tree nodes, allocated on obstack, and
-     chained together by their 'right' pointers.  */
-  /* splay_tree_new_with_allocator uses the provided allocation
-     function to allocate the main splay_tree structure itself, so our
-     free list has to be initialized before we create the tree.  */
-  splay_tree_node free_nodes = nullptr;
-
   /* A splay tree, with a node for each transition; there is a
      transition at address T if T-1 and T map to different objects.
 
@@ -190,9 +182,6 @@ private:
   splay_tree_node splay_tree_successor (CORE_ADDR addr);
   void splay_tree_remove (CORE_ADDR addr);
   void splay_tree_insert (CORE_ADDR key, void *value);
-
-  static void *splay_obstack_alloc (int size, void *closure);
-  static void splay_obstack_free (void *obj, void *closure);
 };
 
 
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 019c2055f60..65c2ac5aff0 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -33,7 +33,6 @@
 #include "block.h"
 #include "cp-support.h"
 #include "dictionary.h"
-#include "addrmap.h"
 #include <algorithm>
 
 /* For cleanup_undefined_stabs_types and finish_global_stabs (somewhat
@@ -418,12 +417,7 @@ buildsym_compunit::record_block_range (struct block *block,
       || end_inclusive + 1 != block->end ())
     m_pending_addrmap_interesting = true;
 
-  if (m_pending_addrmap == nullptr)
-    m_pending_addrmap
-      = (new (&m_pending_addrmap_obstack) addrmap_mutable
-	 (&m_pending_addrmap_obstack));
-
-  m_pending_addrmap->set_empty (start, end_inclusive, block);
+  m_pending_addrmap.set_empty (start, end_inclusive, block);
 }
 
 struct blockvector *
@@ -458,10 +452,10 @@ buildsym_compunit::make_blockvector ()
 
   /* If we needed an address map for this symtab, record it in the
      blockvector.  */
-  if (m_pending_addrmap != nullptr && m_pending_addrmap_interesting)
+  if (m_pending_addrmap_interesting)
     blockvector->set_map
       (new (&m_objfile->objfile_obstack) addrmap_fixed
-       (&m_objfile->objfile_obstack, m_pending_addrmap));
+       (&m_objfile->objfile_obstack, &m_pending_addrmap));
   else
     blockvector->set_map (nullptr);
 
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index c1cd5192a79..2ad00336269 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -21,6 +21,7 @@
 
 #include "gdbsupport/gdb_obstack.h"
 #include "symtab.h"
+#include "addrmap.h"
 
 struct objfile;
 struct symbol;
@@ -385,12 +386,7 @@ private:
   /* The mutable address map for the compilation unit whose symbols
      we're currently reading.  The symtabs' shared blockvector will
      point to a fixed copy of this.  */
-  struct addrmap_mutable *m_pending_addrmap = nullptr;
-
-  /* The obstack on which we allocate pending_addrmap.
-     If pending_addrmap is NULL, this is uninitialized; otherwise, it is
-     initialized (and holds pending_addrmap).  */
-  auto_obstack m_pending_addrmap_obstack;
+  struct addrmap_mutable m_pending_addrmap;
 
   /* True if we recorded any ranges in the addrmap that are different
      from those in the blockvector already.  We set this to false when
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 96378c3a09f..d3e6db4c109 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2267,12 +2267,9 @@ create_addrmap_from_index (dwarf2_per_objfile *per_objfile,
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
   struct gdbarch *gdbarch = objfile->arch ();
   const gdb_byte *iter, *end;
-  struct addrmap_mutable *mutable_map;
   CORE_ADDR baseaddr;
 
-  auto_obstack temp_obstack;
-
-  mutable_map = new (&temp_obstack) addrmap_mutable (&temp_obstack);
+  addrmap_mutable mutable_map;
 
   iter = index->address_table.data ();
   end = iter + index->address_table.size ();
@@ -2305,11 +2302,11 @@ create_addrmap_from_index (dwarf2_per_objfile *per_objfile,
 
       lo = gdbarch_adjust_dwarf2_addr (gdbarch, lo + baseaddr) - baseaddr;
       hi = gdbarch_adjust_dwarf2_addr (gdbarch, hi + baseaddr) - baseaddr;
-      mutable_map->set_empty (lo, hi - 1, per_bfd->get_cu (cu_index));
+      mutable_map.set_empty (lo, hi - 1, per_bfd->get_cu (cu_index));
     }
 
   per_bfd->index_addrmap
-    = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, mutable_map);
+    = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, &mutable_map);
 }
 
 /* Read the address map data from DWARF-5 .debug_aranges, and use it
@@ -2496,14 +2493,12 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 {
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
 
-  auto_obstack temp_obstack;
-  addrmap_mutable *mutable_map
-    = new (&temp_obstack) addrmap_mutable (&temp_obstack);
+  addrmap_mutable mutable_map;
 
-  if (read_addrmap_from_aranges (per_objfile, section, mutable_map))
+  if (read_addrmap_from_aranges (per_objfile, section, &mutable_map))
     per_bfd->index_addrmap
       = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack,
-					       mutable_map);
+					       &mutable_map);
 }
 
 /* A helper function that reads the .gdb_index from BUFFER and fills
@@ -6554,9 +6549,7 @@ public:
 					eq_cutu_reader,
 					htab_delete_entry<cutu_reader>,
 					xcalloc, xfree)),
-      m_index (new cooked_index),
-      m_addrmap_storage (),
-      m_addrmap (new (&m_addrmap_storage) addrmap_mutable (&m_addrmap_storage))
+      m_index (new cooked_index)
   {
   }
 
@@ -6606,14 +6599,14 @@ public:
      then transfer ownership of the index to the caller.  */
   std::unique_ptr<cooked_index> release ()
   {
-    m_index->install_addrmap (m_addrmap);
+    m_index->install_addrmap (&m_addrmap);
     return std::move (m_index);
   }
 
   /* Return the mutable addrmap that is currently being created.  */
   addrmap_mutable *get_addrmap ()
   {
-    return m_addrmap;
+    return &m_addrmap;
   }
 
 private:
@@ -6640,10 +6633,8 @@ private:
   /* The index that is being constructed.  */
   std::unique_ptr<cooked_index> m_index;
 
-  /* Storage for the writeable addrmap.  */
-  auto_obstack m_addrmap_storage;
   /* A writeable addrmap being constructed by this scanner.  */
-  addrmap_mutable *m_addrmap;
+  addrmap_mutable m_addrmap;
 };
 
 /* An instance of this is created to index a CU.  */
@@ -6657,9 +6648,7 @@ public:
 		  enum language language)
     : m_index_storage (storage),
       m_per_cu (per_cu),
-      m_language (language),
-      m_obstack (),
-      m_die_range_map (new (&m_obstack) addrmap_mutable (&m_obstack))
+      m_language (language)
   {
   }
 
@@ -6744,12 +6733,10 @@ private:
   /* The language that we're assuming when reading.  */
   enum language m_language;
 
-  /* Temporary storage.  */
-  auto_obstack m_obstack;
   /* An addrmap that maps from section offsets (see the form_addr
      method) to newly-created entries.  See m_deferred_entries to
      understand this.  */
-  addrmap *m_die_range_map;
+  addrmap_mutable m_die_range_map;
 
   /* A single deferred entry.  */
   struct deferred_entry
@@ -18146,7 +18133,7 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 	    {
 	      CORE_ADDR lookup = form_addr (origin_offset, origin_is_dwz);
 	      *parent_entry
-		= (cooked_index_entry *) m_die_range_map->find (lookup);
+		= (cooked_index_entry *) m_die_range_map.find (lookup);
 	    }
 
 	  unsigned int bytes_read;
@@ -18263,7 +18250,7 @@ cooked_indexer::recurse (cutu_reader *reader,
 				   reader->cu->per_cu->is_dwz);
       CORE_ADDR end = form_addr (sect_offset (info_ptr - 1 - reader->buffer),
 				 reader->cu->per_cu->is_dwz);
-      m_die_range_map->set_empty (start, end, (void *) parent_entry);
+      m_die_range_map.set_empty (start, end, (void *) parent_entry);
     }
 
   return info_ptr;
@@ -18436,7 +18423,7 @@ cooked_indexer::make_index (cutu_reader *reader)
     {
       CORE_ADDR key = form_addr (entry.die_offset, m_per_cu->is_dwz);
       cooked_index_entry *parent
-	= (cooked_index_entry *) m_die_range_map->find (key);
+	= (cooked_index_entry *) m_die_range_map.find (key);
       m_index_storage->add (entry.die_offset, entry.tag, entry.flags,
 			    entry.name, parent, m_per_cu);
     }