[gcc r17-2969] statistics: Fix destruction ordering issues with mem_alloc_description

Lewis Hyatt via Gcc-cvs <[email protected]> Wed, 5 Aug 2026 01:45:59 +0000 (GMT)
Newsgroups gmane.comp.gcc.cvs
Message-ID <[email protected]>
https://gcc.gnu.org/g:8f60387387e6c08071abe1493db849c459492e3e

commit r17-2969-g8f60387387e6c08071abe1493db849c459492e3e
Author: Lewis Hyatt <[email protected]>
Date:   Sat Aug 1 09:52:39 2026 -0400

    statistics: Fix destruction ordering issues with mem_alloc_description
    
    The previous patch fixed the documented issue PR28734 with
    --enable-gather-detailed-mem-stats, but there remain a couple other issues
    not previously reported:
    
        a) Around 200 tests still fail with statistics gathering enabled,
           including:
               gfortran.dg/gomp/class-firstprivate-1.f90   -O  (internal compiler error: in check_complete_insertion, at hash-table.h:572) gfortran.sum
           as well as most GCOV and JIT tests.
    
        b) bootstrap fails when configured with:
               --enable-gather-detailed-mem-stats --with-build-config=bootstrap-lto
           with similar ICEs in lots of places during stage 2.
    
    These both have the same cause. The classes that implement the statistics
    gathering are file-local static variables that are referenced from the
    destructors of the data structures (hash_map, vec, ggc, etc) that they are
    tracking. Consequently, any static instance of such a data structure is
    subject to destruction order issues; if the mem_alloc_description is
    destroyed before the static object it is tracking, then problems will arise.
    
    It seems that the cleanest solution is to arrange that mem_alloc_description
    objects are never destroyed, given that they persist anyway for the duration
    of the process. It does not seem desirable to forbid static instances of all
    data structures, since these seem to be used in many places.
    
    gcc/ChangeLog:
    
            * mem-stats.h (mem_alloc_description::mem_alloc_description): Make
            private.
            (mem_alloc_description::~mem_alloc_description): Disallow
            destruction entirely.
            (mem_alloc_description::instance): New function.
            * alloc-pool.cc (dump_alloc_pool_statistics): Adapt to
            pool_allocator_usage being a function instead of a variable.
            * alloc-pool.h (pool_allocator_usage): New function.
            (TBlockAllocator>::initialize): Adapt to
            pool_allocator_usage being a function instead of a variable.
            (TBlockAllocator>::release): Likewise.
            (TBlockAllocator>::allocate): Likewise.
            (TBlockAllocator>::remove): Likewise.
            * bitmap.cc (bitmap_mem_desc): Change from a static variable to an
            inline function.
            (bitmap_register): Adapt to bitmap_mem_desc being a function instead
            of a variable.
            (register_overhead): Likewise.
            (release_overhead): Likewise.
            (bitmap_list_find_element): Likewise.
            (bitmap_tree_splay): Likewise.
            (bitmap_tree_find_element): Likewise.
            (dump_bitmap_statistics): Likewise.
            * bitmap.h (bitmap_mem_desc): Remove unneeded extern declaration.
            * ggc-common.cc (ggc_mem_desc): Change from a static variable to an
            inline function.
            (ggc_record_overhead): Adapt to ggc_mem_desc being a function
            instead of a variable.
            (ggc_free_overhead): Likewise
            (ggc_prune_overhead_list): Likewise.
            * vec.cc (vec_mem_desc): Change from a static variable to a
            function.
            (vec_prefix::register_overhead): Adapt to vec_mem_desc being a
            function instead of a variable.
            (vec_prefix::release_overhead): Likewise.
            (dump_vec_loc_statistics): Likewise.
            * hash-table.cc (hash_table_usage): Move to...
            * hash-table.h (hash_table_usage): ...here, and use new singleton
            interface.

Diff:
---
 gcc/alloc-pool.cc |  3 +--
 gcc/alloc-pool.h  | 14 +++++++++-----
 gcc/bitmap.cc     | 26 +++++++++++++++-----------
 gcc/bitmap.h      |  3 ---
 gcc/ggc-common.cc | 22 +++++++++++++---------
 gcc/hash-table.cc | 11 -----------
 gcc/hash-table.h  |  6 +++++-
 gcc/mem-stats.h   | 41 ++++++++++++++++++-----------------------
 gcc/vec.cc        | 22 +++++++++++++---------
 9 files changed, 74 insertions(+), 74 deletions(-)

diff --git a/gcc/alloc-pool.cc b/gcc/alloc-pool.cc
index 8f35fee68210..cb69b3262716 100644
--- a/gcc/alloc-pool.cc
+++ b/gcc/alloc-pool.cc
@@ -24,7 +24,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 
 ALLOC_POOL_ID_TYPE last_id;
-mem_alloc_description<pool_usage> pool_allocator_usage;
 bool after_memory_report = false;
 
 /* Output per-alloc_pool memory usage statistics.  */
@@ -34,5 +33,5 @@ dump_alloc_pool_statistics (void)
   if (! GATHER_STATISTICS)
     return;
 
-  pool_allocator_usage.dump (ALLOC_POOL_ORIGIN);
+  pool_allocator_usage ().dump (ALLOC_POOL_ORIGIN);
 }
diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
index f81c1794137b..3c801789edfe 100644
--- a/gcc/alloc-pool.h
+++ b/gcc/alloc-pool.h
@@ -100,7 +100,11 @@ public:
   const char *m_pool_name;
 };
 
-extern mem_alloc_description<pool_usage> pool_allocator_usage;
+inline auto &
+pool_allocator_usage ()
+{
+  return mem_alloc_description<pool_usage>::instance<ALLOC_POOL_ORIGIN> ();
+}
 
 #if 0
 /* If a pool with custom block size is needed, one might use the following
@@ -274,7 +278,7 @@ base_pool_allocator <TBlockAllocator>::initialize ()
 
   if (GATHER_STATISTICS)
     {
-      pool_usage *u = pool_allocator_usage.register_descriptor
+      pool_usage *u = pool_allocator_usage ().register_descriptor
 	(this, new mem_location (m_location));
 
       u->m_element_size = m_elt_size;
@@ -315,7 +319,7 @@ base_pool_allocator <TBlockAllocator>::release ()
 
   if (GATHER_STATISTICS && !after_memory_report)
     {
-      pool_allocator_usage.release_instance_overhead
+      pool_allocator_usage ().release_instance_overhead
 	(this, (m_elts_allocated - m_elts_free) * m_elt_size);
     }
 
@@ -357,7 +361,7 @@ base_pool_allocator <TBlockAllocator>::allocate ()
 
   if (GATHER_STATISTICS)
     {
-      pool_allocator_usage.register_instance_overhead (m_elt_size, this);
+      pool_allocator_usage ().register_instance_overhead (m_elt_size, this);
     }
 
 #ifdef ENABLE_VALGRIND_ANNOTATIONS
@@ -458,7 +462,7 @@ base_pool_allocator <TBlockAllocator>::remove (void *object)
 
   if (GATHER_STATISTICS)
     {
-      pool_allocator_usage.release_instance_overhead (this, m_elt_size);
+      pool_allocator_usage ().release_instance_overhead (this, m_elt_size);
     }
 }
 
diff --git a/gcc/bitmap.cc b/gcc/bitmap.cc
index 6bf0ee99d048..d896a5513945 100644
--- a/gcc/bitmap.cc
+++ b/gcc/bitmap.cc
@@ -40,7 +40,11 @@ using bitmap_splay_tree
   = splay_tree_without_parent<bitmap_splay_tree_accessors>;
 
 /* Memory allocation statistics purpose instance.  */
-mem_alloc_description<bitmap_usage> bitmap_mem_desc;
+inline auto &
+bitmap_mem_desc ()
+{
+  return mem_alloc_description<bitmap_usage>::instance<BITMAP_ORIGIN> ();
+}
 
 /* Static zero-initialized bitmap obstack used for default initialization
    of bitmap_head.  */
@@ -54,8 +58,8 @@ bitmap_register (bitmap b MEM_STAT_DECL)
   gcc_assert (b->alloc_descriptor == 0);
   b->alloc_descriptor = alloc_descriptor_max_uid++;
 
-  bitmap_mem_desc.register_descriptor (b->get_descriptor (), BITMAP_ORIGIN,
-				       false FINAL_PASS_MEM_STAT);
+  bitmap_mem_desc ().register_descriptor (b->get_descriptor (), BITMAP_ORIGIN,
+					  false FINAL_PASS_MEM_STAT);
 }
 
 /* Account the overhead.  */
@@ -63,8 +67,8 @@ static void
 register_overhead (bitmap b, size_t amount)
 {
   unsigned *d = b->get_descriptor ();
-  if (bitmap_mem_desc.contains_descriptor_for_instance (d))
-    bitmap_mem_desc.register_instance_overhead (amount, d);
+  if (bitmap_mem_desc ().contains_descriptor_for_instance (d))
+    bitmap_mem_desc ().register_instance_overhead (amount, d);
 }
 
 /* Release the overhead.  */
@@ -72,8 +76,8 @@ static void
 release_overhead (bitmap b, size_t amount, bool remove_from_map)
 {
   unsigned *d = b->get_descriptor ();
-  if (bitmap_mem_desc.contains_descriptor_for_instance (d))
-    bitmap_mem_desc.release_instance_overhead (d, amount, remove_from_map);
+  if (bitmap_mem_desc ().contains_descriptor_for_instance (d))
+    bitmap_mem_desc ().release_instance_overhead (d, amount, remove_from_map);
 }
 
 
@@ -374,7 +378,7 @@ bitmap_list_find_element (bitmap head, unsigned int indx)
      call initialize function.  */
   bitmap_usage *usage = NULL;
   if (GATHER_STATISTICS)
-    usage = bitmap_mem_desc.get_descriptor_for_instance (head);
+    usage = bitmap_mem_desc ().get_descriptor_for_instance (head);
 
   /* This bitmap has more than one element, and we're going to look
      through the elements list.  Count that as a search.  */
@@ -481,7 +485,7 @@ bitmap_tree_splay (bitmap head, bitmap_element *t, unsigned int indx)
 
   bitmap_usage *usage = NULL;
   if (GATHER_STATISTICS)
-    usage = bitmap_mem_desc.get_descriptor_for_instance (head);
+    usage = bitmap_mem_desc ().get_descriptor_for_instance (head);
 
   N.prev = N.next = NULL;
   l = r = &N;
@@ -587,7 +591,7 @@ bitmap_tree_find_element (bitmap head, unsigned int indx)
      call initialize function.  */
   bitmap_usage *usage = NULL;
   if (GATHER_STATISTICS)
-    usage = bitmap_mem_desc.get_descriptor_for_instance (head);
+    usage = bitmap_mem_desc ().get_descriptor_for_instance (head);
 
   /* This bitmap has more than one element, and we're going to look
      through the elements list.  Count that as a search.  */
@@ -2857,7 +2861,7 @@ dump_bitmap_statistics (void)
   if (!GATHER_STATISTICS)
     return;
 
-  bitmap_mem_desc.dump (BITMAP_ORIGIN);
+  bitmap_mem_desc ().dump (BITMAP_ORIGIN);
 }
 
 DEBUG_FUNCTION void
diff --git a/gcc/bitmap.h b/gcc/bitmap.h
index c6fd91ee90a0..88ff5cf020e7 100644
--- a/gcc/bitmap.h
+++ b/gcc/bitmap.h
@@ -272,9 +272,6 @@ public:
   uint64_t m_search_iter;
 };
 
-/* Bitmap memory description.  */
-extern mem_alloc_description<bitmap_usage> bitmap_mem_desc;
-
 /* Fundamental storage type for bitmap.  */
 
 typedef unsigned long BITMAP_WORD;
diff --git a/gcc/ggc-common.cc b/gcc/ggc-common.cc
index 6d3dca78f034..8752fad81b78 100644
--- a/gcc/ggc-common.cc
+++ b/gcc/ggc-common.cc
@@ -1269,7 +1269,11 @@ public:
 };
 
 /* GCC memory description.  */
-static mem_alloc_description<ggc_usage> ggc_mem_desc;
+inline auto &
+ggc_mem_desc ()
+{
+  return mem_alloc_description<ggc_usage>::instance<GGC_ORIGIN> ();
+}
 
 /* Dump per-site memory statistics.  */
 
@@ -1281,17 +1285,17 @@ dump_ggc_loc_statistics ()
 
   ggc_collect (GGC_COLLECT_FORCE);
 
-  ggc_mem_desc.dump (GGC_ORIGIN);
+  ggc_mem_desc ().dump (GGC_ORIGIN);
 }
 
 /* Record ALLOCATED and OVERHEAD bytes to descriptor NAME:LINE (FUNCTION).  */
 void
 ggc_record_overhead (size_t allocated, size_t overhead, void *ptr MEM_STAT_DECL)
 {
-  ggc_usage *usage = ggc_mem_desc.register_descriptor (ptr, GGC_ORIGIN, false
-						       FINAL_PASS_MEM_STAT);
+  ggc_usage *usage = ggc_mem_desc ().register_descriptor (ptr, GGC_ORIGIN, false
+							  FINAL_PASS_MEM_STAT);
 
-  ggc_mem_desc.register_object_overhead (usage, allocated + overhead, ptr);
+  ggc_mem_desc ().register_object_overhead (usage, allocated + overhead, ptr);
   usage->register_overhead (allocated, overhead);
 }
 
@@ -1299,7 +1303,7 @@ ggc_record_overhead (size_t allocated, size_t overhead, void *ptr MEM_STAT_DECL)
 void
 ggc_free_overhead (void *ptr)
 {
-  ggc_mem_desc.release_object_overhead (ptr);
+  ggc_mem_desc ().release_object_overhead (ptr);
 }
 
 /* After live values has been marked, walk all recorded pointers and see if
@@ -1309,13 +1313,13 @@ ggc_prune_overhead_list (void)
 {
   typedef hash_map<const void *, std::pair<ggc_usage *, size_t > > map_t;
 
-  map_t::iterator it = ggc_mem_desc.m_reverse_object_map->begin ();
+  map_t::iterator it = ggc_mem_desc ().m_reverse_object_map->begin ();
 
-  for (; it != ggc_mem_desc.m_reverse_object_map->end (); ++it)
+  for (; it != ggc_mem_desc ().m_reverse_object_map->end (); ++it)
     if (!ggc_marked_p ((*it).first))
       {
         (*it).second.first->m_collected += (*it).second.second;
-	ggc_mem_desc.m_reverse_object_map->remove ((*it).first);
+	ggc_mem_desc ().m_reverse_object_map->remove ((*it).first);
       }
 }
 
diff --git a/gcc/hash-table.cc b/gcc/hash-table.cc
index 22a08b2735bc..20650d1ac532 100644
--- a/gcc/hash-table.cc
+++ b/gcc/hash-table.cc
@@ -101,17 +101,6 @@ hash_table_higher_prime_index (unsigned long n)
   return low;
 }
 
-/* Return a reference to the lazily initialized hash-table usage description.
-   This needs to be a function rather than a simple global variable so that it
-   is reliably initialized before hash table variables in other files such as
-   sem_item::m_type_hash_cache.  */
-mem_alloc_description<mem_usage>&
-hash_table_usage ()
-{
-  static mem_alloc_description<mem_usage> usage;
-  return usage;
-}
-
 /* Support function for statistics.  */
 void dump_hash_table_loc_statistics (void)
 {
diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index 0bee2850b8cc..664c3e0ce458 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -636,7 +636,11 @@ private:
 #include "mem-stats.h"
 #include "hash-map.h"
 
-extern mem_alloc_description<mem_usage>& hash_table_usage (void);
+inline auto &
+hash_table_usage ()
+{
+  return mem_alloc_description<mem_usage>::instance<HASH_TABLE_ORIGIN> ();
+}
 
 /* Support function for statistics.  */
 extern void dump_hash_table_loc_statistics (void);
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index 2d4de15ccff9..dd83c9198773 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -276,7 +276,23 @@ public:
 template <class T>
 class mem_alloc_description
 {
+
+  /* Constructor is private to enforce singleton.  */
+  mem_alloc_description ();
+
+  /* Destruction is not allowed, since we might be tracking
+     static objects with undefined destruction order.  */
+  ~mem_alloc_description () = delete;
+
 public:
+
+  template<mem_alloc_origin>
+  static auto &instance ()
+  {
+    static const auto self = new mem_alloc_description;
+    return *self;
+  }
+
   struct mem_location_hash : nofree_ptr_hash <mem_location>
   {
     static hashval_t
@@ -306,11 +322,6 @@ public:
   typedef hash_map <const void *, std::pair<T *, size_t> > reverse_object_map_t;
   typedef std::pair <mem_location *, T *> mem_list_t;
 
-  /* Default constructor.  */
-  mem_alloc_description ();
-
-  /* Default destructor.  */
-  ~mem_alloc_description ();
 
   /* Returns true if instance PTR is registered by the memory description.  */
   bool contains_descriptor_for_instance (const void *ptr);
@@ -558,29 +569,13 @@ template <class T>
 inline
 mem_alloc_description<T>::mem_alloc_description ()
 {
+  /* Note it is important to pass false for the 4th argument (GATHER_MEM_STATS)
+     to avoid infinite recursion in instance ().  */
   m_map = new mem_map_t (13, false, false, false);
   m_reverse_map = new reverse_mem_map_t (13, false, false, false);
   m_reverse_object_map = new reverse_object_map_t (13, false, false, false);
 }
 
-/* Default destructor.  */
-
-template <class T>
-inline
-mem_alloc_description<T>::~mem_alloc_description ()
-{
-  for (typename mem_map_t::iterator it = m_map->begin (); it != m_map->end ();
-       ++it)
-    {
-      delete (*it).first;
-      delete (*it).second;
-    }
-
-  delete m_map;
-  delete m_reverse_map;
-  delete m_reverse_object_map;
-}
-
 /* Get all tracked instances registered by the description. Items are filtered
    by ORIGIN type, LENGTH is return value where we register the number of
    elements in the list. If we want to process custom order, CMP comparator
diff --git a/gcc/vec.cc b/gcc/vec.cc
index 0740e4958480..8313adde2b22 100644
--- a/gcc/vec.cc
+++ b/gcc/vec.cc
@@ -111,7 +111,11 @@ public:
 };
 
 /* Vector memory description.  */
-static mem_alloc_description <vec_usage> vec_mem_desc;
+inline auto &
+vec_mem_desc ()
+{
+  return mem_alloc_description<vec_usage>::instance<VEC_ORIGIN> ();
+}
 
 /* Account the overhead.  */
 
@@ -119,10 +123,10 @@ void
 vec_prefix::register_overhead (void *ptr, size_t elements,
 			       size_t element_size MEM_STAT_DECL)
 {
-  vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN, false
-				    FINAL_PASS_MEM_STAT);
+  vec_mem_desc ().register_descriptor (ptr, VEC_ORIGIN, false
+				       FINAL_PASS_MEM_STAT);
   vec_usage *usage
-    = vec_mem_desc.register_instance_overhead (elements * element_size, ptr);
+    = vec_mem_desc ().register_instance_overhead (elements * element_size, ptr);
   usage->m_element_size = element_size;
   usage->m_items += elements;
   if (usage->m_items_peak < usage->m_items)
@@ -135,11 +139,11 @@ void
 vec_prefix::release_overhead (void *ptr, size_t size, size_t elements,
 			      bool in_dtor MEM_STAT_DECL)
 {
-  if (!vec_mem_desc.contains_descriptor_for_instance (ptr))
-    vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN,
+  if (!vec_mem_desc ().contains_descriptor_for_instance (ptr))
+    vec_mem_desc ().register_descriptor (ptr, VEC_ORIGIN,
 				      false FINAL_PASS_MEM_STAT);
-  vec_usage *usage = vec_mem_desc.release_instance_overhead (ptr, size,
-							     in_dtor);
+  vec_usage *usage = vec_mem_desc ().release_instance_overhead (ptr, size,
+								in_dtor);
   usage->m_items -= elements;
 }
 
@@ -173,7 +177,7 @@ vec_prefix::calculate_allocation_1 (unsigned alloc, unsigned desired)
 void
 dump_vec_loc_statistics (void)
 {
-  vec_mem_desc.dump (VEC_ORIGIN);
+  vec_mem_desc ().dump (VEC_ORIGIN);
 }
 
 /* Gets the next token from STR delimited by DELIMS (deliminator not included