This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH]: Fix blocking pthread_join.


Hi,

On s390 (31bit) if glibc is build with -Os, pthread_join sometimes
blocks indefinitely. This is e.g. observable with
testcase intl/tst-gettext6.

pthread_join is calling lll_wait_tid(tid), which performs the futex-wait
syscall in a loop as long as tid != 0 (thread is alive).

On s390 (and build with -Os), tid is loaded from memory before
comparing against zero and then the tid is loaded a second time
in order to pass it to the futex-wait-syscall.
If the thread exits in between, then the futex-wait-syscall is
called with the value zero and it waits until a futex-wake occurs.
As the thread is already exited, there won't be a futex-wake.

In lll_wait_tid, the tid is stored to the local variable __tid,
which is then used as argument for the futex-wait-syscall.
But unfortunately the compiler is allowed to reload the value
from memory.

With this patch, the tid is loaded by dereferencing a volatile pointer.
Then the compiler is not allowed to reload the value for __tid from memory.

Okay to commit?

Bye
Stefan

---

ChangeLog:

	* sysdeps/nptl/lowlevellock.h (lll_wait_tid):
	Use a volatile pointer to load __tid.
commit be2e80e32fa4d0c7f7d021f550d21ab102aa8c42
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Wed Apr 25 12:51:27 2018 +0200

    Fix blocking pthread_join.
    
    On s390 (31bit) if glibc is build with -Os, pthread_join sometimes
    blocks indefinitely. This is e.g. observable with
    testcase intl/tst-gettext6.
    
    pthread_join is calling lll_wait_tid(tid), which performs the futex-wait
    syscall in a loop as long as tid != 0 (thread is alive).
    
    On s390 (and build with -Os), tid is loaded from memory before
    comparing against zero and then the tid is loaded a second time
    in order to pass it to the futex-wait-syscall.
    If the thread exits in between, then the futex-wait-syscall is
    called with the value zero and it waits until a futex-wake occurs.
    As the thread is already exited, there won't be a futex-wake.
    
    In lll_wait_tid, the tid is stored to the local variable __tid,
    which is then used as argument for the futex-wait-syscall.
    But unfortunately the compiler is allowed to reload the value
    from memory.
    
    With this patch, the tid is loaded by dereferencing a volatile pointer.
    Then the compiler is not allowed to reload the value for __tid from memory.
    
    ChangeLog:
    
            * sysdeps/nptl/lowlevellock.h (lll_wait_tid):
            Use a volatile pointer to load __tid.

diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index 8326e2805c..6e06e8498a 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -184,7 +184,7 @@ extern int __lll_timedlock_wait (int *futex, const struct timespec *,
 #define lll_wait_tid(tid) \
   do {					\
     __typeof (tid) __tid;		\
-    while ((__tid = (tid)) != 0)	\
+    while ((__tid = *(volatile __typeof(tid) *) &(tid)) != 0)	\
       lll_futex_wait (&(tid), __tid, LLL_SHARED);\
   } while (0)
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]