Re: a new libgcov interface: __gcov_dump_all

2017-12-29 Thread Martin Liška
On 10/26/2017 10:47 AM, Martin Liška wrote:
> On 07/22/2014 06:04 PM, Xinliang David Li wrote:
>> Please take a look the updated patch. It addresses the issue of using
>> dlclose before dump, and potential races (between a thread closing a
>> library and the dumper call).
>>
>> David
>>
>> On Sun, Jul 20, 2014 at 11:12 PM, Nathan Sidwell  wrote:
>>> On 07/20/14 21:38, Xinliang David Li wrote:

 The gcov_info chain is not duplicated -- there is already one chain
 (linking only modules of the library) per shared library in current
 implementation.  My change does not affect underlying behavior at all
 -- it merely introduces a new interface to access private dumper
 methods associated with shared libs.
>>>
>>>
>>> ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be
>>> doing this, and wondering about dlload/dlclose.  Let me think
>>>
>>> nathan
> 
> Hi.
> 
> Unfortunately, it looks the patch hasn't been installed to trunk. Some folks 
> from Firefox
> are interested in that.
> 
> Are you Nathan OK with the patch? I guess a rebase will be needed.
> 
> Martin
> 

Hi.

I would like to suspend the patch review as there's actually no call for the 
feature.
The person which sent me the link actually wanted to dump all counters. That 
can be
easily done via __gcov_dump interface we already have.

Martin


Re: a new libgcov interface: __gcov_dump_all

2017-10-26 Thread Martin Liška
On 07/22/2014 06:04 PM, Xinliang David Li wrote:
> Please take a look the updated patch. It addresses the issue of using
> dlclose before dump, and potential races (between a thread closing a
> library and the dumper call).
> 
> David
> 
> On Sun, Jul 20, 2014 at 11:12 PM, Nathan Sidwell  wrote:
>> On 07/20/14 21:38, Xinliang David Li wrote:
>>>
>>> The gcov_info chain is not duplicated -- there is already one chain
>>> (linking only modules of the library) per shared library in current
>>> implementation.  My change does not affect underlying behavior at all
>>> -- it merely introduces a new interface to access private dumper
>>> methods associated with shared libs.
>>
>>
>> ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be
>> doing this, and wondering about dlload/dlclose.  Let me think
>>
>> nathan

Hi.

Unfortunately, it looks the patch hasn't been installed to trunk. Some folks 
from Firefox
are interested in that.

Are you Nathan OK with the patch? I guess a rebase will be needed.

Martin


Re: a new libgcov interface: __gcov_dump_all

2014-07-22 Thread Xinliang David Li
Please take a look the updated patch. It addresses the issue of using
dlclose before dump, and potential races (between a thread closing a
library and the dumper call).

David

On Sun, Jul 20, 2014 at 11:12 PM, Nathan Sidwell  wrote:
> On 07/20/14 21:38, Xinliang David Li wrote:
>>
>> The gcov_info chain is not duplicated -- there is already one chain
>> (linking only modules of the library) per shared library in current
>> implementation.  My change does not affect underlying behavior at all
>> -- it merely introduces a new interface to access private dumper
>> methods associated with shared libs.
>
>
> ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be
> doing this, and wondering about dlload/dlclose.  Let me think
>
> nathan
Index: libgcc/libgcov.h
===
--- libgcc/libgcov.h(revision 212682)
+++ libgcc/libgcov.h(working copy)
@@ -218,8 +218,14 @@ extern void __gcov_flush (void) ATTRIBUT
 /* Function to reset all counters to 0.  */
 extern void __gcov_reset (void);
 
-/* Function to enable early write of profile information so far.  */
-extern void __gcov_dump (void);
+/* Function to enable early write of profile information so far.  
+   __GCOV_DUMP is also used by __GCOV_DUMP_ALL. The latter 
+   depends on __GCOV_DUMP to have hidden or protected visibility
+   so that each library has its own copy of the registered dumper.  */
+extern void __gcov_dump (void) ATTRIBUTE_HIDDEN;
+
+/* Call __gcov_dump registered from each shared library.  */
+void __gcov_dump_all (void);
 
 /* The merge function that just sums the counters.  */
 extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
Index: libgcc/ChangeLog
===
--- libgcc/ChangeLog(revision 212682)
+++ libgcc/ChangeLog(working copy)
@@ -1,3 +1,9 @@
+2014-07-18  Xinliang David Li  
+
+   * libgcov-driver.c: Force __gcov_dump to be exported
+   * libgcov-interface.c (register_dumper): New function.
+   (__gcov_dump_all): Ditto.
+
 2014-07-14  Richard Biener  
 
* libgcov.h (struct gcov_fn_info): Make ctrs size 1.
Index: libgcc/libgcov-driver.c
===
--- libgcc/libgcov-driver.c (revision 212682)
+++ libgcc/libgcov-driver.c (working copy)
@@ -55,6 +55,13 @@ extern void set_gcov_dump_complete (void
 extern void reset_gcov_dump_complete (void) ATTRIBUTE_HIDDEN;
 extern int get_gcov_dump_complete (void) ATTRIBUTE_HIDDEN;
 extern void set_gcov_list (struct gcov_info *) ATTRIBUTE_HIDDEN;
+  
+#ifndef IN_GCOV_TOOL
+
+/* Creates strong reference to force _gcov_dump.o to be linked
+   in shared library for exported interfaces.  */
+void (*__gcov_dummy_ref)(void) = &__gcov_dump;
+#endif
 
 struct gcov_fn_buffer
 {
Index: libgcc/libgcov-interface.c
===
--- libgcc/libgcov-interface.c  (revision 212682)
+++ libgcc/libgcov-interface.c  (working copy)
@@ -115,6 +115,100 @@ __gcov_dump (void)
   set_gcov_dump_complete ();
 }
 
+typedef void (*gcov_dumper_type) (void);
+struct dumper_entry
+{
+  gcov_dumper_type dumper;
+  struct dumper_entry *next_dumper;
+};
+
+static struct dumper_entry this_dumper = {&__gcov_dump, 0};
+
+/* global dumper list with default visibilty. */
+struct dumper_entry *__gcov_dumper_list;
+
+#ifdef __GTHREAD_MUTEX_INIT
+__gthread_mutex_t __gcov_dump_mx = __GTHREAD_MUTEX_INIT;
+#define init_mx_once()
+#else
+__gthread_mutex_t __gcov_dump_mx;
+
+static void
+init_mx (void)
+{
+  __GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_dump_mx);
+}
+static void
+init_mx_once (void)
+{
+  static __gthread_once_t once = __GTHREAD_ONCE_INIT;
+  __gthread_once (&once, init_mx);
+}
+#endif
+
+/* Register the library private __gcov_dump method
+   to the global list.  */
+
+__attribute__((constructor))
+static void
+register_dumper (void)
+{
+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_dump_mx);
+  this_dumper.next_dumper = __gcov_dumper_list;
+  __gcov_dumper_list = &this_dumper;
+  __gthread_mutex_unlock (&__gcov_dump_mx);
+}
+
+__attribute__((destructor))
+static void
+unregister_dumper (void)
+{
+  struct dumper_entry *dumper;
+  struct dumper_entry *prev_dumper = 0;
+
+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_dump_mx);
+  dumper = __gcov_dumper_list;
+
+  while (dumper)
+{
+  if (dumper->dumper == &__gcov_dump)
+{
+ if (prev_dumper)
+   prev_dumper->next_dumper = dumper->next_dumper;
+ else
+   __gcov_dumper_list = dumper->next_dumper;
+  break;
+}
+  prev_dumper = dumper;
+  dumper = dumper->next_dumper;
+}
+  __gthread_mutex_unlock (&__gcov_dump_mx);
+}
+
+/* Public interface to dump profile data for all shared libraries
+   via registered dumpers from the libraries. This interface
+   has default visibility (unlike gcov_dump which has hidden
+

Re: a new libgcov interface: __gcov_dump_all

2014-07-20 Thread Nathan Sidwell

On 07/20/14 21:38, Xinliang David Li wrote:

The gcov_info chain is not duplicated -- there is already one chain
(linking only modules of the library) per shared library in current
implementation.  My change does not affect underlying behavior at all
-- it merely introduces a new interface to access private dumper
methods associated with shared libs.


ah, got it.  thanks for clarifying.  Can't help thinking gcov_init should be 
doing this, and wondering about dlload/dlclose.  Let me think


nathan


Re: a new libgcov interface: __gcov_dump_all

2014-07-20 Thread Xinliang David Li
The gcov_info chain is not duplicated -- there is already one chain
(linking only modules of the library) per shared library in current
implementation.  My change does not affect underlying behavior at all
-- it merely introduces a new interface to access private dumper
methods associated with shared libs.

David



On Sun, Jul 20, 2014 at 12:42 PM, Nathan Sidwell  wrote:
> On 07/18/14 22:41, Xinliang David Li wrote:
>>
>> Hi, the following patch implements a new dumper interface to allow
>> dumping of profile data for all instrumented shared libraries.
>>
>> For good reasons, existing libgcov implements the dumping on a
>> per-shared library basis (i.e., gcov_exit is hidden, gcov_list is file
>> static). This allows each shared library's profile data to be dumped
>> independently with separate summary data. The downside is that there
>> is no interface that can be invoked to dump profile data for all
>> shared modules.
>
>
> This seems like useful functionality, but I don't think this is the right
> way to do this.  You're duplicating the gcov info object chain.  Why can't
> you expose gcov_list from gcov-driver.c (possibly with a different name, of
> course?
>
> nathan


Re: a new libgcov interface: __gcov_dump_all

2014-07-20 Thread Nathan Sidwell

On 07/18/14 22:41, Xinliang David Li wrote:

Hi, the following patch implements a new dumper interface to allow
dumping of profile data for all instrumented shared libraries.

For good reasons, existing libgcov implements the dumping on a
per-shared library basis (i.e., gcov_exit is hidden, gcov_list is file
static). This allows each shared library's profile data to be dumped
independently with separate summary data. The downside is that there
is no interface that can be invoked to dump profile data for all
shared modules.


This seems like useful functionality, but I don't think this is the right way to 
do this.  You're duplicating the gcov info object chain.  Why can't you expose 
gcov_list from gcov-driver.c (possibly with a different name, of course?


nathan