Patchwork Backported r185231 from trunk to google/gcc-4_7. (issue 6151043)

login
register
mail settings
Submitter asharif@chromium.org
Date May 1, 2012, 10:47 p.m.
Message ID <20cf300fb4459ac22604bf015b9c@google.com>
Download mbox | patch
Permalink /patch/156258/
State New
Headers show

Comments

asharif@chromium.org - May 1, 2012, 10:47 p.m.
Reviewers: xur, davidxl,

Message:
Please take a look at this.

Description:
This fixes an issue with profile collection when multiple threads call
fork() around the same time.

2012-03-12  Richard Guenther  <rguenther@suse.de>

	* gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
	* gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
	(__gthread_mutex_init_function): New function.
	* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.

	PR gcov/49484
	* libgcov.c: Include gthr.h.
	(__gcov_flush_mx): New global variable.
	(init_mx, init_mx_once): New functions.
	(__gcov_flush): Protect self with a mutex.
	(__gcov_fork): Re-initialize mutex after forking.
	* unwind-dw2-fde.c: Change condition under which to use
	__GTHREAD_MUTEX_INIT_FUNCTION.


Please review this at http://codereview.appspot.com/6151043/

Affected files:
    M    .
   M     libgcc/ChangeLog
   M     libgcc/ChangeLog.google-4_7
   M     libgcc/gthr-posix.h
   M     libgcc/gthr-single.h
   M     libgcc/gthr.h
   M     libgcc/libgcov.c
   M     libgcc/unwind-dw2-fde.c


Index: .
===================================================================
--- .	(revision 187027)
+++ .	(working copy)

Property changes on: .
Xinliang David Li - May 1, 2012, 10:50 p.m.
ok.

David

On Tue, May 1, 2012 at 3:47 PM,  <asharif@chromium.org> wrote:
> Reviewers: xur, davidxl,
>
> Message:
> Please take a look at this.
>
> Description:
> This fixes an issue with profile collection when multiple threads call
> fork() around the same time.
>
> 2012-03-12  Richard Guenther  <rguenther@suse.de>
>
>        * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
>        * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
>        (__gthread_mutex_init_function): New function.
>        * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
>
>        PR gcov/49484
>        * libgcov.c: Include gthr.h.
>        (__gcov_flush_mx): New global variable.
>        (init_mx, init_mx_once): New functions.
>        (__gcov_flush): Protect self with a mutex.
>        (__gcov_fork): Re-initialize mutex after forking.
>        * unwind-dw2-fde.c: Change condition under which to use
>        __GTHREAD_MUTEX_INIT_FUNCTION.
>
>
> Please review this at http://codereview.appspot.com/6151043/
>
> Affected files:
>   M    .
>  M     libgcc/ChangeLog
>  M     libgcc/ChangeLog.google-4_7
>  M     libgcc/gthr-posix.h
>  M     libgcc/gthr-single.h
>  M     libgcc/gthr.h
>  M     libgcc/libgcov.c
>  M     libgcc/unwind-dw2-fde.c
>
>
> Index: .
> ===================================================================
> --- .   (revision 187027)
> +++ .   (working copy)
>
> Property changes on: .
> ___________________________________________________________________
> Modified: svn:mergeinfo
>   Merged /trunk:r185231
> Index: libgcc/ChangeLog
> ===================================================================
> --- libgcc/ChangeLog    (revision 187027)
> +++ libgcc/ChangeLog    (working copy)
> @@ -1,3 +1,19 @@
> +2012-03-12  Richard Guenther  <rguenther@suse.de>
> +
> +       * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
> +       * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
> +       (__gthread_mutex_init_function): New function.
> +       * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
> +
> +       PR gcov/49484
> +       * libgcov.c: Include gthr.h.
> +       (__gcov_flush_mx): New global variable.
> +       (init_mx, init_mx_once): New functions.
> +       (__gcov_flush): Protect self with a mutex.
> +       (__gcov_fork): Re-initialize mutex after forking.
> +       * unwind-dw2-fde.c: Change condition under which to use
> +       __GTHREAD_MUTEX_INIT_FUNCTION.
> +
>  2012-04-20  Thomas Schwinge  <thomas@codesourcery.com>
>
>        struct siginfo vs. siginfo_t
> Index: libgcc/ChangeLog.google-4_7
> ===================================================================
> --- libgcc/ChangeLog.google-4_7 (revision 187027)
> +++ libgcc/ChangeLog.google-4_7 (working copy)
> @@ -1,3 +1,20 @@
> +2012-05-01  Ahmad Sharif  <asharif@google.com>
> +       2012-03-12  Richard Guenther  <rguenther@suse.de>
> +
> +       * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
> +       * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
> +       (__gthread_mutex_init_function): New function.
> +       * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
> +
> +       PR gcov/49484
> +       * libgcov.c: Include gthr.h.
> +       (__gcov_flush_mx): New global variable.
> +       (init_mx, init_mx_once): New functions.
> +       (__gcov_flush): Protect self with a mutex.
> +       (__gcov_fork): Re-initialize mutex after forking.
> +       * unwind-dw2-fde.c: Change condition under which to use
> +       __GTHREAD_MUTEX_INIT_FUNCTION.
> +
>  2012-04-17  Teresa Johnson  <tejohnson@google.com>
>
>        Backport from google/main rev r186550
> Index: libgcc/gthr-single.h
> ===================================================================
> --- libgcc/gthr-single.h        (revision 187027)
> +++ libgcc/gthr-single.h        (working copy)
> @@ -36,6 +36,7 @@
>
>  #define __GTHREAD_ONCE_INIT 0
>  #define __GTHREAD_MUTEX_INIT 0
> +#define __GTHREAD_MUTEX_INIT_FUNCTION (mx)
>  #define __GTHREAD_RECURSIVE_MUTEX_INIT 0
>
>  #define UNUSED __attribute__((unused))
> Index: libgcc/libgcov.c
> ===================================================================
> --- libgcc/libgcov.c    (revision 187027)
> +++ libgcc/libgcov.c    (working copy)
> @@ -48,6 +48,7 @@
>  #include "tm.h"
>  #endif /* __KERNEL__ */
>  #include "libgcc_tm.h"
> +#include "gthr.h"
>
>  #if 1
>  #define THREAD_PREFIX __thread
> @@ -668,6 +669,25 @@
>   info->version = 0;
>  }
>
> +#ifdef __GTHREAD_MUTEX_INIT
> +ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT;
> +#define init_mx_once()
> +#else
> +__gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
> +
> +static void
> +init_mx (void)
> +{
> +  __GTHREAD_MUTEX_INIT_FUNCTION (&mx);
> +}
> +static void
> +init_mx_once (void)
> +{
> +  static __gthread_once_t once = __GTHREAD_ONCE_INIT;
> +  __gthread_once (&once, init_mx);
> +}
> +#endif
> +
>  /* Called before fork or exec - write out profile information gathered so
>    far and reset it to zero.  This avoids duplication or loss of the
>    profile information gathered so far.  */
> @@ -677,6 +697,9 @@
>  {
>   const struct gcov_info *gi_ptr;
>
> +  init_mx_once ();
> +  __gthread_mutex_lock (&__gcov_flush_mx);
> +
>   __gcov_stop_pmu_profiler ();
>   gcov_exit ();
>   for (gi_ptr = __gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> @@ -700,6 +723,7 @@
>         }
>     }
>   __gcov_start_pmu_profiler ();
> +  __gthread_mutex_unlock (&__gcov_flush_mx);
>  }
>
>  #else /* __GCOV_KERNEL__ */
> @@ -1721,8 +1745,13 @@
>  pid_t
>  __gcov_fork (void)
>  {
> +  pid_t pid;
> +  extern __gthread_mutex_t __gcov_flush_mx;
>   __gcov_flush ();
> -  return fork ();
> +  pid = fork ();
> +  if (pid == 0)
> +    __GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx);
> +  return pid;
>  }
>  #endif
>
> Index: libgcc/unwind-dw2-fde.c
> ===================================================================
> --- libgcc/unwind-dw2-fde.c     (revision 187027)
> +++ libgcc/unwind-dw2-fde.c     (working copy)
> @@ -47,11 +47,10 @@
>
>  #ifdef __GTHREAD_MUTEX_INIT
>  static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT;
> +#define init_object_mutex_once()
>  #else
>  static __gthread_mutex_t object_mutex;
> -#endif
>
> -#ifdef __GTHREAD_MUTEX_INIT_FUNCTION
>  static void
>  init_object_mutex (void)
>  {
> @@ -64,8 +63,6 @@
>   static __gthread_once_t once = __GTHREAD_ONCE_INIT;
>   __gthread_once (&once, init_object_mutex);
>  }
> -#else
> -#define init_object_mutex_once()
>  #endif
>
>  /* Called from crtbegin.o to register the unwind info for an object.  */
> Index: libgcc/gthr-posix.h
> ===================================================================
> --- libgcc/gthr-posix.h (revision 187027)
> +++ libgcc/gthr-posix.h (working copy)
> @@ -63,6 +63,7 @@
>  #define __GTHREAD_HAS_COND     1
>
>  #define __GTHREAD_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER
> +#define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function
>  #define __GTHREAD_ONCE_INIT PTHREAD_ONCE_INIT
>  #if defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER)
>  #define __GTHREAD_RECURSIVE_MUTEX_INIT PTHREAD_RECURSIVE_MUTEX_INITIALIZER
> @@ -754,6 +755,14 @@
>  #endif
>
>  static inline int
> +__gthread_mutex_init_function (__gthread_mutex_t *__mutex)
> +{
> +  if (__gthread_active_p ())
> +    return __gthrw_(pthread_mutex_init) (__mutex, NULL);
> +  return 0;
> +}
> +
> +static inline int
>  __gthread_mutex_destroy (__gthread_mutex_t *__mutex)
>  {
>   if (__gthread_active_p ())
> Index: libgcc/gthr.h
> ===================================================================
> --- libgcc/gthr.h       (revision 187027)
> +++ libgcc/gthr.h       (working copy)
> @@ -52,11 +52,12 @@
>                to initialize __gthread_mutex_t to get a fast
>                non-recursive mutex.
>      __GTHREAD_MUTEX_INIT_FUNCTION
> -               some systems can't initialize a mutex without a
> -               function call.  On such systems, define this to a
> -               function which looks like this:
> +               to initialize __gthread_mutex_t to get a fast
> +               non-recursive mutex.
> +               Define this to a function which looks like this:
>                  void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
> -               Don't define __GTHREAD_MUTEX_INIT in this case
> +               Some systems can't initialize a mutex without a
> +               function call.  Don't define __GTHREAD_MUTEX_INIT in this
> case.
>      __GTHREAD_RECURSIVE_MUTEX_INIT
>      __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION
>                as above, but for a recursive mutex.
>
>

Patch

Index: libgcc/ChangeLog
===================================================================
--- libgcc/ChangeLog	(revision 187027)
+++ libgcc/ChangeLog	(working copy)
@@ -1,3 +1,19 @@ 
+2012-03-12  Richard Guenther  <rguenther@suse.de>
+
+	* gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
+	* gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+	(__gthread_mutex_init_function): New function.
+	* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+
+	PR gcov/49484
+	* libgcov.c: Include gthr.h.
+	(__gcov_flush_mx): New global variable.
+	(init_mx, init_mx_once): New functions.
+	(__gcov_flush): Protect self with a mutex.
+	(__gcov_fork): Re-initialize mutex after forking.
+	* unwind-dw2-fde.c: Change condition under which to use
+	__GTHREAD_MUTEX_INIT_FUNCTION.
+
  2012-04-20  Thomas Schwinge  <thomas@codesourcery.com>

  	struct siginfo vs. siginfo_t
Index: libgcc/ChangeLog.google-4_7
===================================================================
--- libgcc/ChangeLog.google-4_7	(revision 187027)
+++ libgcc/ChangeLog.google-4_7	(working copy)
@@ -1,3 +1,20 @@ 
+2012-05-01  Ahmad Sharif  <asharif@google.com>
+	2012-03-12  Richard Guenther  <rguenther@suse.de>
+
+	* gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
+	* gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+	(__gthread_mutex_init_function): New function.
+	* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+
+	PR gcov/49484
+	* libgcov.c: Include gthr.h.
+	(__gcov_flush_mx): New global variable.
+	(init_mx, init_mx_once): New functions.
+	(__gcov_flush): Protect self with a mutex.
+	(__gcov_fork): Re-initialize mutex after forking.
+	* unwind-dw2-fde.c: Change condition under which to use
+	__GTHREAD_MUTEX_INIT_FUNCTION.
+
  2012-04-17  Teresa Johnson  <tejohnson@google.com>

  	Backport from google/main rev r186550
Index: libgcc/gthr-single.h
===================================================================
--- libgcc/gthr-single.h	(revision 187027)
+++ libgcc/gthr-single.h	(working copy)
@@ -36,6 +36,7 @@ 

  #define __GTHREAD_ONCE_INIT 0
  #define __GTHREAD_MUTEX_INIT 0
+#define __GTHREAD_MUTEX_INIT_FUNCTION (mx)
  #define __GTHREAD_RECURSIVE_MUTEX_INIT 0

  #define UNUSED __attribute__((unused))
Index: libgcc/libgcov.c
===================================================================
--- libgcc/libgcov.c	(revision 187027)
+++ libgcc/libgcov.c	(working copy)
@@ -48,6 +48,7 @@ 
  #include "tm.h"
  #endif /* __KERNEL__ */
  #include "libgcc_tm.h"
+#include "gthr.h"

  #if 1
  #define THREAD_PREFIX __thread
@@ -668,6 +669,25 @@ 
    info->version = 0;
  }

+#ifdef __GTHREAD_MUTEX_INIT
+ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT;
+#define init_mx_once()
+#else
+__gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
+
+static void
+init_mx (void)
+{
+  __GTHREAD_MUTEX_INIT_FUNCTION (&mx);
+}
+static void
+init_mx_once (void)
+{
+  static __gthread_once_t once = __GTHREAD_ONCE_INIT;
+  __gthread_once (&once, init_mx);
+}
+#endif
+
  /* Called before fork or exec - write out profile information gathered so
     far and reset it to zero.  This avoids duplication or loss of the
     profile information gathered so far.  */
@@ -677,6 +697,9 @@ 
  {
    const struct gcov_info *gi_ptr;

+  init_mx_once ();
+  __gthread_mutex_lock (&__gcov_flush_mx);
+
    __gcov_stop_pmu_profiler ();
    gcov_exit ();
    for (gi_ptr = __gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
@@ -700,6 +723,7 @@ 
          }
      }
    __gcov_start_pmu_profiler ();
+  __gthread_mutex_unlock (&__gcov_flush_mx);
  }

  #else /* __GCOV_KERNEL__ */
@@ -1721,8 +1745,13 @@ 
  pid_t
  __gcov_fork (void)
  {
+  pid_t pid;
+  extern __gthread_mutex_t __gcov_flush_mx;
    __gcov_flush ();
-  return fork ();
+  pid = fork ();
+  if (pid == 0)
+    __GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx);
+  return pid;
  }
  #endif

Index: libgcc/unwind-dw2-fde.c
===================================================================
--- libgcc/unwind-dw2-fde.c	(revision 187027)
+++ libgcc/unwind-dw2-fde.c	(working copy)
@@ -47,11 +47,10 @@ 

  #ifdef __GTHREAD_MUTEX_INIT
  static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT;
+#define init_object_mutex_once()
  #else
  static __gthread_mutex_t object_mutex;
-#endif

-#ifdef __GTHREAD_MUTEX_INIT_FUNCTION
  static void
  init_object_mutex (void)
  {
@@ -64,8 +63,6 @@ 
    static __gthread_once_t once = __GTHREAD_ONCE_INIT;
    __gthread_once (&once, init_object_mutex);
  }
-#else
-#define init_object_mutex_once()
  #endif

  /* Called from crtbegin.o to register the unwind info for an object.  */
Index: libgcc/gthr-posix.h
===================================================================
--- libgcc/gthr-posix.h	(revision 187027)
+++ libgcc/gthr-posix.h	(working copy)
@@ -63,6 +63,7 @@ 
  #define __GTHREAD_HAS_COND	1

  #define __GTHREAD_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER
+#define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function
  #define __GTHREAD_ONCE_INIT PTHREAD_ONCE_INIT
  #if defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER)
  #define __GTHREAD_RECURSIVE_MUTEX_INIT PTHREAD_RECURSIVE_MUTEX_INITIALIZER
@@ -754,6 +755,14 @@ 
  #endif

  static inline int
+__gthread_mutex_init_function (__gthread_mutex_t *__mutex)
+{
+  if (__gthread_active_p ())
+    return __gthrw_(pthread_mutex_init) (__mutex, NULL);
+  return 0;
+}
+
+static inline int
  __gthread_mutex_destroy (__gthread_mutex_t *__mutex)
  {
    if (__gthread_active_p ())
Index: libgcc/gthr.h
===================================================================
--- libgcc/gthr.h	(revision 187027)
+++ libgcc/gthr.h	(working copy)
@@ -52,11 +52,12 @@ 
       		to initialize __gthread_mutex_t to get a fast
  		non-recursive mutex.
       __GTHREAD_MUTEX_INIT_FUNCTION
-     		some systems can't initialize a mutex without a
-		function call.  On such systems, define this to a
-		function which looks like this:
+		to initialize __gthread_mutex_t to get a fast
+		non-recursive mutex.
+		Define this to a function which looks like this:
  		  void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
-		Don't define __GTHREAD_MUTEX_INIT in this case
+     		Some systems can't initialize a mutex without a
+		function call.  Don't define __GTHREAD_MUTEX_INIT in this case.
       __GTHREAD_RECURSIVE_MUTEX_INIT
       __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION
       		as above, but for a recursive mutex.