about summary refs log tree commit diff
path: root/sysdeps/nptl
diff options
context:
space:
mode:
authorTorvald Riegel <triegel@redhat.com>2016-12-22 10:20:43 +0100
committerTorvald Riegel <triegel@redhat.com>2017-01-13 17:16:07 +0100
commit65810f0ef05e8c9e333f17a44e77808b163ca298 (patch)
tree09928382f6015251c3b351650204f65fc8d6ec21 /sysdeps/nptl
parentf32941d80c7f532031061f8dd4704fab9c275cfe (diff)
downloadglibc-65810f0ef05e8c9e333f17a44e77808b163ca298.tar.gz
glibc-65810f0ef05e8c9e333f17a44e77808b163ca298.tar.xz
glibc-65810f0ef05e8c9e333f17a44e77808b163ca298.zip
robust mutexes: Fix broken x86 assembly by removing it
lll_robust_unlock on i386 and x86_64 first sets the futex word to
FUTEX_WAITERS|0 before calling __lll_unlock_wake, which will set the
futex word to 0.  If the thread is killed between these steps, then the
futex word will be FUTEX_WAITERS|0, and the kernel (at least current
upstream) will not set it to FUTEX_OWNER_DIED|FUTEX_WAITERS because 0 is
not equal to the TID of the crashed thread.

The lll_robust_lock assembly code on i386 and x86_64 is not prepared to
deal with this case because the fastpath tries to only CAS 0 to TID and
not FUTEX_WAITERS|0 to TID; the slowpath simply waits until it can CAS 0
to TID or the futex_word has the FUTEX_OWNER_DIED bit set.

This issue is fixed by removing the custom x86 assembly code and using
the generic C code instead.  However, instead of adding more duplicate
code to the custom x86 lowlevellock.h, the code of the lll_robust* functions
is inlined into the single call sites that exist for each of these functions
in the pthread_mutex_* functions.  The robust mutex paths in the latter
have been slightly reorganized to make them simpler.

This patch is meant to be easy to backport, so C11-style atomics are not
used.

	[BZ #20985]
	* nptl/Makefile: Adapt.
	* nptl/pthread_mutex_cond_lock.c (LLL_ROBUST_MUTEX_LOCK): Remove.
	(LLL_ROBUST_MUTEX_LOCK_MODIFIER): New.
	* nptl/pthread_mutex_lock.c (LLL_ROBUST_MUTEX_LOCK): Remove.
	(LLL_ROBUST_MUTEX_LOCK_MODIFIER): New.
	(__pthread_mutex_lock_full): Inline lll_robust* functions and adapt.
	* nptl/pthread_mutex_timedlock.c (pthread_mutex_timedlock): Inline
	lll_robust* functions and adapt.
	* nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_full): Likewise.
	* sysdeps/nptl/lowlevellock.h (__lll_robust_lock_wait,
	__lll_robust_lock, lll_robust_cond_lock, __lll_robust_timedlock_wait,
	__lll_robust_timedlock, __lll_robust_unlock): Remove.
	* sysdeps/unix/sysv/linux/i386/lowlevellock.h (lll_robust_lock,
	lll_robust_cond_lock, lll_robust_timedlock, lll_robust_unlock): Remove.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_robust_lock,
	lll_robust_cond_lock, lll_robust_timedlock, lll_robust_unlock): Remove.
	* sysdeps/unix/sysv/linux/sparc/lowlevellock.h (__lll_robust_lock_wait,
	__lll_robust_lock, lll_robust_cond_lock, __lll_robust_timedlock_wait,
	__lll_robust_timedlock, __lll_robust_unlock): Remove.
	* nptl/lowlevelrobustlock.c: Remove file.
	* nptl/lowlevelrobustlock.sym: Likewise.
	* sysdeps/unix/sysv/linux/i386/lowlevelrobustlock.S: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevelrobustlock.S: Likewise.
Diffstat (limited to 'sysdeps/nptl')
-rw-r--r--sysdeps/nptl/lowlevellock.h68
1 files changed, 0 insertions, 68 deletions
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index 36a76fab08..42d9658d9d 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -74,7 +74,6 @@
 
 extern void __lll_lock_wait_private (int *futex) attribute_hidden;
 extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
-extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
 
 /* This is an expression rather than a statement even though its value is
    void, so that it can be used in a comma expression or as an expression
@@ -103,28 +102,6 @@ extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
   __lll_lock (&(futex), private)
 
 
-/* If FUTEX is 0 (not acquired), set to ID (acquired with no waiters) and
-   return 0.  Otherwise, ensure that it is set to FUTEX | FUTEX_WAITERS
-   (acquired, possibly with waiters) and block until we acquire the lock.
-   FUTEX will now be ID | FUTEX_WAITERS and we return 0.
-   If the previous owner of the lock dies before we acquire the lock then FUTEX
-   will be the value of id as set by the previous owner, with FUTEX_OWNER_DIED
-   set (FUTEX_WAITERS may or may not be set).  We return this value to indicate
-   that the lock is not acquired.  */
-#define __lll_robust_lock(futex, id, private)                           \
-  ({                                                                    \
-    int *__futex = (futex);                                             \
-    int __val = 0;                                                      \
-                                                                        \
-    if (__glibc_unlikely                                                \
-        (atomic_compare_and_exchange_bool_acq (__futex, id, 0)))        \
-      __val = __lll_robust_lock_wait (__futex, private);                \
-    __val;                                                              \
-  })
-#define lll_robust_lock(futex, id, private)     \
-  __lll_robust_lock (&(futex), id, private)
-
-
 /* This is an expression rather than a statement even though its value is
    void, so that it can be used in a comma expression or as an expression
    that's cast to void.  */
@@ -142,16 +119,8 @@ extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
 #define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
 
 
-/* As __lll_robust_lock, but set to ID | FUTEX_WAITERS (acquired, possibly with
-   waiters) if FUTEX is 0.  */
-#define lll_robust_cond_lock(futex, id, private)	\
-  __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
-
-
 extern int __lll_timedlock_wait (int *futex, const struct timespec *,
 				 int private) attribute_hidden;
-extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
-					int private) attribute_hidden;
 
 
 /* As __lll_lock, but with a timeout.  If the timeout occurs then return
@@ -170,22 +139,6 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
   __lll_timedlock (&(futex), abstime, private)
 
 
-/* As __lll_robust_lock, but with a timeout.  If the timeout occurs then return
-   ETIMEDOUT.  If ABSTIME is invalid, return EINVAL.  */
-#define __lll_robust_timedlock(futex, abstime, id, private)             \
-  ({                                                                    \
-    int *__futex = (futex);                                             \
-    int __val = 0;                                                      \
-                                                                        \
-    if (__glibc_unlikely                                                \
-        (atomic_compare_and_exchange_bool_acq (__futex, id, 0)))        \
-      __val = __lll_robust_timedlock_wait (__futex, abstime, private);  \
-    __val;                                                              \
-  })
-#define lll_robust_timedlock(futex, abstime, id, private)       \
-  __lll_robust_timedlock (&(futex), abstime, id, private)
-
-
 /* This is an expression rather than a statement even though its value is
    void, so that it can be used in a comma expression or as an expression
    that's cast to void.  */
@@ -211,27 +164,6 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
   __lll_unlock (&(futex), private)
 
 
-/* This is an expression rather than a statement even though its value is
-   void, so that it can be used in a comma expression or as an expression
-   that's cast to void.  */
-/* Unconditionally set FUTEX to 0 (not acquired), releasing the lock.  If FUTEX
-   had FUTEX_WAITERS set then wake any waiters.  The waiter that acquires the
-   lock will set FUTEX_WAITERS.
-   Evaluate PRIVATE before releasing the lock so that we do not violate the
-   mutex destruction requirements (see __lll_unlock).  */
-#define __lll_robust_unlock(futex, private)             \
-  ((void)                                               \
-   ({                                                   \
-     int *__futex = (futex);                            \
-     int __private = (private);                         \
-     int __oldval = atomic_exchange_rel (__futex, 0);   \
-     if (__glibc_unlikely (__oldval & FUTEX_WAITERS))	\
-       lll_futex_wake (__futex, 1, __private);          \
-   }))
-#define lll_robust_unlock(futex, private)       \
-  __lll_robust_unlock (&(futex), private)
-
-
 #define lll_islocked(futex) \
   ((futex) != LLL_LOCK_INITIALIZER)