RFA: PATCH to fix crash with --enable-gather-detailed-mem-stats

Message ID CADzB+2k32FHaaDPBtOeyLjXJj0WQqvUuDOA1SuANJMejCNKEaA@mail.gmail.com
State New
Headers show
Series
  • RFA: PATCH to fix crash with --enable-gather-detailed-mem-stats
Related show

Commit Message

Jason Merrill Sept. 7, 2018, 10:38 a.m.
Turning on --enable-gather-detailed-mem-stats was producing for me a
compiler that crashed before main() because initializing the
hash_table sem_item::m_type_hash_cache was trying to add to
hash_table_usage before the latter had been initialized.  This patch
fixes this by changing hash_table_usage to be a function, so that it
is initialized on demand.

Tested x86_64-pc-linux-gnu.  OK for trunk?

Comments

Richard Biener Sept. 14, 2018, 7:08 a.m. | #1
On Fri, Sep 7, 2018 at 12:38 PM Jason Merrill <jason@redhat.com> wrote:
>

> Turning on --enable-gather-detailed-mem-stats was producing for me a

> compiler that crashed before main() because initializing the

> hash_table sem_item::m_type_hash_cache was trying to add to

> hash_table_usage before the latter had been initialized.  This patch

> fixes this by changing hash_table_usage to be a function, so that it

> is initialized on demand.

>

> Tested x86_64-pc-linux-gnu.  OK for trunk?


Ah, nice idea - IIRC in the past this forced us to use pointers for
all global hashes.  I wonder if we inline the function, esp. at places
like

   for (unsigned i = HASH_TABLE_ORIGIN; i <= HASH_SET_ORIGIN; i++)
     {
       mem_alloc_origin origin = (mem_alloc_origin) i;
-      hash_table_usage.dump (origin);
+      hash_table_usage ().dump (origin);
     }
?

OK.

Richard.

Patch

commit 8c58e31b5938f24f3730cda83d9556aad25324df
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Sep 4 17:12:26 2018 -0400

            Fix --enable-gather-detailed-mem-stats.
    
            * hash-table.c (hash_table_usage): Change from variable to function.
            * hash-table.h: Adjust.
            * Makefile.in: Add missing dependencies on hash-table.h.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index e008f63b2ea..4b7cec82382 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2665,7 +2665,7 @@  build/version.o:  version.c version.h \
 build/errors.o : errors.c $(BCONFIG_H) $(SYSTEM_H) errors.h
 build/gensupport.o: gensupport.c $(BCONFIG_H) $(SYSTEM_H) 		\
   $(CORETYPES_H) $(GTM_H) $(RTL_BASE_H) $(OBSTACK_H) errors.h		\
-  $(HASHTAB_H) $(READ_MD_H) $(GENSUPPORT_H)
+  $(HASHTAB_H) $(READ_MD_H) $(GENSUPPORT_H) $(HASH_TABLE_H)
 build/ggc-none.o : ggc-none.c $(BCONFIG_H) $(SYSTEM_H) $(CORETYPES_H) 	\
   $(GGC_H)
 build/min-insn-modes.o : min-insn-modes.c $(BCONFIG_H) $(SYSTEM_H)	\
@@ -2680,7 +2680,7 @@  build/read-rtl.o: read-rtl.c $(BCONFIG_H) $(SYSTEM_H) $(CORETYPES_H)	\
 build/rtl.o: rtl.c $(BCONFIG_H) $(CORETYPES_H) $(GTM_H) $(SYSTEM_H)	\
   $(RTL_H) $(GGC_H) errors.h
 build/vec.o : vec.c $(BCONFIG_H) $(SYSTEM_H) $(CORETYPES_H) $(VEC_H)	\
-  $(GGC_H) toplev.h $(DIAGNOSTIC_CORE_H)
+  $(GGC_H) toplev.h $(DIAGNOSTIC_CORE_H) $(HASH_TABLE_H)
 build/hash-table.o : hash-table.c $(BCONFIG_H) $(SYSTEM_H)		\
   $(CORETYPES_H) $(HASH_TABLE_H) $(GGC_H) toplev.h $(DIAGNOSTIC_CORE_H)
 build/sort.o : sort.cc $(BCONFIG_H) $(SYSTEM_H)
diff --git a/gcc/hash-table.c b/gcc/hash-table.c
index c86d84de217..bff9644ae81 100644
--- a/gcc/hash-table.c
+++ b/gcc/hash-table.c
@@ -98,7 +98,16 @@  hash_table_higher_prime_index (unsigned long n)
   return low;
 }
 
-mem_alloc_description<mem_usage> hash_table_usage;
+/* 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)
@@ -109,7 +118,6 @@  void dump_hash_table_loc_statistics (void)
   for (unsigned i = HASH_TABLE_ORIGIN; i <= HASH_SET_ORIGIN; i++)
     {
       mem_alloc_origin origin = (mem_alloc_origin) i;
-      hash_table_usage.dump (origin);
+      hash_table_usage ().dump (origin);
     }
 }
-
diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index 706b2370e23..bd83345c7b8 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -561,7 +561,7 @@  private:
 #include "mem-stats.h"
 #include "hash-map.h"
 
-extern mem_alloc_description<mem_usage> hash_table_usage;
+extern mem_alloc_description<mem_usage>& hash_table_usage (void);
 
 /* Support function for statistics.  */
 extern void dump_hash_table_loc_statistics (void);
@@ -580,7 +580,7 @@  hash_table<Descriptor, Allocator>::hash_table (size_t size, bool ggc, bool
   size = prime_tab[size_prime_index].prime;
 
   if (m_gather_mem_stats)
-    hash_table_usage.register_descriptor (this, origin, ggc
+    hash_table_usage ().register_descriptor (this, origin, ggc
 					  FINAL_PASS_MEM_STAT);
 
   m_entries = alloc_entries (size PASS_MEM_STAT);
@@ -600,7 +600,7 @@  hash_table<Descriptor, Allocator>::hash_table (const hash_table &h, bool ggc,
   size_t size = h.m_size;
 
   if (m_gather_mem_stats)
-    hash_table_usage.register_descriptor (this, origin, ggc
+    hash_table_usage ().register_descriptor (this, origin, ggc
 					  FINAL_PASS_MEM_STAT);
 
   value_type *nentries = alloc_entries (size PASS_MEM_STAT);
@@ -630,7 +630,7 @@  hash_table<Descriptor, Allocator>::~hash_table ()
     ggc_free (m_entries);
 
   if (m_gather_mem_stats)
-    hash_table_usage.release_instance_overhead (this,
+    hash_table_usage ().release_instance_overhead (this,
 						sizeof (value_type) * m_size,
 						true);
 }
@@ -644,7 +644,7 @@  hash_table<Descriptor, Allocator>::alloc_entries (size_t n MEM_STAT_DECL) const
   value_type *nentries;
 
   if (m_gather_mem_stats)
-    hash_table_usage.register_instance_overhead (sizeof (value_type) * n, this);
+    hash_table_usage ().register_instance_overhead (sizeof (value_type) * n, this);
 
   if (!m_ggc)
     nentries = Allocator <value_type> ::data_alloc (n);
@@ -736,7 +736,7 @@  hash_table<Descriptor, Allocator>::expand ()
   value_type *nentries = alloc_entries (nsize);
 
   if (m_gather_mem_stats)
-    hash_table_usage.release_instance_overhead (this, sizeof (value_type)
+    hash_table_usage ().release_instance_overhead (this, sizeof (value_type)
 						    * osize);
 
   m_entries = nentries;