public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/9] C++-ify addrmap
@ 2022-04-16 16:58 Tom Tromey
  2022-04-16 16:58 ` [PATCH 1/9] Use inheritance for addrmap Tom Tromey
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches

I did some C++-ification of addrmap for some experiments I'm doing,
and I thought it would be good to clean this up and submit it instead
of keeping it on some branch.

Regression tested on x86-64 Fedora 34.

Tom



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] Use inheritance for addrmap
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 2/9] Privacy for addrmap_fixed Tom Tromey
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This is a simply C++-ification of the basics of addrmap: it uses
virtual methods rather than a table of function pointers, and it
changes the concrete implementations to be subclasses.
---
 gdb/addrmap.c | 186 +++++++++++++++++++++-----------------------------
 1 file changed, 77 insertions(+), 109 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index f88880614ec..f1df53cbe20 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -29,26 +29,18 @@ gdb_static_assert (sizeof (splay_tree_key) >= sizeof (CORE_ADDR *));
 gdb_static_assert (sizeof (splay_tree_value) >= sizeof (void *));
 
 \f
-/* The "abstract class".  */
-
-/* Functions implementing the addrmap functions for a particular
-   implementation.  */
-struct addrmap_funcs
-{
-  void (*set_empty) (struct addrmap *self,
-		     CORE_ADDR start, CORE_ADDR end_inclusive,
-		     void *obj);
-  void *(*find) (struct addrmap *self, CORE_ADDR addr);
-  struct addrmap *(*create_fixed) (struct addrmap *self,
-				   struct obstack *obstack);
-  void (*relocate) (struct addrmap *self, CORE_ADDR offset);
-  int (*foreach) (struct addrmap *self, addrmap_foreach_fn fn);
-};
-
+/* The base class for addrmaps.  */
 
-struct addrmap
+struct addrmap : public allocate_on_obstack
 {
-  const struct addrmap_funcs *funcs;
+  virtual ~addrmap () = default;
+
+  virtual void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+			  void *obj) = 0;
+  virtual void *find (CORE_ADDR addr) = 0;
+  virtual struct addrmap *create_fixed (struct obstack *obstack) = 0;
+  virtual void relocate (CORE_ADDR offset) = 0;
+  virtual int foreach (addrmap_foreach_fn fn) = 0;
 };
 
 
@@ -57,21 +49,21 @@ addrmap_set_empty (struct addrmap *map,
 		   CORE_ADDR start, CORE_ADDR end_inclusive,
 		   void *obj)
 {
-  map->funcs->set_empty (map, start, end_inclusive, obj);
+  map->set_empty (start, end_inclusive, obj);
 }
 
 
 void *
 addrmap_find (struct addrmap *map, CORE_ADDR addr)
 {
-  return map->funcs->find (map, addr);
+  return map->find (addr);
 }
 
 
 struct addrmap *
 addrmap_create_fixed (struct addrmap *original, struct obstack *obstack)
 {
-  return original->funcs->create_fixed (original, obstack);
+  return original->create_fixed (obstack);
 }
 
 
@@ -80,14 +72,14 @@ addrmap_create_fixed (struct addrmap *original, struct obstack *obstack)
 void
 addrmap_relocate (struct addrmap *map, CORE_ADDR offset)
 {
-  map->funcs->relocate (map, offset);
+  map->relocate (offset);
 }
 
 
 int
 addrmap_foreach (struct addrmap *map, addrmap_foreach_fn fn)
 {
-  return map->funcs->foreach (map, fn);
+  return map->foreach (fn);
 }
 \f
 /* Fixed address maps.  */
@@ -102,9 +94,14 @@ struct addrmap_transition
 };
 
 
-struct addrmap_fixed
+struct addrmap_fixed : public addrmap
 {
-  struct addrmap addrmap;
+  void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+		  void *obj) override;
+  void *find (CORE_ADDR addr) override;
+  struct addrmap *create_fixed (struct obstack *obstack) override;
+  void relocate (CORE_ADDR offset) override;
+  int foreach (addrmap_foreach_fn fn) override;
 
   /* The number of transitions in TRANSITIONS.  */
   size_t num_transitions;
@@ -114,14 +111,13 @@ struct addrmap_fixed
      ADDR - 1 is mapped to something different, we have an entry here
      containing ADDR and VALUE.  (Note that this means we always have
      an entry for address 0).  */
-  struct addrmap_transition transitions[1];
+  struct addrmap_transition *transitions;
 };
 
 
-static void
-addrmap_fixed_set_empty (struct addrmap *self,
-		   CORE_ADDR start, CORE_ADDR end_inclusive,
-		   void *obj)
+void
+addrmap_fixed::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+			  void *obj)
 {
   internal_error (__FILE__, __LINE__,
 		  "addrmap_fixed_set_empty: "
@@ -129,12 +125,11 @@ addrmap_fixed_set_empty (struct addrmap *self,
 }
 
 
-static void *
-addrmap_fixed_find (struct addrmap *self, CORE_ADDR addr)
+void *
+addrmap_fixed::find (CORE_ADDR addr)
 {
-  struct addrmap_fixed *map = (struct addrmap_fixed *) self;
-  struct addrmap_transition *bottom = &map->transitions[0];
-  struct addrmap_transition *top = &map->transitions[map->num_transitions - 1];
+  struct addrmap_transition *bottom = &transitions[0];
+  struct addrmap_transition *top = &transitions[num_transitions - 1];
 
   while (bottom < top)
     {
@@ -162,8 +157,8 @@ addrmap_fixed_find (struct addrmap *self, CORE_ADDR addr)
 }
 
 
-static struct addrmap *
-addrmap_fixed_create_fixed (struct addrmap *self, struct obstack *obstack)
+struct addrmap *
+addrmap_fixed::create_fixed (struct obstack *obstack)
 {
   internal_error (__FILE__, __LINE__,
 		  _("addrmap_create_fixed is not implemented yet "
@@ -171,26 +166,24 @@ addrmap_fixed_create_fixed (struct addrmap *self, struct obstack *obstack)
 }
 
 
-static void
-addrmap_fixed_relocate (struct addrmap *self, CORE_ADDR offset)
+void
+addrmap_fixed::relocate (CORE_ADDR offset)
 {
-  struct addrmap_fixed *map = (struct addrmap_fixed *) self;
   size_t i;
 
-  for (i = 0; i < map->num_transitions; i++)
-    map->transitions[i].addr += offset;
+  for (i = 0; i < num_transitions; i++)
+    transitions[i].addr += offset;
 }
 
 
-static int
-addrmap_fixed_foreach (struct addrmap *self, addrmap_foreach_fn fn)
+int
+addrmap_fixed::foreach (addrmap_foreach_fn fn)
 {
-  struct addrmap_fixed *map = (struct addrmap_fixed *) self;
   size_t i;
 
-  for (i = 0; i < map->num_transitions; i++)
+  for (i = 0; i < num_transitions; i++)
     {
-      int res = fn (map->transitions[i].addr, map->transitions[i].value);
+      int res = fn (transitions[i].addr, transitions[i].value);
 
       if (res != 0)
 	return res;
@@ -200,22 +193,17 @@ addrmap_fixed_foreach (struct addrmap *self, addrmap_foreach_fn fn)
 }
 
 
-static const struct addrmap_funcs addrmap_fixed_funcs =
-{
-  addrmap_fixed_set_empty,
-  addrmap_fixed_find,
-  addrmap_fixed_create_fixed,
-  addrmap_fixed_relocate,
-  addrmap_fixed_foreach
-};
-
-
 \f
 /* Mutable address maps.  */
 
-struct addrmap_mutable
+struct addrmap_mutable : public addrmap
 {
-  struct addrmap addrmap;
+  void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+		  void *obj) override;
+  void *find (CORE_ADDR addr) override;
+  struct addrmap *create_fixed (struct obstack *obstack) override;
+  void relocate (CORE_ADDR offset) override;
+  int foreach (addrmap_foreach_fn fn) override;
 
   /* The obstack to use for allocations for this map.  */
   struct obstack *obstack;
@@ -334,12 +322,10 @@ force_transition (struct addrmap_mutable *self, CORE_ADDR addr)
 }
 
 
-static void
-addrmap_mutable_set_empty (struct addrmap *self,
-			   CORE_ADDR start, CORE_ADDR end_inclusive,
-			   void *obj)
+void
+addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+			    void *obj)
 {
-  struct addrmap_mutable *map = (struct addrmap_mutable *) self;
   splay_tree_node n, next;
   void *prior_value;
 
@@ -356,14 +342,14 @@ addrmap_mutable_set_empty (struct addrmap *self,
      - Second pass: remove any unnecessary transitions.  */
 
   /* Establish transitions at the start and end.  */
-  force_transition (map, start);
+  force_transition (this, start);
   if (end_inclusive < CORE_ADDR_MAX)
-    force_transition (map, end_inclusive + 1);
+    force_transition (this, end_inclusive + 1);
 
   /* Walk the area, changing all NULL regions to OBJ.  */
-  for (n = addrmap_splay_tree_lookup (map, start), gdb_assert (n);
+  for (n = addrmap_splay_tree_lookup (this, start), gdb_assert (n);
        n && addrmap_node_key (n) <= end_inclusive;
-       n = addrmap_splay_tree_successor (map, addrmap_node_key (n)))
+       n = addrmap_splay_tree_successor (this, addrmap_node_key (n)))
     {
       if (! addrmap_node_value (n))
 	addrmap_node_set_value (n, obj);
@@ -372,34 +358,33 @@ addrmap_mutable_set_empty (struct addrmap *self,
   /* Walk the area again, removing transitions from any value to
      itself.  Be sure to visit both the transitions we forced
      above.  */
-  n = addrmap_splay_tree_predecessor (map, start);
+  n = addrmap_splay_tree_predecessor (this, start);
   prior_value = n ? addrmap_node_value (n) : NULL;
-  for (n = addrmap_splay_tree_lookup (map, start), gdb_assert (n);
+  for (n = addrmap_splay_tree_lookup (this, start), gdb_assert (n);
        n && (end_inclusive == CORE_ADDR_MAX
 	     || addrmap_node_key (n) <= end_inclusive + 1);
        n = next)
     {
-      next = addrmap_splay_tree_successor (map, addrmap_node_key (n));
+      next = addrmap_splay_tree_successor (this, addrmap_node_key (n));
       if (addrmap_node_value (n) == prior_value)
-	addrmap_splay_tree_remove (map, addrmap_node_key (n));
+	addrmap_splay_tree_remove (this, addrmap_node_key (n));
       else
 	prior_value = addrmap_node_value (n);
     }
 }
 
 
-static void *
-addrmap_mutable_find (struct addrmap *self, CORE_ADDR addr)
+void *
+addrmap_mutable::find (CORE_ADDR addr)
 {
-  struct addrmap_mutable *map = (struct addrmap_mutable *) self;
-  splay_tree_node n = addrmap_splay_tree_lookup (map, addr);
+  splay_tree_node n = addrmap_splay_tree_lookup (this, addr);
   if (n != nullptr)
     {
       gdb_assert (addrmap_node_key (n) == addr);
       return addrmap_node_value (n);
     }
 
-  n = addrmap_splay_tree_predecessor (map, addr);
+  n = addrmap_splay_tree_predecessor (this, addr);
   if (n != nullptr)
     {
       gdb_assert (addrmap_node_key (n) < addr);
@@ -438,43 +423,40 @@ splay_foreach_copy (splay_tree_node n, void *closure)
 }
 
 
-static struct addrmap *
-addrmap_mutable_create_fixed (struct addrmap *self, struct obstack *obstack)
+struct addrmap *
+addrmap_mutable::create_fixed (struct obstack *obstack)
 {
-  struct addrmap_mutable *mutable_obj = (struct addrmap_mutable *) self;
   struct addrmap_fixed *fixed;
   size_t num_transitions;
-  size_t alloc_len;
 
   /* Count the number of transitions in the tree.  */
   num_transitions = 0;
-  splay_tree_foreach (mutable_obj->tree, splay_foreach_count, &num_transitions);
+  splay_tree_foreach (tree, splay_foreach_count, &num_transitions);
 
   /* Include an extra entry for the transition at zero (which fixed
      maps have, but mutable maps do not.)  */
   num_transitions++;
 
-  alloc_len = sizeof (*fixed)
-	      + (num_transitions * sizeof (fixed->transitions[0]));
-  fixed = (struct addrmap_fixed *) obstack_alloc (obstack, alloc_len);
-  fixed->addrmap.funcs = &addrmap_fixed_funcs;
+  fixed = new (obstack) struct addrmap_fixed;
   fixed->num_transitions = 1;
+  fixed->transitions = XOBNEWVEC (obstack, struct addrmap_transition,
+				  num_transitions);
   fixed->transitions[0].addr = 0;
   fixed->transitions[0].value = NULL;
 
   /* Copy all entries from the splay tree to the array, in order 
      of increasing address.  */
-  splay_tree_foreach (mutable_obj->tree, splay_foreach_copy, fixed);
+  splay_tree_foreach (tree, splay_foreach_copy, fixed);
 
   /* We should have filled the array.  */
   gdb_assert (fixed->num_transitions == num_transitions);
 
-  return (struct addrmap *) fixed;
+  return fixed;
 }
 
 
-static void
-addrmap_mutable_relocate (struct addrmap *self, CORE_ADDR offset)
+void
+addrmap_mutable::relocate (CORE_ADDR offset)
 {
   /* Not needed yet.  */
   internal_error (__FILE__, __LINE__,
@@ -494,26 +476,13 @@ addrmap_mutable_foreach_worker (splay_tree_node node, void *data)
 }
 
 
-static int
-addrmap_mutable_foreach (struct addrmap *self, addrmap_foreach_fn fn)
+int
+addrmap_mutable::foreach (addrmap_foreach_fn fn)
 {
-  struct addrmap_mutable *mutable_obj = (struct addrmap_mutable *) self;
-
-  return splay_tree_foreach (mutable_obj->tree, addrmap_mutable_foreach_worker,
-			     &fn);
+  return splay_tree_foreach (tree, addrmap_mutable_foreach_worker, &fn);
 }
 
 
-static const struct addrmap_funcs addrmap_mutable_funcs =
-{
-  addrmap_mutable_set_empty,
-  addrmap_mutable_find,
-  addrmap_mutable_create_fixed,
-  addrmap_mutable_relocate,
-  addrmap_mutable_foreach
-};
-
-
 static void *
 splay_obstack_alloc (int size, void *closure)
 {
@@ -570,9 +539,8 @@ splay_compare_CORE_ADDR_ptr (splay_tree_key ak, splay_tree_key bk)
 struct addrmap *
 addrmap_create_mutable (struct obstack *obstack)
 {
-  struct addrmap_mutable *map = XOBNEW (obstack, struct addrmap_mutable);
+  struct addrmap_mutable *map = new (obstack) struct addrmap_mutable;
 
-  map->addrmap.funcs = &addrmap_mutable_funcs;
   map->obstack = obstack;
 
   /* splay_tree_new_with_allocator uses the provided allocation
@@ -587,7 +555,7 @@ addrmap_create_mutable (struct obstack *obstack)
 					     splay_obstack_free,
 					     map);
 
-  return (struct addrmap *) map;
+  return map;
 }
 
 /* See addrmap.h.  */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/9] Privacy for addrmap_fixed
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
  2022-04-16 16:58 ` [PATCH 1/9] Use inheritance for addrmap Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 3/9] Privacy for addrmap_mutable Tom Tromey
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes addrmap_fixed so that its data members are private.
It also makes struct addrmap_transition private as well.
---
 gdb/addrmap.c | 97 +++++++++++++++++++++++----------------------------
 1 file changed, 44 insertions(+), 53 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index f1df53cbe20..55a1b8c6fff 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -84,18 +84,15 @@ addrmap_foreach (struct addrmap *map, addrmap_foreach_fn fn)
 \f
 /* Fixed address maps.  */
 
-/* A transition: a point in an address map where the value changes.
-   The map maps ADDR to VALUE, but if ADDR > 0, it maps ADDR-1 to
-   something else.  */
-struct addrmap_transition
-{
-  CORE_ADDR addr;
-  void *value;
-};
-
+struct addrmap_mutable;
 
 struct addrmap_fixed : public addrmap
 {
+public:
+
+  addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut);
+  DISABLE_COPY_AND_ASSIGN (addrmap_fixed);
+
   void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 		  void *obj) override;
   void *find (CORE_ADDR addr) override;
@@ -103,6 +100,17 @@ struct addrmap_fixed : public addrmap
   void relocate (CORE_ADDR offset) override;
   int foreach (addrmap_foreach_fn fn) override;
 
+private:
+
+  /* A transition: a point in an address map where the value changes.
+     The map maps ADDR to VALUE, but if ADDR > 0, it maps ADDR-1 to
+     something else.  */
+  struct addrmap_transition
+  {
+    CORE_ADDR addr;
+    void *value;
+  };
+
   /* The number of transitions in TRANSITIONS.  */
   size_t num_transitions;
 
@@ -395,63 +403,46 @@ addrmap_mutable::find (CORE_ADDR addr)
 }
 
 
-/* A function to pass to splay_tree_foreach to count the number of nodes
-   in the tree.  */
-static int
-splay_foreach_count (splay_tree_node n, void *closure)
-{
-  size_t *count = (size_t *) closure;
-
-  (*count)++;
-  return 0;
-}
-
-
-/* A function to pass to splay_tree_foreach to copy entries into a
-   fixed address map.  */
-static int
-splay_foreach_copy (splay_tree_node n, void *closure)
-{
-  struct addrmap_fixed *fixed = (struct addrmap_fixed *) closure;
-  struct addrmap_transition *t = &fixed->transitions[fixed->num_transitions];
-
-  t->addr = addrmap_node_key (n);
-  t->value = addrmap_node_value (n);
-  fixed->num_transitions++;
-
-  return 0;
-}
-
-
-struct addrmap *
-addrmap_mutable::create_fixed (struct obstack *obstack)
+addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut)
 {
-  struct addrmap_fixed *fixed;
-  size_t num_transitions;
+  size_t transition_count = 0;
 
   /* Count the number of transitions in the tree.  */
-  num_transitions = 0;
-  splay_tree_foreach (tree, splay_foreach_count, &num_transitions);
+  addrmap_foreach (mut, [&] (CORE_ADDR start, void *obj)
+    {
+      ++transition_count;
+      return 0;
+    });
 
   /* Include an extra entry for the transition at zero (which fixed
      maps have, but mutable maps do not.)  */
-  num_transitions++;
+  transition_count++;
 
-  fixed = new (obstack) struct addrmap_fixed;
-  fixed->num_transitions = 1;
-  fixed->transitions = XOBNEWVEC (obstack, struct addrmap_transition,
-				  num_transitions);
-  fixed->transitions[0].addr = 0;
-  fixed->transitions[0].value = NULL;
+  num_transitions = 1;
+  transitions = XOBNEWVEC (obstack, struct addrmap_transition,
+			   transition_count);
+  transitions[0].addr = 0;
+  transitions[0].value = NULL;
 
   /* Copy all entries from the splay tree to the array, in order 
      of increasing address.  */
-  splay_tree_foreach (tree, splay_foreach_copy, fixed);
+  addrmap_foreach (mut, [&] (CORE_ADDR start, void *obj)
+    {
+      transitions[num_transitions].addr = start;
+      transitions[num_transitions].value = obj;
+      ++num_transitions;
+      return 0;
+    });
 
   /* We should have filled the array.  */
-  gdb_assert (fixed->num_transitions == num_transitions);
+  gdb_assert (num_transitions == transition_count);
+}
 
-  return fixed;
+
+struct addrmap *
+addrmap_mutable::create_fixed (struct obstack *obstack)
+{
+  return new (obstack) struct addrmap_fixed (obstack, this);
 }
 
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/9] Privacy for addrmap_mutable
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
  2022-04-16 16:58 ` [PATCH 1/9] Use inheritance for addrmap Tom Tromey
  2022-04-16 16:58 ` [PATCH 2/9] Privacy for addrmap_fixed Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 4/9] Move addrmap classes to addrmap.h Tom Tromey
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes addrmap_mutable so that its data members are private.
---
 gdb/addrmap.c | 141 ++++++++++++++++++++++++++++----------------------
 1 file changed, 78 insertions(+), 63 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 55a1b8c6fff..e372e0c6e06 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -206,6 +206,11 @@ addrmap_fixed::foreach (addrmap_foreach_fn fn)
 
 struct addrmap_mutable : public addrmap
 {
+public:
+
+  explicit addrmap_mutable (struct obstack *obs);
+  DISABLE_COPY_AND_ASSIGN (addrmap_mutable);
+
   void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 		  void *obj) override;
   void *find (CORE_ADDR addr) override;
@@ -213,9 +218,18 @@ struct addrmap_mutable : public addrmap
   void relocate (CORE_ADDR offset) override;
   int foreach (addrmap_foreach_fn fn) override;
 
+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.
 
@@ -235,17 +249,25 @@ struct addrmap_mutable : public addrmap
      from deleted nodes; they'll be freed when the obstack is freed.  */
   splay_tree tree;
 
-  /* A freelist for splay tree nodes, allocated on obstack, and
-     chained together by their 'right' pointers.  */
-  splay_tree_node free_nodes;
+  /* Various helper methods.  */
+  splay_tree_key allocate_key (CORE_ADDR addr);
+  void force_transition (CORE_ADDR addr);
+  splay_tree_node splay_tree_lookup (CORE_ADDR addr);
+  splay_tree_node splay_tree_predecessor (CORE_ADDR addr);
+  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);
 };
 
 
-/* Allocate a copy of CORE_ADDR in MAP's obstack.  */
-static splay_tree_key
-allocate_key (struct addrmap_mutable *map, CORE_ADDR addr)
+/* Allocate a copy of CORE_ADDR in the obstack.  */
+splay_tree_key
+addrmap_mutable::allocate_key (CORE_ADDR addr)
 {
-  CORE_ADDR *key = XOBNEW (map->obstack, CORE_ADDR);
+  CORE_ADDR *key = XOBNEW (obstack, CORE_ADDR);
 
   *key = addr;
   return (splay_tree_key) key;
@@ -253,31 +275,31 @@ allocate_key (struct addrmap_mutable *map, CORE_ADDR addr)
 
 
 /* Type-correct wrappers for splay tree access.  */
-static splay_tree_node
-addrmap_splay_tree_lookup (struct addrmap_mutable *map, CORE_ADDR addr)
+splay_tree_node
+addrmap_mutable::splay_tree_lookup (CORE_ADDR addr)
 {
-  return splay_tree_lookup (map->tree, (splay_tree_key) &addr);
+  return ::splay_tree_lookup (tree, (splay_tree_key) &addr);
 }
 
 
-static splay_tree_node
-addrmap_splay_tree_predecessor (struct addrmap_mutable *map, CORE_ADDR addr)
+splay_tree_node
+addrmap_mutable::splay_tree_predecessor (CORE_ADDR addr)
 {
-  return splay_tree_predecessor (map->tree, (splay_tree_key) &addr);
+  return ::splay_tree_predecessor (tree, (splay_tree_key) &addr);
 }
 
 
-static splay_tree_node
-addrmap_splay_tree_successor (struct addrmap_mutable *map, CORE_ADDR addr)
+splay_tree_node
+addrmap_mutable::splay_tree_successor (CORE_ADDR addr)
 {
-  return splay_tree_successor (map->tree, (splay_tree_key) &addr);
+  return ::splay_tree_successor (tree, (splay_tree_key) &addr);
 }
 
 
-static void
-addrmap_splay_tree_remove (struct addrmap_mutable *map, CORE_ADDR addr)
+void
+addrmap_mutable::splay_tree_remove (CORE_ADDR addr)
 {
-  splay_tree_remove (map->tree, (splay_tree_key) &addr);
+  ::splay_tree_remove (tree, (splay_tree_key) &addr);
 }
 
 
@@ -302,30 +324,27 @@ addrmap_node_set_value (splay_tree_node node, void *value)
 }
 
 
-static void
-addrmap_splay_tree_insert (struct addrmap_mutable *map,
-			   CORE_ADDR key, void *value)
+void
+addrmap_mutable::splay_tree_insert (CORE_ADDR key, void *value)
 {
-  splay_tree_insert (map->tree,
-		     allocate_key (map, key),
-		     (splay_tree_value) value);
+  ::splay_tree_insert (tree,
+		       allocate_key (key),
+		       (splay_tree_value) value);
 }
 
 
 /* Without changing the mapping of any address, ensure that there is a
    tree node at ADDR, even if it would represent a "transition" from
    one value to the same value.  */
-static void
-force_transition (struct addrmap_mutable *self, CORE_ADDR addr)
+void
+addrmap_mutable::force_transition (CORE_ADDR addr)
 {
-  splay_tree_node n
-    = addrmap_splay_tree_lookup (self, addr);
+  splay_tree_node n = splay_tree_lookup (addr);
 
   if (! n)
     {
-      n = addrmap_splay_tree_predecessor (self, addr);
-      addrmap_splay_tree_insert (self, addr,
-				 n ? addrmap_node_value (n) : NULL);
+      n = splay_tree_predecessor (addr);
+      splay_tree_insert (addr, n ? addrmap_node_value (n) : NULL);
     }
 }
 
@@ -350,14 +369,14 @@ addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
      - Second pass: remove any unnecessary transitions.  */
 
   /* Establish transitions at the start and end.  */
-  force_transition (this, start);
+  force_transition (start);
   if (end_inclusive < CORE_ADDR_MAX)
-    force_transition (this, end_inclusive + 1);
+    force_transition (end_inclusive + 1);
 
   /* Walk the area, changing all NULL regions to OBJ.  */
-  for (n = addrmap_splay_tree_lookup (this, start), gdb_assert (n);
+  for (n = splay_tree_lookup (start), gdb_assert (n);
        n && addrmap_node_key (n) <= end_inclusive;
-       n = addrmap_splay_tree_successor (this, addrmap_node_key (n)))
+       n = splay_tree_successor (addrmap_node_key (n)))
     {
       if (! addrmap_node_value (n))
 	addrmap_node_set_value (n, obj);
@@ -366,16 +385,16 @@ addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
   /* Walk the area again, removing transitions from any value to
      itself.  Be sure to visit both the transitions we forced
      above.  */
-  n = addrmap_splay_tree_predecessor (this, start);
+  n = splay_tree_predecessor (start);
   prior_value = n ? addrmap_node_value (n) : NULL;
-  for (n = addrmap_splay_tree_lookup (this, start), gdb_assert (n);
+  for (n = splay_tree_lookup (start), gdb_assert (n);
        n && (end_inclusive == CORE_ADDR_MAX
 	     || addrmap_node_key (n) <= end_inclusive + 1);
        n = next)
     {
-      next = addrmap_splay_tree_successor (this, addrmap_node_key (n));
+      next = splay_tree_successor (addrmap_node_key (n));
       if (addrmap_node_value (n) == prior_value)
-	addrmap_splay_tree_remove (this, addrmap_node_key (n));
+	splay_tree_remove (addrmap_node_key (n));
       else
 	prior_value = addrmap_node_value (n);
     }
@@ -385,14 +404,14 @@ addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 void *
 addrmap_mutable::find (CORE_ADDR addr)
 {
-  splay_tree_node n = addrmap_splay_tree_lookup (this, addr);
+  splay_tree_node n = splay_tree_lookup (addr);
   if (n != nullptr)
     {
       gdb_assert (addrmap_node_key (n) == addr);
       return addrmap_node_value (n);
     }
 
-  n = addrmap_splay_tree_predecessor (this, addr);
+  n = splay_tree_predecessor (addr);
   if (n != nullptr)
     {
       gdb_assert (addrmap_node_key (n) < addr);
@@ -474,8 +493,8 @@ addrmap_mutable::foreach (addrmap_foreach_fn fn)
 }
 
 
-static void *
-splay_obstack_alloc (int size, void *closure)
+void *
+addrmap_mutable::splay_obstack_alloc (int size, void *closure)
 {
   struct addrmap_mutable *map = (struct addrmap_mutable *) closure;
   splay_tree_node n;
@@ -496,8 +515,8 @@ splay_obstack_alloc (int size, void *closure)
 }
 
 
-static void
-splay_obstack_free (void *obj, void *closure)
+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;
@@ -527,26 +546,22 @@ splay_compare_CORE_ADDR_ptr (splay_tree_key ak, splay_tree_key bk)
 }
 
 
-struct addrmap *
-addrmap_create_mutable (struct obstack *obstack)
+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))
 {
-  struct addrmap_mutable *map = new (obstack) struct addrmap_mutable;
-
-  map->obstack = obstack;
-
-  /* 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.  */
-  map->free_nodes = NULL;
+}
 
-  map->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,
-					     map);
 
-  return map;
+struct addrmap *
+addrmap_create_mutable (struct obstack *obstack)
+{
+  return new (obstack) struct addrmap_mutable (obstack);
 }
 
 /* See addrmap.h.  */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/9] Move addrmap classes to addrmap.h
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (2 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 3/9] Privacy for addrmap_mutable Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 5/9] Remove addrmap wrapper functions Tom Tromey
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This moves the addrmap class definitions to addrmap.h.  This is safe
to do now that the contents are private.
---
 gdb/addrmap.c | 114 ---------------------------------------------
 gdb/addrmap.h | 125 +++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 118 insertions(+), 121 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index e372e0c6e06..e956b00ac7c 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#include "splay-tree.h"
 #include "gdbsupport/gdb_obstack.h"
 #include "addrmap.h"
 #include "gdbsupport/selftest.h"
@@ -29,21 +28,6 @@ gdb_static_assert (sizeof (splay_tree_key) >= sizeof (CORE_ADDR *));
 gdb_static_assert (sizeof (splay_tree_value) >= sizeof (void *));
 
 \f
-/* The base class for addrmaps.  */
-
-struct addrmap : public allocate_on_obstack
-{
-  virtual ~addrmap () = default;
-
-  virtual void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
-			  void *obj) = 0;
-  virtual void *find (CORE_ADDR addr) = 0;
-  virtual struct addrmap *create_fixed (struct obstack *obstack) = 0;
-  virtual void relocate (CORE_ADDR offset) = 0;
-  virtual int foreach (addrmap_foreach_fn fn) = 0;
-};
-
-
 void
 addrmap_set_empty (struct addrmap *map,
 		   CORE_ADDR start, CORE_ADDR end_inclusive,
@@ -84,45 +68,6 @@ addrmap_foreach (struct addrmap *map, addrmap_foreach_fn fn)
 \f
 /* Fixed address maps.  */
 
-struct addrmap_mutable;
-
-struct addrmap_fixed : public addrmap
-{
-public:
-
-  addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut);
-  DISABLE_COPY_AND_ASSIGN (addrmap_fixed);
-
-  void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
-		  void *obj) override;
-  void *find (CORE_ADDR addr) override;
-  struct addrmap *create_fixed (struct obstack *obstack) override;
-  void relocate (CORE_ADDR offset) override;
-  int foreach (addrmap_foreach_fn fn) override;
-
-private:
-
-  /* A transition: a point in an address map where the value changes.
-     The map maps ADDR to VALUE, but if ADDR > 0, it maps ADDR-1 to
-     something else.  */
-  struct addrmap_transition
-  {
-    CORE_ADDR addr;
-    void *value;
-  };
-
-  /* The number of transitions in TRANSITIONS.  */
-  size_t num_transitions;
-
-  /* An array of transitions, sorted by address.  For every point in
-     the map where either ADDR == 0 or ADDR is mapped to one value and
-     ADDR - 1 is mapped to something different, we have an entry here
-     containing ADDR and VALUE.  (Note that this means we always have
-     an entry for address 0).  */
-  struct addrmap_transition *transitions;
-};
-
-
 void
 addrmap_fixed::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 			  void *obj)
@@ -204,65 +149,6 @@ addrmap_fixed::foreach (addrmap_foreach_fn fn)
 \f
 /* Mutable address maps.  */
 
-struct addrmap_mutable : public addrmap
-{
-public:
-
-  explicit addrmap_mutable (struct obstack *obs);
-  DISABLE_COPY_AND_ASSIGN (addrmap_mutable);
-
-  void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
-		  void *obj) override;
-  void *find (CORE_ADDR addr) override;
-  struct addrmap *create_fixed (struct obstack *obstack) override;
-  void relocate (CORE_ADDR offset) override;
-  int foreach (addrmap_foreach_fn fn) override;
-
-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.
-
-     Any addresses below the first node map to NULL.  (Unlike
-     fixed maps, we have no entry at (CORE_ADDR) 0; it doesn't 
-     simplify enough.)
-
-     The last region is assumed to end at CORE_ADDR_MAX.
-
-     Since we can't know whether CORE_ADDR is larger or smaller than
-     splay_tree_key (unsigned long) --- I think both are possible,
-     given all combinations of 32- and 64-bit hosts and targets ---
-     our keys are pointers to CORE_ADDR values.  Since the splay tree
-     library doesn't pass any closure pointer to the key free
-     function, we can't keep a freelist for keys.  Since mutable
-     addrmaps are only used temporarily right now, we just leak keys
-     from deleted nodes; they'll be freed when the obstack is freed.  */
-  splay_tree tree;
-
-  /* Various helper methods.  */
-  splay_tree_key allocate_key (CORE_ADDR addr);
-  void force_transition (CORE_ADDR addr);
-  splay_tree_node splay_tree_lookup (CORE_ADDR addr);
-  splay_tree_node splay_tree_predecessor (CORE_ADDR addr);
-  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);
-};
-
-
 /* Allocate a copy of CORE_ADDR in the obstack.  */
 splay_tree_key
 addrmap_mutable::allocate_key (CORE_ADDR addr)
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index 54567fa0d9d..7837c8521a1 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -20,6 +20,7 @@
 #ifndef ADDRMAP_H
 #define ADDRMAP_H
 
+#include "splay-tree.h"
 #include "gdbsupport/function-view.h"
 
 /* An address map is essentially a table mapping CORE_ADDRs onto GDB
@@ -33,8 +34,123 @@
    A fixed address map, once constructed (from a mutable address map),
    can't be edited.  Both kinds of map are allocated in obstacks.  */
 
-/* The opaque type representing address maps.  */
-struct addrmap;
+/* The type of a function used to iterate over the map.
+   OBJ is NULL for unmapped regions.  */
+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
+{
+  virtual ~addrmap () = default;
+
+  virtual void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+			  void *obj) = 0;
+  virtual void *find (CORE_ADDR addr) = 0;
+  virtual struct addrmap *create_fixed (struct obstack *obstack) = 0;
+  virtual void relocate (CORE_ADDR offset) = 0;
+  virtual int foreach (addrmap_foreach_fn fn) = 0;
+};
+
+struct addrmap_mutable;
+
+/* Fixed address maps.  */
+struct addrmap_fixed : public addrmap
+{
+public:
+
+  addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut);
+  DISABLE_COPY_AND_ASSIGN (addrmap_fixed);
+
+  void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+		  void *obj) override;
+  void *find (CORE_ADDR addr) override;
+  struct addrmap *create_fixed (struct obstack *obstack) override;
+  void relocate (CORE_ADDR offset) override;
+  int foreach (addrmap_foreach_fn fn) override;
+
+private:
+
+  /* A transition: a point in an address map where the value changes.
+     The map maps ADDR to VALUE, but if ADDR > 0, it maps ADDR-1 to
+     something else.  */
+  struct addrmap_transition
+  {
+    CORE_ADDR addr;
+    void *value;
+  };
+
+  /* The number of transitions in TRANSITIONS.  */
+  size_t num_transitions;
+
+  /* An array of transitions, sorted by address.  For every point in
+     the map where either ADDR == 0 or ADDR is mapped to one value and
+     ADDR - 1 is mapped to something different, we have an entry here
+     containing ADDR and VALUE.  (Note that this means we always have
+     an entry for address 0).  */
+  struct addrmap_transition *transitions;
+};
+
+/* Mutable address maps.  */
+
+struct addrmap_mutable : public addrmap
+{
+public:
+
+  explicit addrmap_mutable (struct obstack *obs);
+  DISABLE_COPY_AND_ASSIGN (addrmap_mutable);
+
+  void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
+		  void *obj) override;
+  void *find (CORE_ADDR addr) override;
+  struct addrmap *create_fixed (struct obstack *obstack) override;
+  void relocate (CORE_ADDR offset) override;
+  int foreach (addrmap_foreach_fn fn) override;
+
+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.
+
+     Any addresses below the first node map to NULL.  (Unlike
+     fixed maps, we have no entry at (CORE_ADDR) 0; it doesn't 
+     simplify enough.)
+
+     The last region is assumed to end at CORE_ADDR_MAX.
+
+     Since we can't know whether CORE_ADDR is larger or smaller than
+     splay_tree_key (unsigned long) --- I think both are possible,
+     given all combinations of 32- and 64-bit hosts and targets ---
+     our keys are pointers to CORE_ADDR values.  Since the splay tree
+     library doesn't pass any closure pointer to the key free
+     function, we can't keep a freelist for keys.  Since mutable
+     addrmaps are only used temporarily right now, we just leak keys
+     from deleted nodes; they'll be freed when the obstack is freed.  */
+  splay_tree tree;
+
+  /* Various helper methods.  */
+  splay_tree_key allocate_key (CORE_ADDR addr);
+  void force_transition (CORE_ADDR addr);
+  splay_tree_node splay_tree_lookup (CORE_ADDR addr);
+  splay_tree_node splay_tree_predecessor (CORE_ADDR addr);
+  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);
+};
+
 
 /* Create a mutable address map which maps every address to NULL.
    Allocate entries in OBSTACK.  */
@@ -93,11 +209,6 @@ struct addrmap *addrmap_create_fixed (struct addrmap *original,
    to either mutable or immutable maps.)  */
 void addrmap_relocate (struct addrmap *map, CORE_ADDR offset);
 
-/* The type of a function used to iterate over the map.
-   OBJ is NULL for unmapped regions.  */
-typedef gdb::function_view<int (CORE_ADDR start_addr, void *obj)>
-     addrmap_foreach_fn;
-
 /* Call FN for every address in MAP, following an in-order traversal.
    If FN ever returns a non-zero value, the iteration ceases
    immediately, and the value is returned.  Otherwise, this function
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/9] Remove addrmap wrapper functions
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (3 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 4/9] Move addrmap classes to addrmap.h Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 6/9] Remove addrmap_create_mutable Tom Tromey
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes the various addrmap wrapper functions in favor of simple
method calls on the objects themselves.
---
 gdb/addrmap.c             |  60 ++++-----------------
 gdb/addrmap.h             | 109 +++++++++++++++++---------------------
 gdb/block.c               |   2 +-
 gdb/buildsym.c            |   4 +-
 gdb/dwarf2/cooked-index.h |   4 +-
 gdb/dwarf2/index-write.c  |   2 +-
 gdb/dwarf2/read.c         |  30 +++++------
 gdb/inline-frame.c        |   3 +-
 gdb/objfiles.c            |   2 +-
 gdb/psymtab.c             |   3 +-
 gdb/symtab.c              |   2 +-
 11 files changed, 82 insertions(+), 139 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index e956b00ac7c..5ce7213824e 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -27,44 +27,6 @@
 gdb_static_assert (sizeof (splay_tree_key) >= sizeof (CORE_ADDR *));
 gdb_static_assert (sizeof (splay_tree_value) >= sizeof (void *));
 
-\f
-void
-addrmap_set_empty (struct addrmap *map,
-		   CORE_ADDR start, CORE_ADDR end_inclusive,
-		   void *obj)
-{
-  map->set_empty (start, end_inclusive, obj);
-}
-
-
-void *
-addrmap_find (struct addrmap *map, CORE_ADDR addr)
-{
-  return map->find (addr);
-}
-
-
-struct addrmap *
-addrmap_create_fixed (struct addrmap *original, struct obstack *obstack)
-{
-  return original->create_fixed (obstack);
-}
-
-
-/* Relocate all the addresses in MAP by OFFSET.  (This can be applied
-   to either mutable or immutable maps.)  */
-void
-addrmap_relocate (struct addrmap *map, CORE_ADDR offset)
-{
-  map->relocate (offset);
-}
-
-
-int
-addrmap_foreach (struct addrmap *map, addrmap_foreach_fn fn)
-{
-  return map->foreach (fn);
-}
 \f
 /* Fixed address maps.  */
 
@@ -313,7 +275,7 @@ addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut)
   size_t transition_count = 0;
 
   /* Count the number of transitions in the tree.  */
-  addrmap_foreach (mut, [&] (CORE_ADDR start, void *obj)
+  mut->foreach ([&] (CORE_ADDR start, void *obj)
     {
       ++transition_count;
       return 0;
@@ -331,7 +293,7 @@ addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut)
 
   /* Copy all entries from the splay tree to the array, in order 
      of increasing address.  */
-  addrmap_foreach (mut, [&] (CORE_ADDR start, void *obj)
+  mut->foreach ([&] (CORE_ADDR start, void *obj)
     {
       transitions[num_transitions].addr = start;
       transitions[num_transitions].value = obj;
@@ -482,7 +444,7 @@ addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload)
     return 0;
   };
 
-  addrmap_foreach (map, callback);
+  map->foreach (callback);
 }
 
 #if GDB_SELF_TEST
@@ -502,7 +464,7 @@ core_addr (void *p)
   do									\
     {									\
       for (unsigned i = LOW; i <= HIGH; ++i)				\
-	SELF_CHECK (addrmap_find (MAP, core_addr (&ARRAY[i])) == VAL);	\
+	SELF_CHECK (MAP->find (core_addr (&ARRAY[i])) == VAL);		\
     }									\
   while (0)
 
@@ -528,14 +490,13 @@ test_addrmap ()
   CHECK_ADDRMAP_FIND (map, array, 0, 19, nullptr);
 
   /* Insert address range into mutable addrmap.  */
-  addrmap_set_empty (map, core_addr (&array[10]), core_addr (&array[12]),
-		     val1);
+  map->set_empty (core_addr (&array[10]), core_addr (&array[12]), val1);
   CHECK_ADDRMAP_FIND (map, array, 0, 9, nullptr);
   CHECK_ADDRMAP_FIND (map, array, 10, 12, val1);
   CHECK_ADDRMAP_FIND (map, array, 13, 19, nullptr);
 
   /* Create corresponding fixed addrmap.  */
-  struct addrmap *map2 = addrmap_create_fixed (map, &temp_obstack);
+  struct addrmap *map2 = map->create_fixed (&temp_obstack);
   SELF_CHECK (map2 != nullptr);
   CHECK_ADDRMAP_FIND (map2, array, 0, 9, nullptr);
   CHECK_ADDRMAP_FIND (map2, array, 10, 12, val1);
@@ -554,18 +515,17 @@ test_addrmap ()
 	SELF_CHECK (false);
       return 0;
     };
-  SELF_CHECK (addrmap_foreach (map, callback) == 0);
-  SELF_CHECK (addrmap_foreach (map2, callback) == 0);
+  SELF_CHECK (map->foreach (callback) == 0);
+  SELF_CHECK (map2->foreach (callback) == 0);
 
   /* Relocate fixed addrmap.  */
-  addrmap_relocate (map2, 1);
+  map2->relocate (1);
   CHECK_ADDRMAP_FIND (map2, array, 0, 10, nullptr);
   CHECK_ADDRMAP_FIND (map2, array, 11, 13, val1);
   CHECK_ADDRMAP_FIND (map2, array, 14, 19, nullptr);
 
   /* Insert partially overlapping address range into mutable addrmap.  */
-  addrmap_set_empty (map, core_addr (&array[11]), core_addr (&array[13]),
-		     val2);
+  map->set_empty (core_addr (&array[11]), core_addr (&array[13]), val2);
   CHECK_ADDRMAP_FIND (map, array, 0, 9, nullptr);
   CHECK_ADDRMAP_FIND (map, array, 10, 12, val1);
   CHECK_ADDRMAP_FIND (map, array, 13, 13, val2);
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index 7837c8521a1..948e9b07eaa 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -44,11 +44,61 @@ struct addrmap : public allocate_on_obstack
 {
   virtual ~addrmap () = default;
 
+  /* In the mutable address map MAP, associate the addresses from START
+     to END_INCLUSIVE that are currently associated with NULL with OBJ
+     instead.  Addresses mapped to an object other than NULL are left
+     unchanged.
+
+     As the name suggests, END_INCLUSIVE is also mapped to OBJ.  This
+     convention is unusual, but it allows callers to accurately specify
+     ranges that abut the top of the address space, and ranges that
+     cover the entire address space.
+
+     This operation seems a bit complicated for a primitive: if it's
+     needed, why not just have a simpler primitive operation that sets a
+     range to a value, wiping out whatever was there before, and then
+     let the caller construct more complicated operations from that,
+     along with some others for traversal?
+
+     It turns out this is the mutation operation we want to use all the
+     time, at least for now.  Our immediate use for address maps is to
+     represent lexical blocks whose address ranges are not contiguous.
+     We walk the tree of lexical blocks present in the debug info, and
+     only create 'struct block' objects after we've traversed all a
+     block's children.  If a lexical block declares no local variables
+     (and isn't the lexical block for a function's body), we omit it
+     from GDB's data structures entirely.
+
+     However, this means that we don't decide to create a block (and
+     thus record it in the address map) until after we've traversed its
+     children.  If we do decide to create the block, we do so at a time
+     when all its children have already been recorded in the map.  So
+     this operation --- change only those addresses left unset --- is
+     actually the operation we want to use every time.
+
+     It seems simpler to let the code which operates on the
+     representation directly deal with the hair of implementing these
+     semantics than to provide an interface which allows it to be
+     implemented efficiently, but doesn't reveal too much of the
+     representation.  */
   virtual void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 			  void *obj) = 0;
+
+  /* Return the object associated with ADDR in MAP.  */
   virtual void *find (CORE_ADDR addr) = 0;
+
+  /* Create a fixed address map which is a copy of this mutable
+     address map.  Allocate entries in OBSTACK.  */
   virtual struct addrmap *create_fixed (struct obstack *obstack) = 0;
+
+  /* Relocate all the addresses in MAP by OFFSET.  (This can be applied
+     to either mutable or immutable maps.)  */
   virtual void relocate (CORE_ADDR offset) = 0;
+
+  /* Call FN for every address in MAP, following an in-order traversal.
+     If FN ever returns a non-zero value, the iteration ceases
+     immediately, and the value is returned.  Otherwise, this function
+     returns 0.  */
   virtual int foreach (addrmap_foreach_fn fn) = 0;
 };
 
@@ -156,65 +206,6 @@ struct addrmap_mutable : public addrmap
    Allocate entries in OBSTACK.  */
 struct addrmap *addrmap_create_mutable (struct obstack *obstack);
 
-/* In the mutable address map MAP, associate the addresses from START
-   to END_INCLUSIVE that are currently associated with NULL with OBJ
-   instead.  Addresses mapped to an object other than NULL are left
-   unchanged.
-
-   As the name suggests, END_INCLUSIVE is also mapped to OBJ.  This
-   convention is unusual, but it allows callers to accurately specify
-   ranges that abut the top of the address space, and ranges that
-   cover the entire address space.
-
-   This operation seems a bit complicated for a primitive: if it's
-   needed, why not just have a simpler primitive operation that sets a
-   range to a value, wiping out whatever was there before, and then
-   let the caller construct more complicated operations from that,
-   along with some others for traversal?
-
-   It turns out this is the mutation operation we want to use all the
-   time, at least for now.  Our immediate use for address maps is to
-   represent lexical blocks whose address ranges are not contiguous.
-   We walk the tree of lexical blocks present in the debug info, and
-   only create 'struct block' objects after we've traversed all a
-   block's children.  If a lexical block declares no local variables
-   (and isn't the lexical block for a function's body), we omit it
-   from GDB's data structures entirely.
-
-   However, this means that we don't decide to create a block (and
-   thus record it in the address map) until after we've traversed its
-   children.  If we do decide to create the block, we do so at a time
-   when all its children have already been recorded in the map.  So
-   this operation --- change only those addresses left unset --- is
-   actually the operation we want to use every time.
-
-   It seems simpler to let the code which operates on the
-   representation directly deal with the hair of implementing these
-   semantics than to provide an interface which allows it to be
-   implemented efficiently, but doesn't reveal too much of the
-   representation.  */
-void addrmap_set_empty (struct addrmap *map,
-			CORE_ADDR start, CORE_ADDR end_inclusive,
-			void *obj);
-
-/* Return the object associated with ADDR in MAP.  */
-void *addrmap_find (struct addrmap *map, CORE_ADDR addr);
-
-/* Create a fixed address map which is a copy of the mutable address
-   map ORIGINAL.  Allocate entries in OBSTACK.  */
-struct addrmap *addrmap_create_fixed (struct addrmap *original,
-				      struct obstack *obstack);
-
-/* Relocate all the addresses in MAP by OFFSET.  (This can be applied
-   to either mutable or immutable maps.)  */
-void addrmap_relocate (struct addrmap *map, CORE_ADDR offset);
-
-/* Call FN for every address in MAP, following an in-order traversal.
-   If FN ever returns a non-zero value, the iteration ceases
-   immediately, and the value is returned.  Otherwise, this function
-   returns 0.  */
-int addrmap_foreach (struct addrmap *map, addrmap_foreach_fn fn);
-
 /* Dump the addrmap to OUTFILE.  If PAYLOAD is non-NULL, only dump any
    components that map to PAYLOAD.  (If PAYLOAD is NULL, the entire
    map is dumped.)  */
diff --git a/gdb/block.c b/gdb/block.c
index 3fe096db583..cd3a2c9619a 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -139,7 +139,7 @@ find_block_in_blockvector (const struct blockvector *bl, CORE_ADDR pc)
   /* If we have an addrmap mapping code addresses to blocks, then use
      that.  */
   if (BLOCKVECTOR_MAP (bl))
-    return (const struct block *) addrmap_find (BLOCKVECTOR_MAP (bl), pc);
+    return (const struct block *) BLOCKVECTOR_MAP (bl)->find (pc);
 
   /* Otherwise, use binary search to find the last block that starts
      before PC.
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 628903d674f..91c2f363054 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -419,7 +419,7 @@ buildsym_compunit::record_block_range (struct block *block,
   if (m_pending_addrmap == nullptr)
     m_pending_addrmap = addrmap_create_mutable (&m_pending_addrmap_obstack);
 
-  addrmap_set_empty (m_pending_addrmap, start, end_inclusive, block);
+  m_pending_addrmap->set_empty (start, end_inclusive, block);
 }
 
 struct blockvector *
@@ -458,7 +458,7 @@ buildsym_compunit::make_blockvector ()
      blockvector.  */
   if (m_pending_addrmap != nullptr && m_pending_addrmap_interesting)
     BLOCKVECTOR_MAP (blockvector)
-      = addrmap_create_fixed (m_pending_addrmap, &m_objfile->objfile_obstack);
+      = m_pending_addrmap->create_fixed (&m_objfile->objfile_obstack);
   else
     BLOCKVECTOR_MAP (blockvector) = 0;
 
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 4b52eaf93d0..b4611d9e1f9 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -183,7 +183,7 @@ class cooked_index
   void install_addrmap (addrmap *map)
   {
     gdb_assert (m_addrmap == nullptr);
-    m_addrmap = addrmap_create_fixed (map, &m_storage);
+    m_addrmap = map->create_fixed (&m_storage);
   }
 
   friend class cooked_index_vector;
@@ -202,7 +202,7 @@ class cooked_index
      found.  */
   dwarf2_per_cu_data *lookup (CORE_ADDR addr)
   {
-    return (dwarf2_per_cu_data *) addrmap_find (m_addrmap, addr);
+    return (dwarf2_per_cu_data *) m_addrmap->find (addr);
   }
 
   /* Create a new cooked_index_entry and register it with this object.
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 58b0f0b98e3..2fd95d9951f 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -478,7 +478,7 @@ write_address_map (struct addrmap *addrmap, data_buf &addr_vec,
 {
   struct addrmap_index_data addrmap_index_data (addr_vec, cu_index_htab);
 
-  addrmap_foreach (addrmap, addrmap_index_data);
+  addrmap->foreach (addrmap_index_data);
 
   /* It's highly unlikely the last entry (end address = 0xff...ff)
      is valid, but we should still handle it.
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6dcd446e5f4..488aff7c3d6 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2331,12 +2331,10 @@ 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;
-      addrmap_set_empty (mutable_map, 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 = addrmap_create_fixed (mutable_map,
-						 &per_bfd->obstack);
+  per_bfd->index_addrmap = mutable_map->create_fixed (&per_bfd->obstack);
 }
 
 /* Read the address map data from DWARF-5 .debug_aranges, and use it
@@ -2505,7 +2503,7 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 		   - baseaddr);
 	  end = (gdbarch_adjust_dwarf2_addr (gdbarch, end + baseaddr)
 		 - baseaddr);
-	  addrmap_set_empty (mutable_map, start, end - 1, per_cu);
+	  mutable_map->set_empty (start, end - 1, per_cu);
 	}
 
       per_cu->addresses_seen = true;
@@ -2527,8 +2525,7 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
   addrmap *mutable_map = addrmap_create_mutable (&temp_obstack);
 
   if (read_addrmap_from_aranges (per_objfile, section, mutable_map))
-    per_bfd->index_addrmap = addrmap_create_fixed (mutable_map,
-						   &per_bfd->obstack);
+    per_bfd->index_addrmap = mutable_map->create_fixed (&per_bfd->obstack);
 }
 
 /* A helper function that reads the .gdb_index from BUFFER and fills
@@ -4292,8 +4289,7 @@ dwarf2_base_index_functions::find_pc_sect_compunit_symtab
 
   CORE_ADDR baseaddr = objfile->text_section_offset ();
   data = ((struct dwarf2_per_cu_data *)
-	  addrmap_find (per_objfile->per_bfd->index_addrmap,
-			pc - baseaddr));
+	  per_objfile->per_bfd->index_addrmap->find (pc - baseaddr));
   if (!data)
     return NULL;
 
@@ -12982,7 +12978,7 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
 	  highpc = (gdbarch_adjust_dwarf2_addr (gdbarch,
 						range_end + baseaddr)
 		    - baseaddr);
-	  addrmap_set_empty (map, lowpc, highpc - 1, datum);
+	  map->set_empty (lowpc, highpc - 1, datum);
 	}
 
       /* FIXME: This is recording everything as a low-high
@@ -17944,8 +17940,7 @@ cooked_indexer::check_bounds (cutu_reader *reader)
 	   - baseaddr - 1);
       /* Store the contiguous range if it is not empty; it can be
 	 empty for CUs with no code.  */
-      addrmap_set_empty (m_index_storage->get_addrmap (), low, high,
-			 cu->per_cu);
+      m_index_storage->get_addrmap ()->set_empty (low, high, cu->per_cu);
 
       cu->per_cu->addresses_seen = true;
     }
@@ -18194,8 +18189,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 *) addrmap_find (m_die_range_map,
-						       lookup);
+		= (cooked_index_entry *) m_die_range_map->find (lookup);
 	    }
 
 	  unsigned int bytes_read;
@@ -18235,8 +18229,8 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 	      CORE_ADDR hi
 		= (gdbarch_adjust_dwarf2_addr (gdbarch, *high_pc + baseaddr)
 		   - baseaddr);
-	      addrmap_set_empty (m_index_storage->get_addrmap (), lo, hi - 1,
-				 scanning_per_cu);
+	      m_index_storage->get_addrmap ()->set_empty (lo, hi - 1,
+							  scanning_per_cu);
 	    }
 	}
 
@@ -18312,7 +18306,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);
-      addrmap_set_empty (m_die_range_map, start, end, (void *) parent_entry);
+      m_die_range_map->set_empty (start, end, (void *) parent_entry);
     }
 
   return info_ptr;
@@ -18478,7 +18472,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 *) addrmap_find (m_die_range_map, 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);
     }
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index bcdf36cd067..b0a276dbe18 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -289,8 +289,7 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block)
   if (BLOCKVECTOR_MAP (bv) == NULL)
     return 0;
 
-  new_block = (const struct block *) addrmap_find (BLOCKVECTOR_MAP (bv),
-						   pc - 1);
+  new_block = (const struct block *) BLOCKVECTOR_MAP (bv)->find (pc - 1);
   if (new_block == NULL)
     return 1;
 
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 0c71e0bd6a9..e1f33b32ccb 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -668,7 +668,7 @@ objfile_relocate1 (struct objfile *objfile,
 	int block_line_section = cust->block_line_section ();
 
 	if (BLOCKVECTOR_MAP (bv))
-	  addrmap_relocate (BLOCKVECTOR_MAP (bv), delta[block_line_section]);
+	  BLOCKVECTOR_MAP (bv)->relocate (delta[block_line_section]);
 
 	for (int i = 0; i < BLOCKVECTOR_NBLOCKS (bv); ++i)
 	  {
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 5d9949bca1d..8b36a18a0ff 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -193,8 +193,7 @@ psymbol_functions::find_pc_sect_psymtab (struct objfile *objfile,
 
       struct partial_symtab *pst
 	= ((struct partial_symtab *)
-	   addrmap_find (m_partial_symtabs->psymtabs_addrmap,
-			 pc - baseaddr));
+	   m_partial_symtabs->psymtabs_addrmap->find (pc - baseaddr));
       if (pst != NULL)
 	{
 	  /* FIXME: addrmaps currently do not handle overlayed sections,
diff --git a/gdb/symtab.c b/gdb/symtab.c
index a75492603b8..20cc40b7dd1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2990,7 +2990,7 @@ find_pc_sect_compunit_symtab (CORE_ADDR pc, struct obj_section *section)
 
 	  if (BLOCKVECTOR_MAP (bv))
 	    {
-	      if (addrmap_find (BLOCKVECTOR_MAP (bv), pc) == nullptr)
+	      if (BLOCKVECTOR_MAP (bv)->find (pc) == nullptr)
 		continue;
 
 	      return cust;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6/9] Remove addrmap_create_mutable
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (4 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 5/9] Remove addrmap wrapper functions Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 7/9] Remove addrmap::create_fixed Tom Tromey
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes addrmap_create_mutable in favor of using 'new' at the
spots where the addrmap is created.
---
 gdb/addrmap.c     |  9 ++-------
 gdb/addrmap.h     |  4 ----
 gdb/buildsym.c    |  4 +++-
 gdb/buildsym.h    |  2 +-
 gdb/dwarf2/read.c | 15 ++++++++-------
 5 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 5ce7213824e..f2c93f886ff 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -406,12 +406,6 @@ addrmap_mutable::addrmap_mutable (struct obstack *obs)
 }
 
 
-struct addrmap *
-addrmap_create_mutable (struct obstack *obstack)
-{
-  return new (obstack) struct addrmap_mutable (obstack);
-}
-
 /* See addrmap.h.  */
 
 void
@@ -483,7 +477,8 @@ test_addrmap ()
   /* Create mutable addrmap.  */
   struct obstack temp_obstack;
   obstack_init (&temp_obstack);
-  struct addrmap *map = addrmap_create_mutable (&temp_obstack);
+  struct addrmap_mutable *map
+    = new (&temp_obstack) addrmap_mutable (&temp_obstack);
   SELF_CHECK (map != nullptr);
 
   /* Check initial state.  */
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index 948e9b07eaa..99d38d411b6 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -202,10 +202,6 @@ struct addrmap_mutable : public addrmap
 };
 
 
-/* Create a mutable address map which maps every address to NULL.
-   Allocate entries in OBSTACK.  */
-struct addrmap *addrmap_create_mutable (struct obstack *obstack);
-
 /* Dump the addrmap to OUTFILE.  If PAYLOAD is non-NULL, only dump any
    components that map to PAYLOAD.  (If PAYLOAD is NULL, the entire
    map is dumped.)  */
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 91c2f363054..c59b7c815fe 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -417,7 +417,9 @@ buildsym_compunit::record_block_range (struct block *block,
     m_pending_addrmap_interesting = true;
 
   if (m_pending_addrmap == nullptr)
-    m_pending_addrmap = addrmap_create_mutable (&m_pending_addrmap_obstack);
+    m_pending_addrmap
+      = (new (&m_pending_addrmap_obstack) addrmap_mutable
+	 (&m_pending_addrmap_obstack));
 
   m_pending_addrmap->set_empty (start, end_inclusive, block);
 }
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index ee75e6fd95d..c1cd5192a79 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -385,7 +385,7 @@ struct buildsym_compunit
   /* 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 *m_pending_addrmap = nullptr;
+  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
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 488aff7c3d6..dab4a2eacdc 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2293,12 +2293,12 @@ 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_map;
+  struct addrmap_mutable *mutable_map;
   CORE_ADDR baseaddr;
 
   auto_obstack temp_obstack;
 
-  mutable_map = addrmap_create_mutable (&temp_obstack);
+  mutable_map = new (&temp_obstack) addrmap_mutable (&temp_obstack);
 
   iter = index->address_table.data ();
   end = iter + index->address_table.size ();
@@ -2522,7 +2522,8 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
 
   auto_obstack temp_obstack;
-  addrmap *mutable_map = addrmap_create_mutable (&temp_obstack);
+  addrmap_mutable *mutable_map
+    = new (&temp_obstack) addrmap_mutable (&temp_obstack);
 
   if (read_addrmap_from_aranges (per_objfile, section, mutable_map))
     per_bfd->index_addrmap = mutable_map->create_fixed (&per_bfd->obstack);
@@ -6603,7 +6604,7 @@ class cooked_index_storage
 					xcalloc, xfree)),
       m_index (new cooked_index),
       m_addrmap_storage (),
-      m_addrmap (addrmap_create_mutable (&m_addrmap_storage))
+      m_addrmap (new (&m_addrmap_storage) addrmap_mutable (&m_addrmap_storage))
   {
   }
 
@@ -6658,7 +6659,7 @@ class cooked_index_storage
   }
 
   /* Return the mutable addrmap that is currently being created.  */
-  addrmap *get_addrmap ()
+  addrmap_mutable *get_addrmap ()
   {
     return m_addrmap;
   }
@@ -6690,7 +6691,7 @@ class cooked_index_storage
   /* Storage for the writeable addrmap.  */
   auto_obstack m_addrmap_storage;
   /* A writeable addrmap being constructed by this scanner.  */
-  addrmap *m_addrmap;
+  addrmap_mutable *m_addrmap;
 };
 
 /* An instance of this is created to index a CU.  */
@@ -6706,7 +6707,7 @@ class cooked_indexer
       m_per_cu (per_cu),
       m_language (language),
       m_obstack (),
-      m_die_range_map (addrmap_create_mutable (&m_obstack))
+      m_die_range_map (new (&m_obstack) addrmap_mutable (&m_obstack))
   {
   }
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 7/9] Remove addrmap::create_fixed
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (5 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 6/9] Remove addrmap_create_mutable Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 8/9] Use malloc for mutable addrmaps Tom Tromey
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

addrmap::create_fixed is just a simple wrapper for 'new', so remove it
in favor of uses of 'new'.
---
 gdb/addrmap.c             | 19 ++-----------------
 gdb/addrmap.h             |  6 ------
 gdb/buildsym.c            |  3 ++-
 gdb/dwarf2/cooked-index.h |  4 ++--
 gdb/dwarf2/read.c         |  7 +++++--
 5 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index f2c93f886ff..fe4675cf956 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -72,15 +72,6 @@ addrmap_fixed::find (CORE_ADDR addr)
 }
 
 
-struct addrmap *
-addrmap_fixed::create_fixed (struct obstack *obstack)
-{
-  internal_error (__FILE__, __LINE__,
-		  _("addrmap_create_fixed is not implemented yet "
-		    "for fixed addrmaps"));
-}
-
-
 void
 addrmap_fixed::relocate (CORE_ADDR offset)
 {
@@ -306,13 +297,6 @@ addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut)
 }
 
 
-struct addrmap *
-addrmap_mutable::create_fixed (struct obstack *obstack)
-{
-  return new (obstack) struct addrmap_fixed (obstack, this);
-}
-
-
 void
 addrmap_mutable::relocate (CORE_ADDR offset)
 {
@@ -491,7 +475,8 @@ test_addrmap ()
   CHECK_ADDRMAP_FIND (map, array, 13, 19, nullptr);
 
   /* Create corresponding fixed addrmap.  */
-  struct addrmap *map2 = map->create_fixed (&temp_obstack);
+  struct addrmap *map2
+    = new (&temp_obstack) addrmap_fixed (&temp_obstack, map);
   SELF_CHECK (map2 != nullptr);
   CHECK_ADDRMAP_FIND (map2, array, 0, 9, nullptr);
   CHECK_ADDRMAP_FIND (map2, array, 10, 12, val1);
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index 99d38d411b6..ce57093183f 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -87,10 +87,6 @@ struct addrmap : public allocate_on_obstack
   /* Return the object associated with ADDR in MAP.  */
   virtual void *find (CORE_ADDR addr) = 0;
 
-  /* Create a fixed address map which is a copy of this mutable
-     address map.  Allocate entries in OBSTACK.  */
-  virtual struct addrmap *create_fixed (struct obstack *obstack) = 0;
-
   /* Relocate all the addresses in MAP by OFFSET.  (This can be applied
      to either mutable or immutable maps.)  */
   virtual void relocate (CORE_ADDR offset) = 0;
@@ -115,7 +111,6 @@ struct addrmap_fixed : public addrmap
   void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 		  void *obj) override;
   void *find (CORE_ADDR addr) override;
-  struct addrmap *create_fixed (struct obstack *obstack) override;
   void relocate (CORE_ADDR offset) override;
   int foreach (addrmap_foreach_fn fn) override;
 
@@ -153,7 +148,6 @@ struct addrmap_mutable : public addrmap
   void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
 		  void *obj) override;
   void *find (CORE_ADDR addr) override;
-  struct addrmap *create_fixed (struct obstack *obstack) override;
   void relocate (CORE_ADDR offset) override;
   int foreach (addrmap_foreach_fn fn) override;
 
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index c59b7c815fe..1c6fd11c353 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -460,7 +460,8 @@ buildsym_compunit::make_blockvector ()
      blockvector.  */
   if (m_pending_addrmap != nullptr && m_pending_addrmap_interesting)
     BLOCKVECTOR_MAP (blockvector)
-      = m_pending_addrmap->create_fixed (&m_objfile->objfile_obstack);
+      = (new (&m_objfile->objfile_obstack) addrmap_fixed
+	 (&m_objfile->objfile_obstack, m_pending_addrmap));
   else
     BLOCKVECTOR_MAP (blockvector) = 0;
 
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index b4611d9e1f9..b339bdbc21f 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -180,10 +180,10 @@ class cooked_index
 				 dwarf2_per_cu_data *per_cu);
 
   /* Install a new fixed addrmap from the given mutable addrmap.  */
-  void install_addrmap (addrmap *map)
+  void install_addrmap (addrmap_mutable *map)
   {
     gdb_assert (m_addrmap == nullptr);
-    m_addrmap = map->create_fixed (&m_storage);
+    m_addrmap = new (&m_storage) addrmap_fixed (&m_storage, map);
   }
 
   friend class cooked_index_vector;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index dab4a2eacdc..a9bd901c5a8 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2334,7 +2334,8 @@ create_addrmap_from_index (dwarf2_per_objfile *per_objfile,
       mutable_map->set_empty (lo, hi - 1, per_bfd->get_cu (cu_index));
     }
 
-  per_bfd->index_addrmap = mutable_map->create_fixed (&per_bfd->obstack);
+  per_bfd->index_addrmap
+    = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, mutable_map);
 }
 
 /* Read the address map data from DWARF-5 .debug_aranges, and use it
@@ -2526,7 +2527,9 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
     = new (&temp_obstack) addrmap_mutable (&temp_obstack);
 
   if (read_addrmap_from_aranges (per_objfile, section, mutable_map))
-    per_bfd->index_addrmap = mutable_map->create_fixed (&per_bfd->obstack);
+    per_bfd->index_addrmap
+      = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack,
+					       mutable_map);
 }
 
 /* A helper function that reads the .gdb_index from BUFFER and fills
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 8/9] Use malloc for mutable addrmaps
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (6 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 7/9] Remove addrmap::create_fixed Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-04-16 16:58 ` [PATCH 9/9] Remove psymtab_addrmap Tom Tromey
  2022-06-12 16:48 ` [PATCH 0/9] C++-ify addrmap Tom Tromey
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

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 fe4675cf956..58ecc7152c2 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 ce57093183f..7f6b886f1d8 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 @@ struct addrmap_mutable : public addrmap
 
 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 @@ struct addrmap_mutable : public addrmap
   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 1c6fd11c353..21a1dcf1520 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -32,7 +32,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
@@ -416,12 +415,7 @@ buildsym_compunit::record_block_range (struct block *block,
       || end_inclusive + 1 != BLOCK_END (block))
     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_MAP (blockvector)
       = (new (&m_objfile->objfile_obstack) addrmap_fixed
-	 (&m_objfile->objfile_obstack, m_pending_addrmap));
+	 (&m_objfile->objfile_obstack, &m_pending_addrmap));
   else
     BLOCKVECTOR_MAP (blockvector) = 0;
 
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 @@ struct buildsym_compunit
   /* 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 a9bd901c5a8..e35772fee50 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2293,12 +2293,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 ();
@@ -2331,11 +2328,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
@@ -2522,14 +2519,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
@@ -6605,9 +6600,7 @@ class cooked_index_storage
 					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)
   {
   }
 
@@ -6657,14 +6650,14 @@ class cooked_index_storage
      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:
@@ -6691,10 +6684,8 @@ class cooked_index_storage
   /* 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.  */
@@ -6708,9 +6699,7 @@ class cooked_indexer
 		  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)
   {
   }
 
@@ -6795,12 +6784,10 @@ class cooked_indexer
   /* 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
@@ -18193,7 +18180,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;
@@ -18310,7 +18297,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;
@@ -18476,7 +18463,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.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 9/9] Remove psymtab_addrmap
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (7 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 8/9] Use malloc for mutable addrmaps Tom Tromey
@ 2022-04-16 16:58 ` Tom Tromey
  2022-06-12 16:48 ` [PATCH 0/9] C++-ify addrmap Tom Tromey
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-04-16 16:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

While working on addrmaps, I noticed that psymtab_addrmap is no longer
needed now.  It was introduced in ancient times as an optimization for
DWARF, but no other symbol reader was ever updated to use it.  Now
that DWARF does not use psymtabs, it can be deleted.
---
 gdb/dwarf2/read.c |   4 +-
 gdb/psympriv.h    |  16 +++----
 gdb/psymtab.c     | 108 +---------------------------------------------
 gdb/psymtab.h     |  11 -----
 4 files changed, 8 insertions(+), 131 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e35772fee50..02dd756a78e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2283,7 +2283,7 @@ create_signatured_type_table_from_debug_names
 }
 
 /* Read the address map data from the mapped index, and use it to
-   populate the psymtabs_addrmap.  */
+   populate the index_addrmap.  */
 
 static void
 create_addrmap_from_index (dwarf2_per_objfile *per_objfile,
@@ -2511,7 +2511,7 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 }
 
 /* Read the address map data from DWARF-5 .debug_aranges, and use it to
-   populate the psymtabs_addrmap.  */
+   populate the index_addrmap.  */
 
 static void
 create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index 9964c102403..cdcc1ba7fd0 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -277,11 +277,11 @@ struct partial_symtab
   const char *dirname = nullptr;
 
   /* Range of text addresses covered by this file; texthigh is the
-     beginning of the next section.  Do not use if PSYMTABS_ADDRMAP_SUPPORTED
-     is set.  Do not refer directly to these fields.  Instead, use the
-     accessors.  The validity of these fields is determined by the
-     text_low_valid and text_high_valid fields; these are located later
-     in this structure for better packing.  */
+     beginning of the next section.  Do not refer directly to these
+     fields.  Instead, use the accessors.  The validity of these
+     fields is determined by the text_low_valid and text_high_valid
+     fields; these are located later in this structure for better
+     packing.  */
 
   CORE_ADDR m_text_low = 0;
   CORE_ADDR m_text_high = 0;
@@ -343,12 +343,6 @@ struct partial_symtab
 
   std::vector<partial_symbol *> static_psymbols;
 
-  /* True iff objfile->psymtabs_addrmap is properly populated for this
-     partial_symtab.  For discontiguous overlapping psymtabs is the only usable
-     info in PSYMTABS_ADDRMAP.  */
-
-  bool psymtabs_addrmap_supported = false;
-
   /* True if the name of this partial symtab is not a source file name.  */
 
   bool anonymous = false;
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 8b36a18a0ff..4ab1f0ec395 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -101,8 +101,6 @@ find_pc_sect_psymtab_closer (struct objfile *objfile,
   struct partial_symtab *best_pst = pst;
   CORE_ADDR best_addr = pst->text_low (objfile);
 
-  gdb_assert (!pst->psymtabs_addrmap_supported);
-
   /* An objfile that has its functions reordered might have
      many partial symbol tables containing the PC, but
      we want the partial symbol table that contains the
@@ -174,70 +172,8 @@ psymbol_functions::find_pc_sect_psymtab (struct objfile *objfile,
 					 struct obj_section *section,
 					 struct bound_minimal_symbol msymbol)
 {
-  /* Try just the PSYMTABS_ADDRMAP mapping first as it has better
-     granularity than the later used TEXTLOW/TEXTHIGH one.  However, we need
-     to take care as the PSYMTABS_ADDRMAP can hold things other than partial
-     symtabs in some cases.
-
-     This function should only be called for objfiles that are using partial
-     symtabs, not for objfiles that are using indexes (.gdb_index or
-     .debug_names), however 'maintenance print psymbols' calls this function
-     directly for all objfiles.  If we assume that PSYMTABS_ADDRMAP contains
-     partial symtabs then we will end up returning a pointer to an object
-     that is not a partial_symtab, which doesn't end well.  */
-
-  if (m_partial_symtabs->psymtabs != NULL
-      && m_partial_symtabs->psymtabs_addrmap != NULL)
-    {
-      CORE_ADDR baseaddr = objfile->text_section_offset ();
-
-      struct partial_symtab *pst
-	= ((struct partial_symtab *)
-	   m_partial_symtabs->psymtabs_addrmap->find (pc - baseaddr));
-      if (pst != NULL)
-	{
-	  /* FIXME: addrmaps currently do not handle overlayed sections,
-	     so fall back to the non-addrmap case if we're debugging
-	     overlays and the addrmap returned the wrong section.  */
-	  if (overlay_debugging && msymbol.minsym != NULL && section != NULL)
-	    {
-	      struct partial_symbol *p;
-
-	      /* NOTE: This assumes that every psymbol has a
-		 corresponding msymbol, which is not necessarily
-		 true; the debug info might be much richer than the
-		 object's symbol table.  */
-	      p = find_pc_sect_psymbol (objfile, pst, pc, section);
-	      if (p == NULL
-		  || (p->address (objfile)
-		      != msymbol.value_address ()))
-		goto next;
-	    }
-
-	  /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
-	     PSYMTABS_ADDRMAP we used has already the best 1-byte
-	     granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
-	     a worse chosen section due to the TEXTLOW/TEXTHIGH ranges
-	     overlap.  */
-
-	  return pst;
-	}
-    }
-
- next:
-
-  /* Existing PSYMTABS_ADDRMAP mapping is present even for PARTIAL_SYMTABs
-     which still have no corresponding full SYMTABs read.  But it is not
-     present for non-DWARF2 debug infos not supporting PSYMTABS_ADDRMAP in GDB
-     so far.  */
-
-  /* Check even OBJFILE with non-zero PSYMTABS_ADDRMAP as only several of
-     its CUs may be missing in PSYMTABS_ADDRMAP as they may be varying
-     debug info type in single OBJFILE.  */
-
   for (partial_symtab *pst : require_partial_symbols (objfile))
-    if (!pst->psymtabs_addrmap_supported
-	&& pc >= pst->text_low (objfile) && pc < pst->text_high (objfile))
+    if (pc >= pst->text_low (objfile) && pc < pst->text_high (objfile))
       {
 	struct partial_symtab *best_pst;
 
@@ -783,8 +719,6 @@ dump_psymtab (struct objfile *objfile, struct partial_symtab *psymtab,
   gdb_printf (outfile, "-");
   gdb_puts (paddress (gdbarch, psymtab->text_high (objfile)), outfile);
   gdb_printf (outfile, "\n");
-  gdb_printf (outfile, "  Address map supported - %s.\n",
-	      psymtab->psymtabs_addrmap_supported ? "yes" : "no");
   gdb_printf (outfile, "  Depends on %d other partial symtabs.\n",
 	      psymtab->number_of_dependencies);
   for (i = 0; i < psymtab->number_of_dependencies; i++)
@@ -1453,27 +1387,6 @@ psymtab_storage::discard_psymtab (struct partial_symtab *pst)
 
 \f
 
-/* Helper function for maintenance_print_psymbols to print the addrmap
-   of PSYMTAB.  If PSYMTAB is NULL print the entire addrmap.  */
-
-static void
-dump_psymtab_addrmap (struct objfile *objfile,
-		      psymtab_storage *partial_symtabs,
-		      struct partial_symtab *psymtab,
-		      struct ui_file *outfile)
-{
-  if ((psymtab == NULL
-       || psymtab->psymtabs_addrmap_supported)
-      && partial_symtabs->psymtabs_addrmap != NULL)
-    {
-      if (psymtab == nullptr)
-	gdb_printf (outfile, _("Entire address map:\n"));
-      else
-	gdb_printf (outfile, _("Address map:\n"));
-      addrmap_dump (partial_symtabs->psymtabs_addrmap, outfile, psymtab);
-    }
-}
-
 static void
 maintenance_print_psymbols (const char *args, int from_tty)
 {
@@ -1567,9 +1480,6 @@ maintenance_print_psymbols (const char *args, int from_tty)
 	  if (psf == nullptr)
 	    continue;
 
-	  psymtab_storage *partial_symtabs
-	    = psf->get_partial_symtabs ().get ();
-
 	  if (address_arg != NULL)
 	    {
 	      struct bound_minimal_symbol msymbol;
@@ -1587,7 +1497,6 @@ maintenance_print_psymbols (const char *args, int from_tty)
 		      printed_objfile_header = 1;
 		    }
 		  dump_psymtab (objfile, ps, outfile);
-		  dump_psymtab_addrmap (objfile, partial_symtabs, ps, outfile);
 		  found = 1;
 		}
 	    }
@@ -1614,21 +1523,9 @@ maintenance_print_psymbols (const char *args, int from_tty)
 			  printed_objfile_header = 1;
 			}
 		      dump_psymtab (objfile, ps, outfile);
-		      dump_psymtab_addrmap (objfile, partial_symtabs, ps,
-					    outfile);
 		    }
 		}
 	    }
-
-	  /* If we're printing all the objfile's symbols dump the full addrmap.  */
-
-	  if (address_arg == NULL
-	      && source_arg == NULL
-	      && partial_symtabs->psymtabs_addrmap != NULL)
-	    {
-	      outfile->puts ("\n");
-	      dump_psymtab_addrmap (objfile, partial_symtabs, NULL, outfile);
-	    }
 	}
     }
 
@@ -1697,9 +1594,6 @@ maintenance_info_psymtabs (const char *regexp, int from_tty)
 		    gdb_puts (paddress (gdbarch,
 					psymtab->text_high (objfile)));
 		    gdb_printf ("\n");
-		    gdb_printf ("    psymtabs_addrmap_supported %s\n",
-				(psymtab->psymtabs_addrmap_supported
-				 ? "yes" : "no"));
 		    gdb_printf ("    globals ");
 		    if (!psymtab->global_psymbols.empty ())
 		      gdb_printf
diff --git a/gdb/psymtab.h b/gdb/psymtab.h
index 70eb3e246ba..68ef99edcb5 100644
--- a/gdb/psymtab.h
+++ b/gdb/psymtab.h
@@ -121,17 +121,6 @@ class psymtab_storage
 
   struct partial_symtab *psymtabs = nullptr;
 
-  /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
-     have a map per the whole process but ADDRMAP cannot selectively remove
-     its items during FREE_OBJFILE.  This mapping is already present even for
-     PARTIAL_SYMTABs which still have no corresponding full SYMTABs read.
-
-     The DWARF parser reuses this addrmap to store things other than
-     psymtabs in the cases where debug information is being read from, for
-     example, the .debug-names section.  */
-
-  struct addrmap *psymtabs_addrmap = nullptr;
-
   /* A byte cache where we can stash arbitrary "chunks" of bytes that
      will not change.  */
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/9] C++-ify addrmap
  2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
                   ` (8 preceding siblings ...)
  2022-04-16 16:58 ` [PATCH 9/9] Remove psymtab_addrmap Tom Tromey
@ 2022-06-12 16:48 ` Tom Tromey
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-06-12 16:48 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I did some C++-ification of addrmap for some experiments I'm doing,
Tom> and I thought it would be good to clean this up and submit it instead
Tom> of keeping it on some branch.

Tom> Regression tested on x86-64 Fedora 34.

I've rebased this and I'm checking it in now.

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-06-12 16:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-16 16:58 [PATCH 0/9] C++-ify addrmap Tom Tromey
2022-04-16 16:58 ` [PATCH 1/9] Use inheritance for addrmap Tom Tromey
2022-04-16 16:58 ` [PATCH 2/9] Privacy for addrmap_fixed Tom Tromey
2022-04-16 16:58 ` [PATCH 3/9] Privacy for addrmap_mutable Tom Tromey
2022-04-16 16:58 ` [PATCH 4/9] Move addrmap classes to addrmap.h Tom Tromey
2022-04-16 16:58 ` [PATCH 5/9] Remove addrmap wrapper functions Tom Tromey
2022-04-16 16:58 ` [PATCH 6/9] Remove addrmap_create_mutable Tom Tromey
2022-04-16 16:58 ` [PATCH 7/9] Remove addrmap::create_fixed Tom Tromey
2022-04-16 16:58 ` [PATCH 8/9] Use malloc for mutable addrmaps Tom Tromey
2022-04-16 16:58 ` [PATCH 9/9] Remove psymtab_addrmap Tom Tromey
2022-06-12 16:48 ` [PATCH 0/9] C++-ify addrmap Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).