On 12/16/20 10:38 AM, Rainer Orth wrote:
Hi Jakub,

On Wed, Dec 16, 2020 at 10:20:09AM +0100, Martin Liška wrote:
So vec_mem_desc is not initialized before a static member in module.cc.
We can fix it by using constructor attribute.
[...]
+   of all static variables.  */
+
+static void
+__attribute__((constructor (101)))

I think this needs to be guarded based on which compiler is used to compile
GCC.  Perhaps we could say that we don't support
--enable-gather-detailed-mem-stats when the compiler isn't built by GCC (or
other compiler that supports the constructor attribute) and #error on that.

not only that: if a target doesn't support constructor priorities (like
Solaris 11.3, unlike 11.4), this makes gcc error out.

        Rainer


I see, I'm then suggesting version 3 of the patch that does not depend
on a constructor.

Thoughts?
Thanks,
Martin

Martin
>From 3ac0d258887426b30d3e1885841b1bdf4e53100b Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Wed, 16 Dec 2020 10:02:10 +0100
Subject: [PATCH] Fix --enable-gather-detailed-mem-stats build.

The build suffers from the static initialization order fiasco:

==30085== Invalid read of size 4
==30085==    at 0x1D451CD: hash_table_mod1 (hash-table.h:344)
==30085==    by 0x1D451CD: hash_table<hash_map<mem_alloc_description<vec_usage>::mem_location_hash, vec_usage*, simple_hashmap_traits<default_hash_traits<mem_alloc_description<vec_usage>::mem_location_hash>, vec_usage*> >::hash_entry, false, xcallocator>::find_with_hash(mem_location* const&, unsigned int) (hash-table.h:911)
==30085==    by 0x1D411F7: get (hash-map.h:185)
==30085==    by 0x1D411F7: register_descriptor (mem-stats.h:417)
==30085==    by 0x1D411F7: register_descriptor (mem-stats.h:451)
==30085==    by 0x1D411F7: vec_prefix::register_overhead(void*, unsigned long, unsigned long, char const*, int, char const*) (vec.c:132)
==30085==    by 0xA2DB28: reserve<loc_spans::span> (vec.h:294)
==30085==    by 0xA2DB28: vec<loc_spans::span, va_heap, vl_ptr>::reserve(unsigned int, bool, char const*, int, char const*) [clone .isra.0] (vec.h:1778)
==30085==    by 0x9039C7: reserve_exact (vec.h:1798)
==30085==    by 0x9039C7: create (vec.h:1813)
==30085==    by 0x9039C7: loc_spans (module.cc:3281)
==30085==    by 0x9039C7: __static_initialization_and_destruction_0 (module.cc:3340)
==30085==    by 0x9039C7: _GLOBAL__sub_I_map_context_from (gt-cp-module.h:360)
==30085==    by 0x1E00F6C: __libc_csu_init (elf-init.c:89)
==30085==    by 0x4FFF0DD: (below main) (in /lib64/libc-2.32.so)
==30085==  Address 0x28 is not stack'd, malloc'd or (recently) free'd

So vec_mem_desc is not initialized before a static member in module.cc.

gcc/ChangeLog:

	* vec.c (vec_prefix::register_overhead): Initialize vec_mem_desc
	if it is NULL.
	(vec_prefix::release_overhead): Use pointer type for
	vec_mem_desc.
	(dump_vec_loc_statistics): Likewise.
---
 gcc/vec.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/gcc/vec.c b/gcc/vec.c
index a28899170ed..7df0f54605e 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -121,7 +121,7 @@ public:
 };
 
 /* Vector memory description.  */
-static mem_alloc_description <vec_usage> vec_mem_desc;
+static mem_alloc_description <vec_usage> *vec_mem_desc = NULL;
 
 /* Account the overhead.  */
 
@@ -129,10 +129,13 @@ 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);
+  if (vec_mem_desc == NULL)
+    vec_mem_desc = new mem_alloc_description<vec_usage> ();
+
+  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)
@@ -145,11 +148,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;
 }
 
@@ -183,7 +186,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);
 }
 
 #if CHECKING_P
-- 
2.29.2

Reply via email to