25845f
commit 5920a4a624b1f4db310d1c44997b640e2a4653e5
25845f
Author: Carlos O'Donell <carlos@redhat.com>
25845f
Date:   Sat Jul 29 00:02:03 2017 -0400
25845f
25845f
    mutex: Fix robust mutex lock acquire (Bug 21778)
25845f
    
25845f
    65810f0ef05e8c9e333f17a44e77808b163ca298 fixed a robust mutex bug but
25845f
    introduced BZ 21778: if the CAS used to try to acquire a lock fails, the
25845f
    expected value is not updated, which breaks other cases in the loce
25845f
    acquisition loop.  The fix is to simply update the expected value with
25845f
    the value returned by the CAS, which ensures that behavior is as if the
25845f
    first case with the CAS never happened (if the CAS fails).
25845f
    
25845f
    This is a regression introduced in the last release.
25845f
    
25845f
    Tested on x86_64, i686, ppc64, ppc64le, s390x, aarch64, armv7hl.
25845f
25845f
Index: glibc-2.17-c758a686/nptl/Makefile
25845f
===================================================================
25845f
--- glibc-2.17-c758a686.orig/nptl/Makefile
25845f
+++ glibc-2.17-c758a686/nptl/Makefile
25845f
@@ -204,7 +204,7 @@ CFLAGS-tst-thread-exit-clobber.o = -std=
25845f
 tests = tst-typesizes \
25845f
 	tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
25845f
 	tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 tst-mutex5 tst-mutex6 \
25845f
-	tst-mutex7 tst-mutex8 tst-mutex9 tst-mutex5a tst-mutex7a \
25845f
+	tst-mutex7 tst-mutex8 tst-mutex9 tst-mutex5a tst-mutex7a tst-mutex7robust \
25845f
 	tst-mutexpi1 tst-mutexpi2 tst-mutexpi3 tst-mutexpi4 tst-mutexpi5 \
25845f
 	tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a tst-mutexpi8 \
25845f
 	tst-mutexpi9 \
25845f
Index: glibc-2.17-c758a686/nptl/pthread_mutex_lock.c
25845f
===================================================================
25845f
--- glibc-2.17-c758a686.orig/nptl/pthread_mutex_lock.c
25845f
+++ glibc-2.17-c758a686/nptl/pthread_mutex_lock.c
25845f
@@ -198,11 +198,14 @@ __pthread_mutex_lock_full (pthread_mutex
25845f
 	{
25845f
 	  /* Try to acquire the lock through a CAS from 0 (not acquired) to
25845f
 	     our TID | assume_other_futex_waiters.  */
25845f
-	  if (__glibc_likely ((oldval == 0)
25845f
-			      && (atomic_compare_and_exchange_bool_acq
25845f
-				  (&mutex->__data.__lock,
25845f
-				   id | assume_other_futex_waiters, 0) == 0)))
25845f
-	      break;
25845f
+	  if (__glibc_likely (oldval == 0))
25845f
+	    {
25845f
+	      oldval
25845f
+	        = atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,
25845f
+	            id | assume_other_futex_waiters, 0);
25845f
+	      if (__glibc_likely (oldval == 0))
25845f
+		break;
25845f
+	    }
25845f
 
25845f
 	  if ((oldval & FUTEX_OWNER_DIED) != 0)
25845f
 	    {
25845f
Index: glibc-2.17-c758a686/nptl/pthread_mutex_timedlock.c
25845f
===================================================================
25845f
--- glibc-2.17-c758a686.orig/nptl/pthread_mutex_timedlock.c
25845f
+++ glibc-2.17-c758a686/nptl/pthread_mutex_timedlock.c
25845f
@@ -154,11 +154,14 @@ pthread_mutex_timedlock (pthread_mutex_t
25845f
 	{
25845f
 	  /* Try to acquire the lock through a CAS from 0 (not acquired) to
25845f
 	     our TID | assume_other_futex_waiters.  */
25845f
-	  if (__glibc_likely ((oldval == 0)
25845f
-			      && (atomic_compare_and_exchange_bool_acq
25845f
-				  (&mutex->__data.__lock,
25845f
-				   id | assume_other_futex_waiters, 0) == 0)))
25845f
-	      break;
25845f
+	  if (__glibc_likely (oldval == 0))
25845f
+	    {
25845f
+	      oldval
25845f
+	        = atomic_compare_and_exchange_val_acq (&mutex->__data.__lock,
25845f
+	            id | assume_other_futex_waiters, 0);
25845f
+	      if (__glibc_likely (oldval == 0))
25845f
+		break;
25845f
+	    }
25845f
 
25845f
 	  if ((oldval & FUTEX_OWNER_DIED) != 0)
25845f
 	    {
25845f
Index: glibc-2.17-c758a686/nptl/tst-mutex7.c
25845f
===================================================================
25845f
--- glibc-2.17-c758a686.orig/nptl/tst-mutex7.c
25845f
+++ glibc-2.17-c758a686/nptl/tst-mutex7.c
25845f
@@ -22,25 +22,41 @@
25845f
 #include <stdlib.h>
25845f
 #include <time.h>
25845f
 
25845f
-
25845f
+/* This test is a template for other tests to use.  Other tests define
25845f
+   the following macros to change the behaviour of the template test.
25845f
+   The test is very simple, it configures N threads given the parameters
25845f
+   below and then proceeds to go through mutex lock and unlock
25845f
+   operations in each thread as described before for the thread
25845f
+   function.  */
25845f
 #ifndef TYPE
25845f
 # define TYPE PTHREAD_MUTEX_DEFAULT
25845f
 #endif
25845f
-
25845f
+#ifndef ROBUST
25845f
+# define ROBUST PTHREAD_MUTEX_STALLED
25845f
+#endif
25845f
+#ifndef DELAY_NSEC
25845f
+# define DELAY_NSEC 11000
25845f
+#endif
25845f
+#ifndef ROUNDS
25845f
+# define ROUNDS 1000
25845f
+#endif
25845f
+#ifndef N
25845f
+# define N 100
25845f
+#endif
25845f
 
25845f
 static pthread_mutex_t lock;
25845f
 
25845f
-
25845f
-#define ROUNDS 1000
25845f
-#define N 100
25845f
-
25845f
-
25845f
+/* Each thread locks and the subsequently unlocks the lock, yielding
25845f
+   the smallest critical section possible.  After the unlock the thread
25845f
+   waits DELAY_NSEC nanoseconds before doing the lock and unlock again.
25845f
+   Every thread does this ROUNDS times.  The lock and unlock are
25845f
+   checked for errors.  */
25845f
 static void *
25845f
 tf (void *arg)
25845f
 {
25845f
   int nr = (long int) arg;
25845f
   int cnt;
25845f
-  struct timespec ts = { .tv_sec = 0, .tv_nsec = 11000 };
25845f
+  struct timespec ts = { .tv_sec = 0, .tv_nsec = DELAY_NSEC };
25845f
 
25845f
   for (cnt = 0; cnt < ROUNDS; ++cnt)
25845f
     {
25845f
@@ -56,13 +72,16 @@ tf (void *arg)
25845f
 	  return (void *) 1l;
25845f
 	}
25845f
 
25845f
-      nanosleep (&ts, NULL);
25845f
+      if ((ts.tv_sec > 0) || (ts.tv_nsec > 0))
25845f
+	nanosleep (&ts, NULL);
25845f
     }
25845f
 
25845f
   return NULL;
25845f
 }
25845f
 
25845f
-
25845f
+/* Setup and run N threads, where each thread does as described
25845f
+   in the above thread function.  The threads are given a minimal 1MiB
25845f
+   stack since they don't do anything between the lock and unlock.  */
25845f
 static int
25845f
 do_test (void)
25845f
 {
25845f
@@ -80,6 +99,12 @@ do_test (void)
25845f
       exit (1);
25845f
     }
25845f
 
25845f
+  if (pthread_mutexattr_setrobust (&a, ROBUST) != 0)
25845f
+    {
25845f
+      puts ("mutexattr_setrobust failed");
25845f
+      exit (1);
25845f
+    }
25845f
+
25845f
 #ifdef ENABLE_PI
25845f
   if (pthread_mutexattr_setprotocol (&a, PTHREAD_PRIO_INHERIT) != 0)
25845f
     {
25845f
Index: glibc-2.17-c758a686/nptl/tst-mutex7robust.c
25845f
===================================================================
25845f
--- /dev/null
25845f
+++ glibc-2.17-c758a686/nptl/tst-mutex7robust.c
25845f
@@ -0,0 +1,7 @@
25845f
+/* Bug 21778: Fix oversight in robust mutex lock acquisition.  */
25845f
+#define TYPE PTHREAD_MUTEX_NORMAL
25845f
+#define ROBUST PTHREAD_MUTEX_ROBUST
25845f
+#define DELAY_NSEC 0
25845f
+#define ROUNDS 1000
25845f
+#define N 32
25845f
+#include "tst-mutex7.c"