| Submitter | VandeVondele Joost |
|---|---|
| Date | Jan. 31, 2013, 2:50 p.m. |
| Message ID | <908103EDB4893A42920B21D3568BFD9308C78C5D@MBX11.d.ethz.ch> |
| Download | mbox | patch |
| Permalink | /patch/217197/ |
| State | New |
| Headers | show |
Comments
On Thu, Jan 31, 2013 at 02:50:51PM +0000, VandeVondele Joost wrote: > The attached patch fixes a race condition in libgomp. Based on the suggestions by Dmitry, and verified that it fixes the corresponding sanitizer warnings. > > 2012-12-30 Dmitry Vyukov <dvyukov@gcc.gnu.org> Ok, but please use current date rather than 2012-12-30, and also add your name and email address below Dmitry's. Jakub
The updated changelog entry is below, but somebody with write access should do the commit, please.
2013-01-31 Dmitry Vyukov <dvyukov@gcc.gnu.org>
Joost VandeVondele <Joost.VandeVondele@mat.ethz.ch>
PR libgomp/55561
* config/linux/wait.h (do_spin): Use atomic load for addr.
* config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic
for intptr and ptrlock.
* config/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load
for ptrlock.
LGTM On Thu, Jan 31, 2013 at 8:54 PM, VandeVondele Joost <joost.vandevondele@mat.ethz.ch> wrote: > The updated changelog entry is below, but somebody with write access should do the commit, please. > > 2013-01-31 Dmitry Vyukov <dvyukov@gcc.gnu.org> > Joost VandeVondele <Joost.VandeVondele@mat.ethz.ch> > > PR libgomp/55561 > * config/linux/wait.h (do_spin): Use atomic load for addr. > * config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic > for intptr and ptrlock. > * config/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load > for ptrlock.
Patch
Index: libgomp/config/linux/wait.h =================================================================== --- libgomp/config/linux/wait.h (revision 194730) +++ libgomp/config/linux/wait.h (working copy) @@ -51,7 +51,7 @@ static inline int do_spin (int *addr, in if (__builtin_expect (gomp_managed_threads > gomp_available_cpus, 0)) count = gomp_throttled_spin_count_var; for (i = 0; i < count; i++) - if (__builtin_expect (*addr != val, 0)) + if (__builtin_expect (__atomic_load_n (addr, MEMMODEL_RELAXED) != val, 0)) return 0; else cpu_relax (); Index: libgomp/config/linux/ptrlock.c =================================================================== --- libgomp/config/linux/ptrlock.c (revision 194730) +++ libgomp/config/linux/ptrlock.c (working copy) @@ -50,9 +50,9 @@ gomp_ptrlock_get_slow (gomp_ptrlock_t *p #endif do do_wait (intptr, 2); - while (*intptr == 2); + while (__atomic_load_n (intptr, MEMMODEL_RELAXED) == 2); __asm volatile ("" : : : "memory"); - return *ptrlock; + return (void *) __atomic_load_n (ptrlock, MEMMODEL_ACQUIRE); } void Index: libgomp/config/linux/ptrlock.h =================================================================== --- libgomp/config/linux/ptrlock.h (revision 194730) +++ libgomp/config/linux/ptrlock.h (working copy) @@ -48,8 +48,9 @@ static inline void *gomp_ptrlock_get (go { uintptr_t oldval; - if ((uintptr_t) *ptrlock > 2) - return *ptrlock; + uintptr_t v = (uintptr_t) __atomic_load_n (ptrlock, MEMMODEL_ACQUIRE); + if (v > 2) + return (void *) v; oldval = 0; if (__atomic_compare_exchange_n (ptrlock, &oldval, 1, false,
The attached patch fixes a race condition in libgomp. Based on the suggestions by Dmitry, and verified that it fixes the corresponding sanitizer warnings. 2012-12-30 Dmitry Vyukov <dvyukov@gcc.gnu.org> PR libgomp/55561 * config/linux/wait.h (do_spin): Use atomic load for addr. * config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic for intptr and ptrlock. * onfig/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load for ptrlock.