Patchwork Fix for PR55561 race condition in libgomp

login
register
mail settings
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

VandeVondele Joost - Jan. 31, 2013, 2:50 p.m.
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.
Jakub Jelinek - Jan. 31, 2013, 2:58 p.m.
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
VandeVondele Joost - Jan. 31, 2013, 4:54 p.m.
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.
Dmitriy Vyukov - Feb. 1, 2013, 7:59 p.m.
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,