[PATCH,nptl] Fix lock elision of pthread_mutex_trylock vs unlock

[Samuel Thibault <samuel dot thibault at ens-lyon dot org>]

Hello,

On hardware with RTM, the following crashes:

#include <pthread.h>
pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
int main(int argc, char *argv[]) {
	pthread_mutex_trylock(&m);
	pthread_mutex_unlock(&m);
	if (pthread_mutex_destroy(&m))
		*(int*)0 = 0;
	return 0;
}

The state of the mutex is indeed:

(gdb) p/x m
$1 = {__data = {__lock = 0x0, __count = 0x0, __owner = 0x0, __nusers = 0xffffffff, __kind = 0x0, 
    __spins = 0x0, __elision = 0x3, __list = {__prev = 0x0, __next = 0x0}}, __size = {0x0 <repeats 12 times>, 
    0xff, 0xff, 0xff, 0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x3, 0x0 <repeats 17 times>}, __align = 0x0}

What happens is that pthread_mutex_trylock does elision (DO_ELISION),
but pthread_mutex_unlock is not aware that trylock did, and doesn't do
elision, and thus erroneously does __nusers-- and pthread_mutex_destroy
returns EBUSY.

pthread_mutex_trylock.c mentions early in the file that we "don't force
elision in trylock, because this can lead to inconsistent lock state if
the lock was actually busy.". I don't know the details, but if trylock
should really not do elision, then it shouldn't do it :) The following
patch does this, and it indeed fixes the issue.

--- a/nptl/pthread_mutex_trylock.c
+++ b/nptl/pthread_mutex_trylock.c
@@ -77,9 +77,6 @@ __pthread_mutex_trylock (mutex)
       return 0;
 
     case PTHREAD_MUTEX_TIMED_NP:
-      if (DO_ELISION (mutex))
-	goto elision;
-      /*FALL THROUGH*/
     case PTHREAD_MUTEX_ADAPTIVE_NP:
     case PTHREAD_MUTEX_ERRORCHECK_NP:
       if (lll_trylock (mutex->__data.__lock) != 0)

But perhaps this case is actually safe (I haven't investigated the
details) and thus it's the unlock part which needs fixed, as the
following patch does:

Andy?

Samuel

diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index 95ae933..299877b 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -27,6 +27,10 @@
 #define lll_unlock_elision(a,b) ({ lll_unlock (a,b); 0; })
 #endif
 
+#ifndef DO_ELISION
+#define DO_ELISION(m) 0
+#endif
+
 static int
 internal_function
 __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
@@ -44,7 +48,7 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
     return __pthread_mutex_unlock_full (mutex, decr);
 
   if (__builtin_expect (type, PTHREAD_MUTEX_TIMED_NP)
-      == PTHREAD_MUTEX_TIMED_NP)
+      == PTHREAD_MUTEX_TIMED_NP && !DO_ELISION(mutex))
     {
       /* Always reset the owner field.  */
     normal:
@@ -60,7 +64,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
 
       return 0;
     }
-  else if (__glibc_likely (type == PTHREAD_MUTEX_TIMED_ELISION_NP))
+  else if (__glibc_likely (type == PTHREAD_MUTEX_TIMED_ELISION_NP) ||
+           (__glibc_likely (type == PTHREAD_MUTEX_TIMED_NP) &&
+		  DO_ELISION(mutex)))
     {
       /* Don't reset the owner/users fields for elision.  */
       return lll_unlock_elision (mutex->__data.__lock,
diff --git a/sysdeps/unix/sysv/linux/s390/pthread_mutex_unlock.c b/sysdeps/unix/sysv/linux/s390/pthread_mutex_unlock.c
new file mode 100644
index 0000000..048dd5d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/pthread_mutex_unlock.c
@@ -0,0 +1,22 @@
+/* Elided version of pthread_mutex_trylock.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <elision-conf.h>
+#include <force-elision.h>
+
+#include <nptl/pthread_mutex_unlock.c>
diff --git a/sysdeps/unix/sysv/linux/x86/pthread_mutex_unlock.c b/sysdeps/unix/sysv/linux/x86/pthread_mutex_unlock.c
new file mode 100644
index 0000000..80b594c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/pthread_mutex_unlock.c
@@ -0,0 +1,22 @@
+/* Elided version of pthread_mutex_trylock.
+   Copyright (C) 2011-2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <elision-conf.h>
+#include "force-elision.h"
+
+#include "nptl/pthread_mutex_unlock.c"