From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2126) id 470A038560B3; Sun, 12 Jun 2022 16:56:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 470A038560B3 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Tom Tromey To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Use malloc for mutable addrmaps X-Act-Checkin: binutils-gdb X-Git-Author: Tom Tromey X-Git-Refname: refs/heads/master X-Git-Oldrev: d89120e9493281a60d6e7280e9cfa3741ea7e379 X-Git-Newrev: 93b527ef787b7c98611abd21bdd77de6c41769f1 Message-Id: <20220612165601.470A038560B3@sourceware.org> Date: Sun, 12 Jun 2022 16:56:01 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Jun 2022 16:56:01 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D93b527ef787b= 7c98611abd21bdd77de6c41769f1 commit 93b527ef787b7c98611abd21bdd77de6c41769f1 Author: Tom Tromey Date: Sat Apr 16 10:12:49 2022 -0600 Use malloc for mutable addrmaps =20 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) =0C /* Mutable address maps. */ =20 -/* 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 =3D XOBNEW (obstack, CORE_ADDR); + CORE_ADDR *key =3D XNEW (CORE_ADDR); =20 *key =3D addr; return (splay_tree_key) key; @@ -325,42 +325,6 @@ addrmap_mutable::foreach (addrmap_foreach_fn fn) } =20 =20 -void * -addrmap_mutable::splay_obstack_alloc (int size, void *closure) -{ - struct addrmap_mutable *map =3D (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 >=3D sizeof (*n)); - - if (map->free_nodes) - { - n =3D map->free_nodes; - map->free_nodes =3D 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 =3D (struct addrmap_mutable *) closure; - splay_tree_node n =3D (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 =3D map->free_nodes; - map->free_nodes =3D 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) } =20 =20 -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); } =20 =20 @@ -461,8 +431,7 @@ test_addrmap () /* Create mutable addrmap. */ struct obstack temp_obstack; obstack_init (&temp_obstack); - struct addrmap_mutable *map - =3D new (&temp_obstack) addrmap_mutable (&temp_obstack); + struct addrmap_mutable *map =3D new (&temp_obstack) addrmap_mutable; SELF_CHECK (map !=3D nullptr); =20 /* 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. */ =20 /* 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; =20 /* The base class for addrmaps. */ -struct addrmap : public allocate_on_obstack +struct addrmap { virtual ~addrmap () =3D default; =20 @@ -101,7 +101,8 @@ struct addrmap : public allocate_on_obstack struct addrmap_mutable; =20 /* Fixed address maps. */ -struct addrmap_fixed : public addrmap +struct addrmap_fixed : public addrmap, + public allocate_on_obstack { public: =20 @@ -142,7 +143,8 @@ struct addrmap_mutable : public addrmap { public: =20 - explicit addrmap_mutable (struct obstack *obs); + addrmap_mutable (); + ~addrmap_mutable (); DISABLE_COPY_AND_ASSIGN (addrmap_mutable); =20 void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, @@ -153,16 +155,6 @@ public: =20 private: =20 - /* 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 =3D 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. =20 @@ -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); }; =20 =20 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 =20 /* For cleanup_undefined_stabs_types and finish_global_stabs (somewhat @@ -418,12 +417,7 @@ buildsym_compunit::record_block_range (struct block *b= lock, || end_inclusive + 1 !=3D block->end ()) m_pending_addrmap_interesting =3D true; =20 - if (m_pending_addrmap =3D=3D nullptr) - m_pending_addrmap - =3D (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); } =20 struct blockvector * @@ -458,10 +452,10 @@ buildsym_compunit::make_blockvector () =20 /* If we needed an address map for this symtab, record it in the blockvector. */ - if (m_pending_addrmap !=3D 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); =20 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 @@ =20 #include "gdbsupport/gdb_obstack.h" #include "symtab.h" +#include "addrmap.h" =20 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 =3D 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; =20 /* 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_o= bjfile, dwarf2_per_bfd *per_bfd =3D per_objfile->per_bfd; struct gdbarch *gdbarch =3D objfile->arch (); const gdb_byte *iter, *end; - struct addrmap_mutable *mutable_map; CORE_ADDR baseaddr; =20 - auto_obstack temp_obstack; - - mutable_map =3D new (&temp_obstack) addrmap_mutable (&temp_obstack); + addrmap_mutable mutable_map; =20 iter =3D index->address_table.data (); end =3D iter + index->address_table.size (); @@ -2305,11 +2302,11 @@ create_addrmap_from_index (dwarf2_per_objfile *per_= objfile, =20 lo =3D gdbarch_adjust_dwarf2_addr (gdbarch, lo + baseaddr) - baseadd= r; hi =3D gdbarch_adjust_dwarf2_addr (gdbarch, hi + baseaddr) - baseadd= r; - 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)); } =20 per_bfd->index_addrmap - =3D new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, mutable_= map); + =3D new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, &mutable= _map); } =20 /* Read the address map data from DWARF-5 .debug_aranges, and use it @@ -2496,14 +2493,12 @@ create_addrmap_from_aranges (dwarf2_per_objfile *pe= r_objfile, { dwarf2_per_bfd *per_bfd =3D per_objfile->per_bfd; =20 - auto_obstack temp_obstack; - addrmap_mutable *mutable_map - =3D new (&temp_obstack) addrmap_mutable (&temp_obstack); + addrmap_mutable mutable_map; =20 - if (read_addrmap_from_aranges (per_objfile, section, mutable_map)) + if (read_addrmap_from_aranges (per_objfile, section, &mutable_map)) per_bfd->index_addrmap =3D new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, - mutable_map); + &mutable_map); } =20 /* 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_stor= age)) + m_index (new cooked_index) { } =20 @@ -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); } =20 /* Return the mutable addrmap that is currently being created. */ addrmap_mutable *get_addrmap () { - return m_addrmap; + return &m_addrmap; } =20 private: @@ -6640,10 +6633,8 @@ private: /* The index that is being constructed. */ std::unique_ptr m_index; =20 - /* 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; }; =20 /* 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) { } =20 @@ -6744,12 +6733,10 @@ private: /* The language that we're assuming when reading. */ enum language m_language; =20 - /* 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; =20 /* 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 =3D form_addr (origin_offset, origin_is_dwz); *parent_entry - =3D (cooked_index_entry *) m_die_range_map->find (lookup); + =3D (cooked_index_entry *) m_die_range_map.find (lookup); } =20 unsigned int bytes_read; @@ -18263,7 +18250,7 @@ cooked_indexer::recurse (cutu_reader *reader, reader->cu->per_cu->is_dwz); CORE_ADDR end =3D form_addr (sect_offset (info_ptr - 1 - reader->buf= fer), 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); } =20 return info_ptr; @@ -18436,7 +18423,7 @@ cooked_indexer::make_index (cutu_reader *reader) { CORE_ADDR key =3D form_addr (entry.die_offset, m_per_cu->is_dwz); cooked_index_entry *parent - =3D (cooked_index_entry *) m_die_range_map->find (key); + =3D (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); }