This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Patch] Memory fencing fixes



I think this incorporates all the input from the list. If someone could take a looksie at the ARM stuff it'd be appreciated. While I think I stitched the bits from Carlos & Joseph together correctly, sanity checking helps.


diff --git a/ChangeLog b/ChangeLog
index 2e84358..f47338e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2013-02-04  Jeff Law  <law@redhat.com>
+            Carlos O'Donell  <codonell@redhat.com>
+	    Joseph Myers  <joseph@codesourcery.com>
+
+        * sysdeps/gnu/unwind-resume.c (init): Update comments.
+        (_maybe_init): New function.
+        (Unwind_Resume, __gcc_personality_v0): Use it.
+
 2013-02-04  David S. Miller  <davem@davemloft.net>
 
 	* sysdeps/sparc/fpu/libm-test-ulps: Update.
diff --git a/nptl/ChangeLog b/nptl/ChangeLog
index bcc1660..0a01134 100644
--- a/nptl/ChangeLog
+++ b/nptl/ChangeLog
@@ -1,3 +1,14 @@
+2013-02-04  Jeff Law  <law@redhat.com>
+	    Carlos O'Donell  <codonell@redhat.com>
+	    Joseph Myers  <joseph@codesourcery.com>
+
+	* sysdeps/pthread/unwind-forcedunwind.c (pthread_cancel_init): Add
+	missing call to atomic_read_barrier in early return path.  Update
+	comments.
+	(_maybe_pthread_cancel_init): New function.
+	(Unwind_Resume, __gcc_personality_v0): Use it.
+	(_Unwind_ForcedUnwind, _Unwind_GetCFA): Likewise.
+
 2013-02-04  Andreas Schwab  <schwab@suse.de>
 
 	[BZ #14142]
diff --git a/nptl/sysdeps/pthread/unwind-forcedunwind.c b/nptl/sysdeps/pthread/unwind-forcedunwind.c
index 9718606..1c3ea58 100644
--- a/nptl/sysdeps/pthread/unwind-forcedunwind.c
+++ b/nptl/sysdeps/pthread/unwind-forcedunwind.c
@@ -42,10 +42,12 @@ pthread_cancel_init (void)
   void *getcfa;
   void *handle;
 
-  if (__builtin_expect (libgcc_s_handle != NULL, 1))
+  if (__glibc_likely (libgcc_s_handle != NULL))
     {
-      /* Force gcc to reload all values.  */
-      asm volatile ("" ::: "memory");
+      /* Order reads so as to prevent speculation of loads
+	 of libgcc_s_{resume,personality,forcedunwind,getcfa}
+	 to points prior to the write barrier.  */
+      atomic_read_barrier ();
       return;
     }
 
@@ -71,9 +73,14 @@ pthread_cancel_init (void)
   libgcc_s_forcedunwind = forcedunwind;
   PTR_MANGLE (getcfa);
   libgcc_s_getcfa = getcfa;
-  /* Make sure libgcc_s_handle is written last.  Otherwise,
-     pthread_cancel_init might return early even when the pointer the
-     caller is interested in is not initialized yet.  */
+  /* At the point at which any thread writes the handle
+     to libgcc_s_handle, the initialization is complete.
+     The writing of libgcc_s_handle is atomic. All other
+     threads reading libgcc_s_handle do so atomically. Any
+     thread that does not execute this function must issue
+     a read barrier to ensure that all of the above has
+     actually completed and that the values of the
+     function pointers are correct.   */
   atomic_write_barrier ();
   libgcc_s_handle = handle;
 }
@@ -90,13 +97,19 @@ __unwind_freeres (void)
     }
 }
 
-void
-_Unwind_Resume (struct _Unwind_Exception *exc)
+static __always_inline void
+_maybe_pthread_cancel_init (void)
 {
-  if (__builtin_expect (libgcc_s_handle == NULL, 0))
+  if (__glibc_unlikely (libgcc_s_handle == NULL))
     pthread_cancel_init ();
   else
     atomic_read_barrier ();
+}
+
+void
+_Unwind_Resume (struct _Unwind_Exception *exc)
+{
+  _maybe_pthread_cancel_init ();
 
   void (*resume) (struct _Unwind_Exception *exc) = libgcc_s_resume;
   PTR_DEMANGLE (resume);
@@ -109,10 +122,7 @@ __gcc_personality_v0 (int version, _Unwind_Action actions,
 		      struct _Unwind_Exception *ue_header,
 		      struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_handle == NULL, 0))
-    pthread_cancel_init ();
-  else
-    atomic_read_barrier ();
+  _maybe_pthread_cancel_init ();
 
   _Unwind_Reason_Code (*personality)
     (int, _Unwind_Action, _Unwind_Exception_Class, struct _Unwind_Exception *,
@@ -125,10 +135,7 @@ _Unwind_Reason_Code
 _Unwind_ForcedUnwind (struct _Unwind_Exception *exc, _Unwind_Stop_Fn stop,
 		      void *stop_argument)
 {
-  if (__builtin_expect (libgcc_s_handle == NULL, 0))
-    pthread_cancel_init ();
-  else
-    atomic_read_barrier ();
+  _maybe_pthread_cancel_init ();
 
   _Unwind_Reason_Code (*forcedunwind)
     (struct _Unwind_Exception *, _Unwind_Stop_Fn, void *)
@@ -140,10 +147,7 @@ _Unwind_ForcedUnwind (struct _Unwind_Exception *exc, _Unwind_Stop_Fn stop,
 _Unwind_Word
 _Unwind_GetCFA (struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_handle == NULL, 0))
-    pthread_cancel_init ();
-  else
-    atomic_read_barrier ();
+  _maybe_pthread_cancel_init ();
 
   _Unwind_Word (*getcfa) (struct _Unwind_Context *) = libgcc_s_getcfa;
   PTR_DEMANGLE (getcfa);
diff --git a/ports/ChangeLog.arm b/ports/ChangeLog.arm
index b4fb546..f16617c 100644
--- a/ports/ChangeLog.arm
+++ b/ports/ChangeLog.arm
@@ -1,3 +1,22 @@
+2013-01-14  Jeff Law  <law@redhat.com>
+	    Carlos O'Donell  <codonell@redhat.com>
+	    Joseph Myers  <joseph@codesourcery.com>
+
+	* sysdeps/unix/sysv/linux/arm/nptl/unwind-forcedunwind.c
+	(pthread_cancel_init):  Add missing call to atomic_read_barrier
+	in early return path.  Improve comments.
+	(_Unwind_Resume): Add missing barrier.  Check libgcc_s_handle to
+	determine initialization complete.
+	(_maybe_pthread_cancel_init): New function.
+	(__gcc_personality_v0): Use it.
+	(_Unwind_ForcedUnwind, _Unwind_GetCFA): Likewise.
+	* sysdeps/unix/sysv/linux/arm/nptl/unwind-resume.c:
+	(init): Add missing write barrier and signal initialization
+	complete via writing libgcc_s_handle.
+	(_Unwind_Resume): Add missing barrier.  Check libgcc_s_handle to
+	determine initialization complete.
+	(__gcc_personality_v0): Use _maybe_init.
+
 2013-02-04  Joseph Myers  <joseph@codesourcery.com>
 
 	[BZ #13550]
diff --git a/ports/ChangeLog.ia64 b/ports/ChangeLog.ia64
index 3b00daf..8282eeb 100644
--- a/ports/ChangeLog.ia64
+++ b/ports/ChangeLog.ia64
@@ -1,3 +1,10 @@
+2013-01-14  Jeff Law  <law@redhat.com>
+	    Carlos O'Donell  <codonell@redhat.com>
+	    Joseph Myers  <joseph@codesourcery.com>
+
+        * sysdeps/unix/sysv/linux/ia64/nptl/unwind-forcedunwind.c
+        (_Unwind_GetBSP): Use _maybe_pthread_cancel_init.
+
 2013-02-04  Joseph Myers  <joseph@codesourcery.com>
 
 	[BZ #13550]
diff --git a/ports/sysdeps/arm/bits/atomic.h b/ports/sysdeps/arm/bits/atomic.h
index 6e20df7..e2660b1 100644
--- a/ports/sysdeps/arm/bits/atomic.h
+++ b/ports/sysdeps/arm/bits/atomic.h
@@ -42,6 +42,7 @@ void __arm_link_error (void);
 # define atomic_full_barrier() __sync_synchronize ()
 #else
 # define atomic_full_barrier() __arm_assisted_full_barrier ()
+# define atomic_asm_full_barrier() __arm_asm_assisted_full_barrier ()
 #endif
 
 /* An OS-specific bits/atomic.h file will define this macro if
diff --git a/ports/sysdeps/unix/sysv/linux/arm/bits/atomic.h b/ports/sysdeps/unix/sysv/linux/arm/bits/atomic.h
index c76b8f3..88110c0 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/bits/atomic.h
+++ b/ports/sysdeps/unix/sysv/linux/arm/bits/atomic.h
@@ -17,7 +17,8 @@
    <http://www.gnu.org/licenses/>.  */
 
 /* If the compiler doesn't provide a primitive, we'll use this macro
-   to get assistance from the kernel.  */
+   to get assistance from the kernel. This should call the
+   __kuser_memory_barrier helper in the kernel.  */
 #ifdef __thumb2__
 # define __arm_assisted_full_barrier() \
      __asm__ __volatile__						      \
@@ -25,6 +26,11 @@
 	      "movt\tip, #0xffff\n\t"					      \
 	      "blx\tip"							      \
 	      : : : "ip", "lr", "cc", "memory");
+/* The asm variant is used as an insert into existing asm statements.  */
+# define __arm_asm_assisted_full_barrier() \
+       "       movw    ip, #0x0fa0\n" \
+       "       movt    ip, #0xffff\n" \
+       "       blx     ip"
 #else
 # define __arm_assisted_full_barrier() \
      __asm__ __volatile__						      \
@@ -32,6 +38,11 @@
 	      "mov\tlr, pc\n\t"						      \
 	      "add\tpc, ip, #(0xffff0fa0 - 0xffff0fff)"			      \
 	      : : : "ip", "lr", "cc", "memory");
+/* The asm variant is used as an insert into existing asm statements.  */
+# define __arm_asm_assisted_full_barrier() \
+       "       mov     ip, #0xffff0fff\n" \
+       "       mov     lr, pc\n" \
+       "       add     pc, ip, #(0xffff0fa0 - 0xffff0fff)"
 #endif
 
 /* Atomic compare and exchange.  This sequence relies on the kernel to
diff --git a/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-forcedunwind.c b/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-forcedunwind.c
index 58ca9ac..9b5e88b 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-forcedunwind.c
+++ b/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-forcedunwind.c
@@ -36,10 +36,12 @@ pthread_cancel_init (void)
   void *resume, *personality, *forcedunwind, *getcfa;
   void *handle;
 
-  if (__builtin_expect (libgcc_s_handle != NULL, 1))
+  if (glibc_likely (libgcc_s_handle != NULL))
     {
-      /* Force gcc to reload all values.  */
-      asm volatile ("" ::: "memory");
+     /* Order reads so as to prevent speculation of loads
+       of libgcc_s_{resume,personality,forcedunwind,getcfa}
+       to points prior to the write barrier.  */
+      atomic_read_barrier ();
       return;
     }
 
@@ -61,9 +63,14 @@ pthread_cancel_init (void)
   libgcc_s_personality = personality;
   libgcc_s_forcedunwind = forcedunwind;
   libgcc_s_getcfa = getcfa;
-  /* Make sure libgcc_s_getcfa is written last.  Otherwise,
-     pthread_cancel_init might return early even when the pointer the
-     caller is interested in is not initialized yet.  */
+/* At the point at which any thread writes the handle
+   to libgcc_s_handle, the initialization is complete.
+   The writing of libgcc_s_handle is atomic. All other
+   threads reading libgcc_s_handle do so atomically. Any
+   thread that does not execute this function must issue
+   a read barrier to ensure that all of the above has
+   actually completed and that the values of the
+   function pointers are correct.   */
   atomic_write_barrier ();
   libgcc_s_handle = handle;
 }
@@ -104,7 +111,12 @@ asm (
 "	mov	r6, r0\n"
 "	cmp	r3, #0\n"
 "	beq	4f\n"
-"5:	mov	r0, r6\n"
+	atomic_asm_full_barrier()
+"5:	ldr     r4, 6f\n"
+"	ldr     r5, 7f\n"
+"8:	add     r4, pc, r4\n"
+"	ldr     r3, [r4, r5]\n"
+"	mov     r0, r6\n"
 "	ldmfd	sp!, {r4, r5, r6, lr}\n"
 "	" CFI_ADJUST_CFA_OFFSET (-16) "\n"
 "	" CFI_RESTORE (r4) "\n"
@@ -123,18 +135,32 @@ asm (
 #else
 "1:	.word	_GLOBAL_OFFSET_TABLE_ - 3b - 8\n"
 #endif
-"2:	.word	libgcc_s_resume(GOTOFF)\n"
+"2:	.word   libgcc_s_handle(GOTOFF)\n"
+#ifdef __thumb2__
+"6:	.word   _GLOBAL_OFFSET_TABLE_ - 8b - 4\n"
+#else
+"6:	.word   _GLOBAL_OFFSET_TABLE_ - 8b - 8\n"
+#endif
+"7:	.word   libgcc_s_resume(GOTOFF)\n"
 "	.size	_Unwind_Resume, .-_Unwind_Resume\n"
 );
 
+static __always_inline void
+_maybe_pthread_cancel_init ()
+{
+  if (__glibc_unlikely (libgcc_s_handle == NULL, 0))
+    pthread_cancel_init ();
+  else
+    atomic_read_barrier ();
+
+}
+
 _Unwind_Reason_Code
 __gcc_personality_v0 (_Unwind_State state,
 		      struct _Unwind_Exception *ue_header,
 		      struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_personality == NULL, 0))
-    pthread_cancel_init ();
-
+  _maybe_pthread_cancel_init ();
   return libgcc_s_personality (state, ue_header, context);
 }
 
@@ -142,17 +168,13 @@ _Unwind_Reason_Code
 _Unwind_ForcedUnwind (struct _Unwind_Exception *exc, _Unwind_Stop_Fn stop,
 		      void *stop_argument)
 {
-  if (__builtin_expect (libgcc_s_forcedunwind == NULL, 0))
-    pthread_cancel_init ();
-
+  _maybe_pthread_cancel_init ();
   return libgcc_s_forcedunwind (exc, stop, stop_argument);
 }
 
 _Unwind_Word
 _Unwind_GetCFA (struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_getcfa == NULL, 0))
-    pthread_cancel_init ();
-
+  _maybe_pthread_cancel_init ();
   return libgcc_s_getcfa (context);
 }
diff --git a/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-resume.c b/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-resume.c
index 0a3ad95..aedded4 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-resume.c
+++ b/ports/sysdeps/unix/sysv/linux/arm/nptl/unwind-resume.c
@@ -20,6 +20,7 @@
 #include <stdio.h>
 #include <unwind.h>
 
+static void *libgcc_s_handle;
 static void (*libgcc_s_resume) (struct _Unwind_Exception *exc);
 static _Unwind_Reason_Code (*libgcc_s_personality)
   (_Unwind_State, struct _Unwind_Exception *, struct _Unwind_Context *);
@@ -41,6 +42,16 @@ init (void)
 
   libgcc_s_resume = resume;
   libgcc_s_personality = personality;
+  atomic_write_barrier ();
+  /* At the point at which any thread writes the handle
+     to libgcc_s_handle, the initialization is complete.
+     The writing of libgcc_s_handle is atomic. All other
+     threads reading libgcc_s_handle do so atomically. Any
+     thread that does not execute this function must issue
+     a read barrier to ensure that all of the above has
+     actually completed and that the values of the
+     function pointers are correct.   */
+  libgcc_s_handle = handle;
 }
 
 /* It's vitally important that _Unwind_Resume not have a stack frame; the
@@ -67,7 +78,12 @@ asm (
 "	mov	r6, r0\n"
 "	cmp	r3, #0\n"
 "	beq	4f\n"
-"5:	mov	r0, r6\n"
+	atomic_asm_full_barrier()
+"5:	ldr     r4, 6f\n"
+"	ldr     r5, 7f\n"
+"8:	add     r4, pc, r4\n"
+"	ldr     r3, [r4, r5]\n"
+"	mov     r0, r6\n"
 "	ldmfd	sp!, {r4, r5, r6, lr}\n"
 "	" CFI_ADJUST_CFA_OFFSET (-16) "\n"
 "	" CFI_RESTORE (r4) "\n"
@@ -86,16 +102,30 @@ asm (
 #else
 "1:	.word	_GLOBAL_OFFSET_TABLE_ - 3b - 8\n"
 #endif
-"2:	.word	libgcc_s_resume(GOTOFF)\n"
+"2:	.word	libgcc_s_handle(GOTOFF)\n"
+#ifdef __thumb2__
+"6:	.word   _GLOBAL_OFFSET_TABLE_ - 8b - 4\n"
+#else
+"6:	.word   _GLOBAL_OFFSET_TABLE_ - 8b - 8\n"
+#endif
+"7:	.word   libgcc_s_resume(GOTOFF)\n"
 "	.size	_Unwind_Resume, .-_Unwind_Resume\n"
 );
 
+static __always_inline void
+_maybe_init (void)
+{
+  if (__glibc_unlikely (libgcc_s_handle == NULL))
+    init ();
+  else
+    atomic_read_barrier ();
+}
+
 _Unwind_Reason_Code
 __gcc_personality_v0 (_Unwind_State state,
 		      struct _Unwind_Exception *ue_header,
 		      struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_personality == NULL, 0))
-    init ();
+  _maybe_init ();
   return libgcc_s_personality (state, ue_header, context);
 }
diff --git a/ports/sysdeps/unix/sysv/linux/ia64/nptl/unwind-forcedunwind.c b/ports/sysdeps/unix/sysv/linux/ia64/nptl/unwind-forcedunwind.c
index 8562afd..94e6138 100644
--- a/ports/sysdeps/unix/sysv/linux/ia64/nptl/unwind-forcedunwind.c
+++ b/ports/sysdeps/unix/sysv/linux/ia64/nptl/unwind-forcedunwind.c
@@ -31,8 +31,7 @@ static _Unwind_Word (*libgcc_s_getbsp) (struct _Unwind_Context *);
 _Unwind_Word
 _Unwind_GetBSP (struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_getbsp == NULL, 0))
-    pthread_cancel_init ();
+  _maybe_pthread_cancel_init ();
 
   return libgcc_s_getbsp (context);
 }
diff --git a/sysdeps/gnu/unwind-resume.c b/sysdeps/gnu/unwind-resume.c
index df845cd..7ae885f 100644
--- a/sysdeps/gnu/unwind-resume.c
+++ b/sysdeps/gnu/unwind-resume.c
@@ -19,8 +19,11 @@
 #include <dlfcn.h>
 #include <stdio.h>
 #include <unwind.h>
+#include <pthreadP.h>
+#include <sysdep.h>
 #include <gnu/lib-names.h>
 
+static void *libgcc_s_handle;
 static void (*libgcc_s_resume) (struct _Unwind_Exception *exc);
 static _Unwind_Reason_Code (*libgcc_s_personality)
   (int, _Unwind_Action, _Unwind_Exception_Class, struct _Unwind_Exception *,
@@ -41,13 +44,32 @@ init (void)
 
   libgcc_s_resume = resume;
   libgcc_s_personality = personality;
+  atomic_write_barrier ();
+  /* At the point at which any thread writes the handle
+     to libgcc_s_handle, the initialization is complete.
+     The writing of libgcc_s_handle is atomic. All other
+     threads reading libgcc_s_handle do so atomically. Any
+     thread that does not execute this function must issue
+     a read barrier to ensure that all of the above has
+     actually completed and that the values of the
+     function pointers are correct.   */
+  libgcc_s_handle = handle;
 }
 
+static __always_inline void
+_maybe_init (void)
+{
+  if (__glibc_unlikely (libgcc_s_handle == NULL))
+    init ();
+  else
+    atomic_read_barrier ();
+}
+
+
 void
 _Unwind_Resume (struct _Unwind_Exception *exc)
 {
-  if (__builtin_expect (libgcc_s_resume == NULL, 0))
-    init ();
+  _maybe_init ();
   libgcc_s_resume (exc);
 }
 
@@ -57,8 +79,7 @@ __gcc_personality_v0 (int version, _Unwind_Action actions,
                       struct _Unwind_Exception *ue_header,
                       struct _Unwind_Context *context)
 {
-  if (__builtin_expect (libgcc_s_personality == NULL, 0))
-    init ();
+  _maybe_init ();
   return libgcc_s_personality (version, actions, exception_class,
 			       ue_header, context);
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]