On Sat, Aug 1, 2026 at 4:04 PM Lewis Hyatt <[email protected]> wrote:
>
> 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.
OK.
Thanks,
Richard.
> 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.
> ---
> 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 8f35fee6821..cb69b326271 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 f81c1794137..3c801789edf 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 6bf0ee99d04..d896a551394 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 c6fd91ee90a..88ff5cf020e 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 6d3dca78f03..8752fad81b7 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 22a08b2735b..20650d1ac53 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 0bee2850b8c..664c3e0ce45 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 2d4de15ccff..dd83c919877 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 0740e495848..8313adde2b2 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
