From 93b527ef787b7c98611abd21bdd77de6c41769f1 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sat, 16 Apr 2022 10:12:49 -0600 Subject: [PATCH] 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. --- 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) /* 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 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 /* 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, 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 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 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); } -- 2.30.2