On Mon, Mar 25, 2019 at 09:42:01PM +0100, Jakub Jelinek wrote:
> On Mon, Mar 25, 2019 at 03:10:04PM -0400, Jason Merrill wrote:
> > > 2) has the false -> true fixed
> > > 3) ditto, but furthermore is moved out of add_capture into the lambda
> > > introducer parsing routine only, tests for [this, this] and [this, *this]
> > > etc. are done using LAMBDA_EXPR_THIS_CAPTURE and the duplicate check for
> > > capture_id is simplified too.
> > 
> > I needed to make the below change to avoid crashing with
> > --enable-gather-detailed-mem-stats, make sense?  Incidentally, why not set
> 
> CCing Martin on this, I really don't know how the mem-stats.h stuff is
> supposed to work and how it works on reallocations.
> Seems release_instance_overhead is used in expand method without the second
> argument true and in the dtor with true, so the latter likely does both
> what expand needs to do and undo what register_descriptor did.
> And because register_descriptor has been called, it needs to be undone, but
> as no register_instance_overhead has been called, that part should not be.
> We don't have unregister_descriptor or something similar though.
> So, the fix probably needs to add something in mem-stats.h and do what your
> patch did + else if (m_gather_mem_stats) call this new mem_stat.h method
> after the if (!Lazy || m_entries) block.

Here is a patch that does that.

Bootstrapped on x86_64-linux with --enable-gather-detailed-mem-stats,
tested on a couple of lambda testcases with -fmem-report, some of them with
zero or one explicit captures, on others with more of them (e.g. on
lambda-init1{8,9}.C) and tested build without
--enable-gather-detailed-mem-stats.  Ok for trunk?

2019-03-26  Jason Merrill  <ja...@redhat.com>
            Jakub Jelinek  <ja...@redhat.com>

        * mem-stats.h (mem_alloc_description::unregister_descriptor): New
        method.
        (mem_alloc_description::release_object_overhead): Fix comment typos.
        * hash-table.h (hash_table::~hash_table): Call
        release_instance_overhead only if m_entries is non-NULL, otherwise
        call unregister_descriptor.

--- gcc/mem-stats.h.jj  2019-02-26 21:35:28.959081094 +0100
+++ gcc/mem-stats.h     2019-03-26 09:25:10.132128088 +0100
@@ -342,9 +342,15 @@ public:
   T *release_instance_overhead (void *ptr, size_t size,
                                bool remove_from_map = false);
 
-  /* Release intance object identified by PTR pointer.  */
+  /* Release instance object identified by PTR pointer.  */
   void release_object_overhead (void *ptr);
 
+  /* Unregister a memory allocation descriptor registered with
+     register_descriptor (remove from reverse map), unless it is
+     unregistered through release_instance_overhead with
+     REMOVE_FROM_MAP = true.  */
+  void unregister_descriptor (void *ptr);
+
   /* Get sum value for ORIGIN type of allocation for the descriptor.  */
   T get_sum (mem_alloc_origin origin);
 
@@ -522,7 +528,7 @@ mem_alloc_description<T>::release_instan
   return usage;
 }
 
-/* Release intance object identified by PTR pointer.  */
+/* Release instance object identified by PTR pointer.  */
 
 template <class T>
 inline void
@@ -536,6 +542,17 @@ mem_alloc_description<T>::release_object
     }
 }
 
+/* Unregister a memory allocation descriptor registered with
+   register_descriptor (remove from reverse map), unless it is
+   unregistered through release_instance_overhead with
+   REMOVE_FROM_MAP = true.  */
+template <class T>
+inline void
+mem_alloc_description<T>::unregister_descriptor (void *ptr)
+{
+  m_reverse_map->remove (ptr);
+}
+
 /* Default contructor.  */
 
 template <class T>
--- gcc/hash-table.h.jj 2019-03-26 08:52:52.739640547 +0100
+++ gcc/hash-table.h    2019-03-26 09:26:27.697864773 +0100
@@ -652,12 +652,13 @@ hash_table<Descriptor, Lazy, Allocator>:
        Allocator <value_type> ::data_free (m_entries);
       else
        ggc_free (m_entries);
+      if (m_gather_mem_stats)
+       hash_table_usage ().release_instance_overhead (this,
+                                                      sizeof (value_type)
+                                                      * m_size, true);
     }
-
-  if (m_gather_mem_stats)
-    hash_table_usage ().release_instance_overhead (this,
-                                                  sizeof (value_type)
-                                                  * m_size, true);
+  else if (m_gather_mem_stats)
+    hash_table_usage ().unregister_descriptor (this);
 }
 
 /* This function returns an array of empty hash table elements.  */


        Jakub

Reply via email to