]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Use unordered_set for frame_stash
authorTom Tromey <tom@tromey.com>
Sun, 6 Jul 2025 14:38:55 +0000 (08:38 -0600)
committerTom Tromey <tom@tromey.com>
Mon, 10 Nov 2025 17:52:28 +0000 (10:52 -0700)
This changes the frame_stash global to be an unordered_set rather than
a htab_t.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
gdb/frame.c

index b84e30c23903883017931d30311f01ed47143ccc..6c646e5faff708cb4c6a190369009da4bdfec368 100644 (file)
@@ -213,82 +213,129 @@ get_frame_pc_masked (const frame_info_ptr &frame)
   return frame->next->prev_pc.masked;
 }
 
-/* A frame stash used to speed up frame lookups.  Create a hash table
-   to stash frames previously accessed from the frame cache for
-   quicker subsequent retrieval.  The hash table is emptied whenever
-   the frame cache is invalidated.  */
+/* Deletion function for the frame cache hash table.  */
+
+static void
+frame_info_del (frame_info *frame)
+{
+  if (frame->prologue_cache != nullptr)
+    frame->unwind->dealloc_cache (frame, frame->prologue_cache);
 
-static htab_t frame_stash;
+  if (frame->base_cache != nullptr)
+    frame->base->unwind->dealloc_cache (frame, frame->base_cache);
+}
 
-/* Internal function to calculate a hash from the frame_id addresses,
-   using as many valid addresses as possible.  Frames below level 0
-   are not stored in the hash table.  */
+/* A wrapper for frame_info that calls the 'frame_info_del' when
+   destroyed.  */
 
-static hashval_t
-frame_addr_hash (const void *ap)
+struct frame_info_stash_entry
 {
-  const frame_info *frame = (const frame_info *) ap;
-  const struct frame_id f_id = frame->this_id.value;
-  hashval_t hash = 0;
+  explicit frame_info_stash_entry (frame_info *info)
+    : frame (info)
+  {
+  }
 
-  gdb_assert (f_id.stack_status != FID_STACK_INVALID
-             || f_id.code_addr_p
-             || f_id.special_addr_p);
+  ~frame_info_stash_entry ()
+  {
+    destroy ();
+  }
 
-  if (f_id.stack_status == FID_STACK_VALID)
-    hash = iterative_hash (&f_id.stack_addr,
-                          sizeof (f_id.stack_addr), hash);
-  if (f_id.code_addr_p)
-    hash = iterative_hash (&f_id.code_addr,
-                          sizeof (f_id.code_addr), hash);
-  if (f_id.special_addr_p)
-    hash = iterative_hash (&f_id.special_addr,
-                          sizeof (f_id.special_addr), hash);
+  DISABLE_COPY_AND_ASSIGN (frame_info_stash_entry);
 
-  char user_created_p = f_id.user_created_p;
-  hash = iterative_hash (&user_created_p, sizeof (user_created_p), hash);
+  frame_info_stash_entry (frame_info_stash_entry &&other)
+    : frame (other.frame)
+  {
+    other.frame = nullptr;
+  }
 
-  return hash;
-}
+  frame_info_stash_entry &operator= (frame_info_stash_entry &&other)
+  {
+    destroy ();
+    frame = other.frame;
+    other.frame = nullptr;
+    return *this;
+  }
+
+  void destroy ()
+  {
+    if (frame != nullptr)
+      frame_info_del (frame);
+  }
 
-/* Internal equality function for the hash table.  This function
-   defers equality operations to frame_id::operator==.  */
+  frame_info *frame;
+};
 
-static int
-frame_addr_hash_eq (const void *a, const void *b)
+struct frame_info_stash_hash
 {
-  const frame_info *f_entry = (const frame_info *) a;
-  const frame_info *f_element = (const frame_info *) b;
+  using is_transparent = void;
+  using is_avalanching = void;
 
-  return f_entry->this_id.value == f_element->this_id.value;
-}
+  uint64_t operator() (const frame_id &f_id) const noexcept
+  {
+    hashval_t hash = 0;
 
-/* Deletion function for the frame cache hash table.  */
+    gdb_assert (f_id.stack_status != FID_STACK_INVALID
+               || f_id.code_addr_p
+               || f_id.special_addr_p);
 
-static void
-frame_info_del (frame_info *frame)
-{
-  if (frame->prologue_cache != nullptr)
-    frame->unwind->dealloc_cache (frame, frame->prologue_cache);
+    if (f_id.stack_status == FID_STACK_VALID)
+      hash = iterative_hash (&f_id.stack_addr,
+                            sizeof (f_id.stack_addr), hash);
+    if (f_id.code_addr_p)
+      hash = iterative_hash (&f_id.code_addr,
+                            sizeof (f_id.code_addr), hash);
+    if (f_id.special_addr_p)
+      hash = iterative_hash (&f_id.special_addr,
+                            sizeof (f_id.special_addr), hash);
 
-  if (frame->base_cache != nullptr)
-    frame->base->unwind->dealloc_cache (frame, frame->base_cache);
-}
+    char user_created_p = f_id.user_created_p;
+    hash = iterative_hash (&user_created_p, sizeof (user_created_p), hash);
 
-/* Internal function to create the frame_stash hash table.  100 seems
-   to be a good compromise to start the hash table at.  */
+    return hash;
+  }
 
-static void
-frame_stash_create (void)
+  uint64_t operator() (const frame_info_stash_entry &info) const noexcept
+  {
+    return (*this) (info.frame->this_id.value);
+  }
+
+  uint64_t operator() (const frame_info *frame) const noexcept
+  {
+    return (*this) (frame->this_id.value);
+  }
+};
+
+struct frame_info_stash_eq
 {
-  frame_stash = htab_create
-    (100, frame_addr_hash, frame_addr_hash_eq,
-     [] (void *p)
-       {
-        auto frame = static_cast<frame_info *> (p);
-        frame_info_del (frame);
-       });
-}
+  using is_transparent = void;
+
+  bool operator() (const frame_id &lhs, const frame_info_stash_entry &rhs)
+    const noexcept
+  {
+    return lhs == rhs.frame->this_id.value;
+  }
+
+  bool operator() (const frame_info *lhs, const frame_info_stash_entry &rhs)
+    const noexcept
+  {
+    return (*this) (lhs->this_id.value, rhs);
+  }
+
+  bool operator() (const frame_info_stash_entry &lhs,
+                  const frame_info_stash_entry &rhs)
+    const noexcept
+  {
+    return (*this) (lhs.frame->this_id.value, rhs);
+  }
+};
+
+/* A frame stash used to speed up frame lookups.  Create a hash table
+   to stash frames previously accessed from the frame cache for
+   quicker subsequent retrieval.  The hash table is emptied whenever
+   the frame cache is invalidated.  */
+
+static gdb::unordered_set<frame_info_stash_entry, frame_info_stash_hash,
+                         frame_info_stash_eq> frame_stash;
 
 /* Internal function to add a frame to the frame_stash hash table.
    Returns false if a frame with the same ID was already stashed, true
@@ -300,18 +347,7 @@ frame_stash_add (frame_info *frame)
   /* Valid frame levels are -1 (sentinel frames) and above.  */
   gdb_assert (frame->level >= -1);
 
-  frame_info **slot = (frame_info **) htab_find_slot (frame_stash,
-                                                     frame, INSERT);
-
-  /* If we already have a frame in the stack with the same id, we
-     either have a stack cycle (corrupted stack?), or some bug
-     elsewhere in GDB.  In any case, ignore the duplicate and return
-     an indication to the caller.  */
-  if (*slot != nullptr)
-    return false;
-
-  *slot = frame;
-  return true;
+  return frame_stash.emplace (frame).second;
 }
 
 /* Internal function to search the frame stash for an entry with the
@@ -321,12 +357,10 @@ frame_stash_add (frame_info *frame)
 static frame_info_ptr
 frame_stash_find (struct frame_id id)
 {
-  struct frame_info dummy;
-  frame_info *frame;
-
-  dummy.this_id.value = id;
-  frame = (frame_info *) htab_find (frame_stash, &dummy);
-  return frame_info_ptr (frame);
+  auto iter = frame_stash.find (id);
+  if (iter == frame_stash.end ())
+    return nullptr;
+  return frame_info_ptr (iter->frame);
 }
 
 /* Internal function to invalidate the frame stash by removing all
@@ -336,7 +370,7 @@ frame_stash_find (struct frame_id id)
 static void
 frame_stash_invalidate (void)
 {
-  htab_empty (frame_stash);
+  frame_stash.clear ();
 }
 
 /* See frame.h  */
@@ -2111,7 +2145,7 @@ reinit_frame_cache (void)
 {
   ++frame_cache_generation;
 
-  if (htab_elements (frame_stash) > 0)
+  if (!frame_stash.empty ())
     annotate_frames_invalid ();
 
   invalidate_selected_frame ();
@@ -3426,8 +3460,6 @@ INIT_GDB_FILE (frame)
 {
   obstack_init (&frame_cache_obstack);
 
-  frame_stash_create ();
-
   gdb::observers::target_changed.attach (frame_observer_target_changed,
                                         "frame");