diff mbox

a new libgcov interface: __gcov_dump_all

Message ID CAAkRFZL97Tp3Xfexh+-So43juYUsPar1yQKgHLSXu8jt0PNYnw@mail.gmail.com
State New
Headers show

Commit Message

Xinliang David Li July 22, 2014, 4:04 p.m. UTC
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 <nathan@acm.org> 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

Comments

Martin Liška Oct. 26, 2017, 8:47 a.m. UTC | #1
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 <nathan@acm.org> 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
Martin Liška Dec. 29, 2017, 2:17 p.m. UTC | #2
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 <nathan@acm.org> 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
diff mbox

Patch

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  <davidxl@google.com>
+
+	* 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  <rguenther@suse.de>
 
 	* 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
+   visbility.  */
+
+void
+__gcov_dump_all (void)
+{
+  struct dumper_entry *dumper;
+
+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_dump_mx);
+
+  dumper = __gcov_dumper_list;
+  while (dumper)
+   {
+     dumper->dumper ();
+     dumper = dumper->next_dumper;
+   }
+  __gthread_mutex_unlock (&__gcov_dump_mx);
+}
+
 #endif /* L_gcov_dump */