This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[Patch] Memory fencing fixes
- From: Jeff Law <law at redhat dot com>
- To: libc-alpha <libc-alpha at sourceware dot org>
- Date: Mon, 04 Feb 2013 14:39:05 -0700
- Subject: [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);
}