* [PATCH 6/7] nptl: Cleanup cancellation macros
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 5/7] i386: Remove bogus THREAD_ATOMIC_* macros Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 2/7] nptl: Remove tst-cancel-wrappers test and related macros Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 7/7] support: Add support_create_temp_fifo Adhemerval Zanella
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
This patch wraps all uses of *_{enable,disable}_asynccancel and
and *_CANCEL_{ASYNC,RESET} in either already provided macros
(lll_futex_timed_wait_cancel) or creates new ones if the
functionality is not provided (SYSCALL_CANCEL_NCS, lll_futex_wait_cancel,
and lll_futex_timed_wait_cancel).
Also for some generic implementations, the direct call of the macros
are removed since the underlying symbols are suppose to provide
cancellation support.
This is a priliminary patch intended to simplify the work required
for BZ#12683 fix. It is a refactor change, no semantic changes are
expected.
Checked on x86_64-linux-gnu and i686-linux-gnu.
* nptl/pthread_join_common.c (__pthread_timedjoin_ex): Use
lll_wait_tid with timeout.
* nptl/sem_wait.c (__old_sem_wait): Use lll_futex_wait_cancel.
* sysdeps/nptl/aio_misc.h (AIO_MISC_WAIT): Use
futex_reltimed_wait_cancelable for cancelabla mode.
* sysdeps/nptl/gai_misc.h (GAI_MISC_WAIT): Likewise.
* sysdeps/posix/open64.c (__libc_open64): Do not call cancelation
macros.
* sysdeps/posix/sigwait.c (__sigwait): Likewise.
* sysdeps/posix/waitid.c (__sigwait): Likewise.
* sysdeps/unix/sysdep.h (__SYSCALL_CANCEL_CALL,
SYSCALL_CANCEL_NCS): New macro.
* sysdeps/nptl/lowlevellock.h (lll_wait_tid): Add timeout argument.
(lll_timedwait_tid): Remove macro.
* sysdeps/unix/sysv/linux/i386/lowlevellock.h (lll_wait_tid):
Likewise.
(lll_timedwait_tid): Likewise.
* sysdeps/unix/sysv/linux/sparc/lowlevellock.h (lll_wait_tid):
Likewise.
(lll_timedwait_tid): Likewise.
* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_wait_tid):
Likewise.
(lll_timedwait_tid): Likewise.
* sysdeps/unix/sysv/linux/clock_nanosleep.c (__clock_nanosleep):
Use INTERNAL_SYSCALL_CANCEL.
* sysdeps/unix/sysv/linux/futex-internal.h
(futex_reltimed_wait_cancelable): Use LIBC_CANCEL_{ASYNC,RESET}
instead of __pthread_{enable,disable}_asynccancel.
* sysdeps/unix/sysv/linux/lowlevellock-futex.h
(lll_futex_wait_cancel): New macro.
---
ChangeLog | 31 ++++++++++++++
nptl/pthread_join_common.c | 9 +---
nptl/sem_wait.c | 8 +---
sysdeps/nptl/aio_misc.h | 15 +++----
sysdeps/nptl/gai_misc.h | 15 +++----
sysdeps/nptl/lowlevellock.h | 40 ++++++++----------
sysdeps/posix/open64.c | 12 +-----
sysdeps/posix/sigwait.c | 12 +-----
sysdeps/posix/waitid.c | 12 +-----
sysdeps/unix/sysv/linux/clock_nanosleep.c | 23 ++++------
sysdeps/unix/sysv/linux/futex-internal.h | 4 +-
sysdeps/unix/sysv/linux/i386/lowlevellock.h | 42 +++++++++----------
sysdeps/unix/sysv/linux/lowlevellock-futex.h | 11 +++++
sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 39 ++++++++---------
sysdeps/unix/sysv/linux/x86_64/lowlevellock.h | 40 +++++++++---------
15 files changed, 148 insertions(+), 165 deletions(-)
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index 702fcd1545..63d693210d 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -81,14 +81,7 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
un-wait-ed for again. */
pthread_cleanup_push (cleanup, &pd->joinid);
- int oldtype = CANCEL_ASYNC ();
-
- if (abstime != NULL)
- result = lll_timedwait_tid (pd->tid, abstime);
- else
- lll_wait_tid (pd->tid);
-
- CANCEL_RESET (oldtype);
+ result = lll_wait_tid (pd->tid, abstime);
pthread_cleanup_pop (0);
}
diff --git a/nptl/sem_wait.c b/nptl/sem_wait.c
index e7d910613f..e057b05dd6 100644
--- a/nptl/sem_wait.c
+++ b/nptl/sem_wait.c
@@ -56,14 +56,8 @@ __old_sem_wait (sem_t *sem)
if (atomic_decrement_if_positive (futex) > 0)
return 0;
- /* Enable asynchronous cancellation. Required by the standard. */
- int oldtype = __pthread_enable_asynccancel ();
-
/* Always assume the semaphore is shared. */
- err = lll_futex_wait (futex, 0, LLL_SHARED);
-
- /* Disable asynchronous cancellation. */
- __pthread_disable_asynccancel (oldtype);
+ err = lll_futex_wait_cancel (futex, 0, LLL_SHARED);
}
while (err == 0 || err == -EWOULDBLOCK);
diff --git a/sysdeps/nptl/aio_misc.h b/sysdeps/nptl/aio_misc.h
index 206d8e193d..70f1e057b5 100644
--- a/sysdeps/nptl/aio_misc.h
+++ b/sysdeps/nptl/aio_misc.h
@@ -41,15 +41,15 @@
{ \
pthread_mutex_unlock (&__aio_requests_mutex); \
\
- int oldtype; \
- if (cancel) \
- oldtype = LIBC_CANCEL_ASYNC (); \
- \
int status; \
do \
{ \
- status = futex_reltimed_wait ((unsigned int *) futexaddr, oldval, \
- timeout, FUTEX_PRIVATE); \
+ if (cancel) \
+ status = futex_reltimed_wait_cancelable ( \
+ (unsigned int *) futexaddr, oldval, timeout, FUTEX_PRIVATE); \
+ else \
+ status = futex_reltimed_wait ((unsigned int *) futexaddr, \
+ oldval, timeout, FUTEX_PRIVATE); \
if (status != EAGAIN) \
break; \
\
@@ -57,9 +57,6 @@
} \
while (oldval != 0); \
\
- if (cancel) \
- LIBC_CANCEL_RESET (oldtype); \
- \
if (status == EINTR) \
result = EINTR; \
else if (status == ETIMEDOUT) \
diff --git a/sysdeps/nptl/gai_misc.h b/sysdeps/nptl/gai_misc.h
index 815e6c0dc6..831b43a479 100644
--- a/sysdeps/nptl/gai_misc.h
+++ b/sysdeps/nptl/gai_misc.h
@@ -42,15 +42,15 @@
{ \
pthread_mutex_unlock (&__gai_requests_mutex); \
\
- int oldtype; \
- if (cancel) \
- oldtype = LIBC_CANCEL_ASYNC (); \
- \
int status; \
do \
{ \
- status = futex_reltimed_wait ((unsigned int *) futexaddr, oldval, \
- timeout, FUTEX_PRIVATE); \
+ if (cancel) \
+ status = futex_reltimed_wait_cancelable ( \
+ (unsigned int *) futexaddr, oldval, timeout, FUTEX_PRIVATE); \
+ else \
+ status = futex_reltimed_wait ((unsigned int *) futexaddr, \
+ oldval, timeout, FUTEX_PRIVATE); \
if (status != EAGAIN) \
break; \
\
@@ -58,9 +58,6 @@
} \
while (oldval != 0); \
\
- if (cancel) \
- LIBC_CANCEL_RESET (oldtype); \
- \
if (status == EINTR) \
result = EINTR; \
else if (status == ETIMEDOUT) \
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index bfbda99940..d7837be5d4 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -175,33 +175,29 @@ extern int __lll_timedlock_wait (int *futex, const struct timespec *,
#define LLL_LOCK_INITIALIZER (0)
#define LLL_LOCK_INITIALIZER_LOCKED (1)
+extern int __lll_timedwait_tid (int *, const struct timespec *)
+ attribute_hidden;
/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
wake-up when the clone terminates. The memory location contains the
thread ID while the clone is running and is reset to zero by the kernel
afterwards. The kernel up to version 3.16.3 does not use the private futex
- operations for futex wake-up when the clone terminates. */
-#define lll_wait_tid(tid) \
- do { \
- __typeof (tid) __tid; \
- /* We need acquire MO here so that we synchronize \
- with the kernel's store to 0 when the clone \
- terminates. (see above) */ \
- while ((__tid = atomic_load_acquire (&(tid))) != 0) \
- lll_futex_wait (&(tid), __tid, LLL_SHARED); \
- } while (0)
-
-extern int __lll_timedwait_tid (int *, const struct timespec *)
- attribute_hidden;
-
-/* As lll_wait_tid, but with a timeout. If the timeout occurs then return
- ETIMEDOUT. If ABSTIME is invalid, return EINVAL. */
-#define lll_timedwait_tid(tid, abstime) \
- ({ \
- int __res = 0; \
- if ((tid) != 0) \
- __res = __lll_timedwait_tid (&(tid), (abstime)); \
- __res; \
+ operations for futex wake-up when the clone terminates.
+ If ABSTIME is not NULL, is used a timeout for futex call. If the timeout
+ occurs then return ETIMEOUT, if ABSTIME is invalid, return EINVAL.
+ The futex operation are issues with cancellable versions. */
+#define lll_wait_tid(tid, abstime) \
+ ({ \
+ int __res = 0; \
+ __typeof (tid) __tid; \
+ if (abstime != NULL) \
+ __res = __lll_timedwait_tid (&(tid), (abstime)); \
+ else \
+ /* We need acquire MO here so that we synchronize with the \
+ kernel's store to 0 when the clone terminates. (see above) */ \
+ while ((__tid = atomic_load_acquire (&(tid))) != 0) \
+ lll_futex_wait_cancel (&(tid), __tid, LLL_SHARED); \
+ __res; \
})
diff --git a/sysdeps/posix/open64.c b/sysdeps/posix/open64.c
index c4209c8cdb..e4ec5d1876 100644
--- a/sysdeps/posix/open64.c
+++ b/sysdeps/posix/open64.c
@@ -34,16 +34,8 @@ __libc_open64 (const char *file, int oflag, ...)
va_end (arg);
}
- if (SINGLE_THREAD_P)
- return __libc_open (file, oflag | O_LARGEFILE, mode);
-
- int oldtype = LIBC_CANCEL_ASYNC ();
-
- int result = __libc_open (file, oflag | O_LARGEFILE, mode);
-
- LIBC_CANCEL_RESET (oldtype);
-
- return result;
+ /* __libc_open should be a cancellation point. */
+ return __libc_open (file, oflag | O_LARGEFILE, mode);
}
weak_alias (__libc_open64, __open64)
libc_hidden_weak (__open64)
diff --git a/sysdeps/posix/sigwait.c b/sysdeps/posix/sigwait.c
index 4ff9d847d4..acc82e1965 100644
--- a/sysdeps/posix/sigwait.c
+++ b/sysdeps/posix/sigwait.c
@@ -85,16 +85,8 @@ do_sigwait (const sigset_t *set, int *sig)
int
__sigwait (const sigset_t *set, int *sig)
{
- if (SINGLE_THREAD_P)
- return do_sigwait (set, sig);
-
- int oldtype = LIBC_CANCEL_ASYNC ();
-
- int result = do_sigwait (set, sig);
-
- LIBC_CANCEL_RESET (oldtype);
-
- return result;
+ /* __sigsuspend should be a cancellation point. */
+ return do_waitid (idtype, id, infop, options);
}
libc_hidden_def (__sigwait)
weak_alias (__sigwait, sigwait)
diff --git a/sysdeps/posix/waitid.c b/sysdeps/posix/waitid.c
index 3207c742c2..98f35d078e 100644
--- a/sysdeps/posix/waitid.c
+++ b/sysdeps/posix/waitid.c
@@ -151,16 +151,8 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
int
__waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options)
{
- if (SINGLE_THREAD_P)
- return do_waitid (idtype, id, infop, options);
-
- int oldtype = LIBC_CANCEL_ASYNC ();
-
- int result = do_waitid (idtype, id, infop, options);
-
- LIBC_CANCEL_RESET (oldtype);
-
- return result;
+ /* __waitpid should be a cancellation point. */
+ return do_waitid (idtype, id, infop, options);
}
weak_alias (__waitid, waitid)
strong_alias (__waitid, __libc_waitid)
diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
index 93d5d6ef12..31229bbd85 100644
--- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
+++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
@@ -28,27 +28,18 @@ int
__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
struct timespec *rem)
{
- INTERNAL_SYSCALL_DECL (err);
- int r;
-
if (clock_id == CLOCK_THREAD_CPUTIME_ID)
return EINVAL;
if (clock_id == CLOCK_PROCESS_CPUTIME_ID)
clock_id = MAKE_PROCESS_CPUCLOCK (0, CPUCLOCK_SCHED);
- if (SINGLE_THREAD_P)
- r = INTERNAL_SYSCALL (clock_nanosleep, err, 4, clock_id, flags, req, rem);
- else
- {
- int oldstate = LIBC_CANCEL_ASYNC ();
-
- r = INTERNAL_SYSCALL (clock_nanosleep, err, 4, clock_id, flags, req,
- rem);
-
- LIBC_CANCEL_RESET (oldstate);
- }
-
+ /* If the call is interrupted by a signal handler or encounters an error,
+ it returns a positive value similar to errno. */
+ INTERNAL_SYSCALL_DECL (err);
+ int r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id, flags,
+ req, rem);
return (INTERNAL_SYSCALL_ERROR_P (r, err)
- ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
+ ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
+
}
weak_alias (__clock_nanosleep, clock_nanosleep)
diff --git a/sysdeps/unix/sysv/linux/futex-internal.h b/sysdeps/unix/sysv/linux/futex-internal.h
index 96a07b05b9..d36235a925 100644
--- a/sysdeps/unix/sysv/linux/futex-internal.h
+++ b/sysdeps/unix/sysv/linux/futex-internal.h
@@ -138,9 +138,9 @@ futex_reltimed_wait_cancelable (unsigned int *futex_word,
const struct timespec *reltime, int private)
{
int oldtype;
- oldtype = __pthread_enable_asynccancel ();
+ oldtype = LIBC_CANCEL_ASYNC ();
int err = lll_futex_timed_wait (futex_word, expected, reltime, private);
- __pthread_disable_asynccancel (oldtype);
+ LIBC_CANCEL_RESET (oldtype);
switch (err)
{
case 0:
diff --git a/sysdeps/unix/sysv/linux/i386/lowlevellock.h b/sysdeps/unix/sysv/linux/i386/lowlevellock.h
index 38fbc2556f..c0b938bd9f 100644
--- a/sysdeps/unix/sysv/linux/i386/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/i386/lowlevellock.h
@@ -221,32 +221,30 @@ extern int __lll_timedlock_elision (int *futex, short *adapt_count,
#define lll_islocked(futex) \
(futex != LLL_LOCK_INITIALIZER)
+extern int __lll_timedwait_tid (int *tid, const struct timespec *abstime)
+ __attribute__ ((regparm (2))) attribute_hidden;
+
/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
wake-up when the clone terminates. The memory location contains the
thread ID while the clone is running and is reset to zero by the kernel
afterwards. The kernel up to version 3.16.3 does not use the private futex
- operations for futex wake-up when the clone terminates. */
-#define lll_wait_tid(tid) \
- do { \
- __typeof (tid) __tid; \
- while ((__tid = (tid)) != 0) \
- lll_futex_wait (&(tid), __tid, LLL_SHARED);\
- } while (0)
-
-extern int __lll_timedwait_tid (int *tid, const struct timespec *abstime)
- __attribute__ ((regparm (2))) attribute_hidden;
-
-/* As lll_wait_tid, but with a timeout. If the timeout occurs then return
- ETIMEDOUT. If ABSTIME is invalid, return EINVAL.
- XXX Note that this differs from the generic version in that we do the
- error checking here and not in __lll_timedwait_tid. */
-#define lll_timedwait_tid(tid, abstime) \
- ({ \
- int __result = 0; \
- if ((tid) != 0) \
- __result = __lll_timedwait_tid (&(tid), (abstime)); \
- __result; })
-
+ operations for futex wake-up when the clone terminates.
+ If ABSTIME is not NULL, is used a timeout for futex call. If the timeout
+ occurs then return ETIMEOUT, if ABSTIME is invalid, return EINVAL.
+ The futex operation are issues with cancellable versions. */
+#define lll_wait_tid(tid, abstime) \
+ ({ \
+ int __res = 0; \
+ __typeof (tid) __tid; \
+ if (abstime != NULL) \
+ __res = __lll_timedwait_tid (&(tid), (abstime)); \
+ else \
+ /* We need acquire MO here so that we synchronize with the \
+ kernel's store to 0 when the clone terminates. (see above) */ \
+ while ((__tid = atomic_load_acquire (&(tid))) != 0) \
+ lll_futex_wait_cancel (&(tid), __tid, LLL_SHARED); \
+ __res; \
+ })
extern int __lll_lock_elision (int *futex, short *adapt_count, int private)
attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/lowlevellock-futex.h b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
index fc834ed16e..71420fea89 100644
--- a/sysdeps/unix/sysv/linux/lowlevellock-futex.h
+++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
@@ -125,6 +125,17 @@
private), \
nr_wake, nr_move, mutex, val)
+
+/* Cancellable futex macros. */
+#define lll_futex_wait_cancel(futexp, val, private) \
+ ({ \
+ int __oldtype = CANCEL_ASYNC (); \
+ long int __err = lll_futex_wait (futexp, val, LLL_SHARED); \
+ CANCEL_RESET (__oldtype); \
+ __err; \
+ })
+
+
#endif /* !__ASSEMBLER__ */
#endif /* lowlevellock-futex.h */
diff --git a/sysdeps/unix/sysv/linux/sparc/lowlevellock.h b/sysdeps/unix/sysv/linux/sparc/lowlevellock.h
index e2c0b2abaa..28f368dbe7 100644
--- a/sysdeps/unix/sysv/linux/sparc/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/sparc/lowlevellock.h
@@ -108,28 +108,29 @@ __lll_timedlock (int *futex, const struct timespec *abstime, int private)
#define LLL_LOCK_INITIALIZER (0)
#define LLL_LOCK_INITIALIZER_LOCKED (1)
-/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
- wakeup when the clone terminates. The memory location contains the
- thread ID while the clone is running and is reset to zero
- afterwards. */
-#define lll_wait_tid(tid) \
- do \
- { \
- __typeof (tid) __tid; \
- while ((__tid = (tid)) != 0) \
- lll_futex_wait (&(tid), __tid, LLL_SHARED); \
- } \
- while (0)
-
extern int __lll_timedwait_tid (int *, const struct timespec *)
attribute_hidden;
-#define lll_timedwait_tid(tid, abstime) \
- ({ \
- int __res = 0; \
- if ((tid) != 0) \
- __res = __lll_timedwait_tid (&(tid), (abstime)); \
- __res; \
+/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
+ wake-up when the clone terminates. The memory location contains the
+ thread ID while the clone is running and is reset to zero by the kernel
+ afterwards. The kernel up to version 3.16.3 does not use the private futex
+ operations for futex wake-up when the clone terminates.
+ If ABSTIME is not NULL, is used a timeout for futex call. If the timeout
+ occurs then return ETIMEOUT, if ABSTIME is invalid, return EINVAL.
+ The futex operation are issues with cancellable versions. */
+#define lll_wait_tid(tid, abstime) \
+ ({ \
+ int __res = 0; \
+ __typeof (tid) __tid; \
+ if (abstime != NULL) \
+ __res = __lll_timedwait_tid (&(tid), (abstime)); \
+ else \
+ /* We need acquire MO here so that we synchronize with the \
+ kernel's store to 0 when the clone terminates. (see above) */ \
+ while ((__tid = atomic_load_acquire (&(tid))) != 0) \
+ lll_futex_wait_cancel (&(tid), __tid, LLL_SHARED); \
+ __res; \
})
#endif /* lowlevellock.h */
diff --git a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index eedb6fc990..3c5860d4cf 100644
--- a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -224,32 +224,30 @@ extern int __lll_timedlock_elision (int *futex, short *adapt_count,
#define lll_islocked(futex) \
(futex != LLL_LOCK_INITIALIZER)
+extern int __lll_timedwait_tid (int *, const struct timespec *)
+ attribute_hidden;
/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
wake-up when the clone terminates. The memory location contains the
thread ID while the clone is running and is reset to zero by the kernel
afterwards. The kernel up to version 3.16.3 does not use the private futex
- operations for futex wake-up when the clone terminates. */
-#define lll_wait_tid(tid) \
- do { \
- __typeof (tid) __tid; \
- while ((__tid = (tid)) != 0) \
- lll_futex_wait (&(tid), __tid, LLL_SHARED);\
- } while (0)
-
-extern int __lll_timedwait_tid (int *, const struct timespec *)
- attribute_hidden;
-
-/* As lll_wait_tid, but with a timeout. If the timeout occurs then return
- ETIMEDOUT. If ABSTIME is invalid, return EINVAL.
- XXX Note that this differs from the generic version in that we do the
- error checking here and not in __lll_timedwait_tid. */
-#define lll_timedwait_tid(tid, abstime) \
- ({ \
- int __result = 0; \
- if ((tid) != 0) \
- __result = __lll_timedwait_tid (&(tid), (abstime)); \
- __result; })
+ operations for futex wake-up when the clone terminates.
+ If ABSTIME is not NULL, is used a timeout for futex call. If the timeout
+ occurs then return ETIMEOUT, if ABSTIME is invalid, return EINVAL.
+ The futex operation are issues with cancellable versions. */
+#define lll_wait_tid(tid, abstime) \
+ ({ \
+ int __res = 0; \
+ __typeof (tid) __tid; \
+ if (abstime != NULL) \
+ __res = __lll_timedwait_tid (&(tid), (abstime)); \
+ else \
+ /* We need acquire MO here so that we synchronize with the \
+ kernel's store to 0 when the clone terminates. (see above) */ \
+ while ((__tid = atomic_load_acquire (&(tid))) != 0) \
+ lll_futex_wait_cancel (&(tid), __tid, LLL_SHARED); \
+ __res; \
+ })
extern int __lll_lock_elision (int *futex, short *adapt_count, int private)
attribute_hidden;
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] nptl: Fix testcases for new pthread cancellation mechanism
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
` (4 preceding siblings ...)
2018-09-04 20:46 ` [PATCH 1/7] powerpc: Add CFI information on indirect syscall Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 4/7] x86: Remove wrong THREAD_ATOMIC_* macros Adhemerval Zanella
2018-09-25 21:13 ` [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
7 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha; +Cc: Adhemerval Zanella
From: Adhemerval Zanella <adhemerval.zanella@linaro.com>
With upcoming fix for BZ#12683, pthread cancellation does not act for:
1. If syscall is blocked but with some side effects already having
taken place (e.g. a partial read or write).
2. After the syscall has returned.
The main change is due the fact programs need to act in syscalls with
side-effects (for instance, to avoid leak of allocated resources or
handle partial read/write).
This patch changes the NPTL testcase that assumes the old behavior and
also changes the tst-backtrace{5,6} to ignore the cancellable wrappers.
Checked on i686-linux-gnu, x86_64-linux-gnu, x86_64-linux-gnux32,
aarch64-linux-gnu, arm-linux-gnueabihf, powerpc64le-linux-gnu,
powerpc-linux-gnu, sparcv9-linux-gnu, and sparc64-linux-gnu.
* debug/tst-backtrace5.c (handle_signal): Avoid cancellable wrappers
in backtrace analysis.
* nptl/tst-cancel4.c (tf_write): Handle cancelled syscall with
side-effects.
(tf_send): Likewise.
---
ChangeLog | 6 ++++++
debug/tst-backtrace5.c | 19 ++++++++++---------
nptl/tst-cancel4.c | 8 ++++++++
3 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/debug/tst-backtrace5.c b/debug/tst-backtrace5.c
index 0e6fb1a024..dad741e12a 100644
--- a/debug/tst-backtrace5.c
+++ b/debug/tst-backtrace5.c
@@ -69,17 +69,18 @@ handle_signal (int signum)
FAIL ();
return;
}
- /* Do not check name for signal trampoline. */
- i = 2;
- if (!match (symbols[i++], "read"))
+
+ /* Do not check name for signal trampoline or cancellable syscall
+ wrappers (__syscall_cancel*). */
+ for (; i < n - 1; i++)
+ if (match (symbols[i], "read"))
+ break;
+ if (i == n - 1)
{
- /* Perhaps symbols[2] is __kernel_vsyscall? */
- if (!match (symbols[i++], "read"))
- {
- FAIL ();
- return;
- }
+ FAIL ();
+ return;
}
+
for (; i < n - 1; i++)
if (!match (symbols[i], "fn"))
{
diff --git a/nptl/tst-cancel4.c b/nptl/tst-cancel4.c
index 05325385b1..45b3d43c3b 100644
--- a/nptl/tst-cancel4.c
+++ b/nptl/tst-cancel4.c
@@ -166,6 +166,10 @@ tf_write (void *arg)
char buf[WRITE_BUFFER_SIZE];
memset (buf, '\0', sizeof (buf));
s = write (fd, buf, sizeof (buf));
+ /* The write can return a value higher than 0 (meaning partial write)
+ due to the SIGCANCEL, but the thread may still be pending
+ cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
@@ -743,6 +747,10 @@ tf_send (void *arg)
char mem[WRITE_BUFFER_SIZE];
send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0);
+ /* The send can return a value higher than 0 (meaning partial send)
+ due to the SIGCANCEL, but the thread may still be pending
+ cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/7] General fixes and refactor for BZ#12683
@ 2018-09-04 20:46 Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 5/7] i386: Remove bogus THREAD_ATOMIC_* macros Adhemerval Zanella
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
These patches are refactoring and fixes required for my proposal to
BZ#12683 [1]. They can be applied independently of the cancellation
refactor fix and the idea is to simplify the patch submission with
changes only direct related to the issue.
[1] https://sourceware.org/ml/libc-alpha/2018-02/msg00782.html
Adhemerval Zanella (7):
powerpc: Add CFI information on indirect syscall
nptl: Remove tst-cancel-wrappers test and related macros
nptl: Fix testcases for new pthread cancellation mechanism
x86: Remove wrong THREAD_ATOMIC_* macros
i386: Remove bogus THREAD_ATOMIC_* macros
nptl: Cleanup cancellation macros
support: Add support_create_temp_fifo
ChangeLog | 70 ++++++++++++++
debug/tst-backtrace5.c | 19 ++--
io/creat.c | 3 -
io/ppoll.c | 2 -
misc/pselect.c | 2 -
nptl/Makefile | 18 +---
nptl/pthreadP.h | 10 --
nptl/pthread_join_common.c | 9 +-
nptl/sem_wait.c | 8 +-
nptl/tst-cancel-wrappers.sh | 92 -------------------
nptl/tst-cancel4.c | 8 ++
support/temp_file.c | 23 +++++
support/temp_file.h | 6 ++
sysdeps/generic/sysdep-cancel.h | 1 -
sysdeps/i386/nptl/tls.h | 37 --------
sysdeps/mach/hurd/sysdep-cancel.h | 1 -
sysdeps/nptl/aio_misc.h | 15 ++-
sysdeps/nptl/gai_misc.h | 15 ++-
sysdeps/nptl/lowlevellock.h | 40 ++++----
sysdeps/posix/open64.c | 12 +--
sysdeps/posix/pause.c | 2 -
sysdeps/posix/sigpause.c | 3 -
sysdeps/posix/sigwait.c | 12 +--
sysdeps/posix/waitid.c | 12 +--
sysdeps/unix/sysv/linux/clock_nanosleep.c | 23 ++---
sysdeps/unix/sysv/linux/creat.c | 2 -
sysdeps/unix/sysv/linux/creat64.c | 2 -
sysdeps/unix/sysv/linux/futex-internal.h | 4 +-
sysdeps/unix/sysv/linux/i386/lowlevellock.h | 42 ++++-----
sysdeps/unix/sysv/linux/lowlevellock-futex.h | 11 +++
sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
sysdeps/unix/sysv/linux/sigwait.c | 3 -
sysdeps/unix/sysv/linux/sigwaitinfo.c | 3 -
sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 39 ++++----
sysdeps/unix/sysv/linux/x86_64/lowlevellock.h | 40 ++++----
sysdeps/x86_64/nptl/tls.h | 37 --------
36 files changed, 237 insertions(+), 390 deletions(-)
delete mode 100644 nptl/tst-cancel-wrappers.sh
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/7] powerpc: Add CFI information on indirect syscall
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
` (3 preceding siblings ...)
2018-09-04 20:46 ` [PATCH 7/7] support: Add support_create_temp_fifo Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-06 15:18 ` Tulio Magno Quites Machado Filho
2018-09-04 20:46 ` [PATCH 3/7] nptl: Fix testcases for new pthread cancellation mechanism Adhemerval Zanella
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
This patch the required CFI information on powerpc indirect syscall
so backtrace works correctly on signal handler.
Checked on powerpc-linux-gnu and powerpc64le-linux-gnu.
* sysdeps/unix/sysv/linux/powerpc/syscall.S (syscall): Add CFI
information.
---
ChangeLog | 5 +++++
sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
2 files changed, 6 insertions(+)
diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall.S b/sysdeps/unix/sysv/linux/powerpc/syscall.S
index 2da91721be..e30f461a17 100644
--- a/sysdeps/unix/sysv/linux/powerpc/syscall.S
+++ b/sysdeps/unix/sysv/linux/powerpc/syscall.S
@@ -19,6 +19,7 @@
ENTRY (syscall)
ABORT_TRANSACTION
+ cfi_def_cfa_offset (0)
mr r0,r3
mr r3,r4
mr r4,r5
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/7] i386: Remove bogus THREAD_ATOMIC_* macros
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 2/7] nptl: Remove tst-cancel-wrappers test and related macros Adhemerval Zanella
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
The x86 defines optimized THREAD_ATOMIC_* macros where reference always
the current thread instead of the one indicated by input 'descr' argument.
It work as long the input is the self thread pointer, however it generates
wrong code is the semantic is to set a bit atomicialy from another thread.
This is not an issue for current GLIBC usage, however the new cancellation
code expects that some synchronization code to atomically set bits from
different threads.
If some usage indeed proves to be a hotspot we can add an extra macro
with a more descriptive name (THREAD_ATOMIC_BIT_SET_SELF for instance)
where i386 might optimize it.
Checked on i686-linux-gnu.
* sysdeps/i686/nptl/tls.h (THREAD_ATOMIC_CMPXCHG_VAL,
THREAD_ATOMIC_AND, THREAD_ATOMIC_BIT_SET): Remove macros.
---
ChangeLog | 3 +++
sysdeps/i386/nptl/tls.h | 37 -------------------------------------
2 files changed, 3 insertions(+), 37 deletions(-)
diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
index 12285d3217..22ebf3d741 100644
--- a/sysdeps/i386/nptl/tls.h
+++ b/sysdeps/i386/nptl/tls.h
@@ -362,43 +362,6 @@ tls_fill_user_desc (union user_desc_init *desc,
}})
-/* Atomic compare and exchange on TLS, returning old value. */
-#define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \
- ({ __typeof (descr->member) __ret; \
- __typeof (oldval) __old = (oldval); \
- if (sizeof (descr->member) == 4) \
- asm volatile (LOCK_PREFIX "cmpxchgl %2, %%gs:%P3" \
- : "=a" (__ret) \
- : "0" (__old), "r" (newval), \
- "i" (offsetof (struct pthread, member))); \
- else \
- /* Not necessary for other sizes in the moment. */ \
- abort (); \
- __ret; })
-
-
-/* Atomic logical and. */
-#define THREAD_ATOMIC_AND(descr, member, val) \
- (void) ({ if (sizeof ((descr)->member) == 4) \
- asm volatile (LOCK_PREFIX "andl %1, %%gs:%P0" \
- :: "i" (offsetof (struct pthread, member)), \
- "ir" (val)); \
- else \
- /* Not necessary for other sizes in the moment. */ \
- abort (); })
-
-
-/* Atomic set bit. */
-#define THREAD_ATOMIC_BIT_SET(descr, member, bit) \
- (void) ({ if (sizeof ((descr)->member) == 4) \
- asm volatile (LOCK_PREFIX "orl %1, %%gs:%P0" \
- :: "i" (offsetof (struct pthread, member)), \
- "ir" (1 << (bit))); \
- else \
- /* Not necessary for other sizes in the moment. */ \
- abort (); })
-
-
/* Set the stack guard field in TCB head. */
#define THREAD_SET_STACK_GUARD(value) \
THREAD_SETMEM (THREAD_SELF, header.stack_guard, value)
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 7/7] support: Add support_create_temp_fifo
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
` (2 preceding siblings ...)
2018-09-04 20:46 ` [PATCH 6/7] nptl: Cleanup cancellation macros Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 1/7] powerpc: Add CFI information on indirect syscall Adhemerval Zanella
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
Checked on x86_64-linux-gnu.
* support/temp_file.c (support_create_temp_fifo): New function.
* support/temp_file.h (support_create_temp_fifo): New prototype.
---
ChangeLog | 3 +++
support/temp_file.c | 23 +++++++++++++++++++++++
support/temp_file.h | 6 ++++++
3 files changed, 32 insertions(+)
diff --git a/support/temp_file.c b/support/temp_file.c
index 0bbc7f9972..362ef171cc 100644
--- a/support/temp_file.c
+++ b/support/temp_file.c
@@ -86,6 +86,29 @@ create_temp_file (const char *base, char **filename)
return fd;
}
+int
+support_create_temp_fifo (const char *base, char **fifoname)
+{
+ char *fname = xasprintf ("%s/%sXXXXXX", test_dir, base);
+ mktemp (fname);
+
+ int fd = mkfifo (fname, 0600);
+ if (fd == -1)
+ {
+ printf ("cannot open temporary fifo '%s': %m\n", fname);
+ free (fname);
+ return -1;
+ }
+
+ add_temp_file (fname);
+ if (fifoname != NULL)
+ *fifoname = fname;
+ else
+ free (fname);
+
+ return fd;
+}
+
char *
support_create_temp_directory (const char *base)
{
diff --git a/support/temp_file.h b/support/temp_file.h
index c7795cc577..081b241b68 100644
--- a/support/temp_file.h
+++ b/support/temp_file.h
@@ -32,6 +32,12 @@ void add_temp_file (const char *name);
*FILENAME. */
int create_temp_file (const char *base, char **filename);
+/* Create a temporary fifo. Return the opened file descriptor on
+ success, or -1 on failure. Write the file name to *FILENAME if
+ FILENAME is not NULL. In this case, the caller is expected to free
+ *FILENAME. */
+int support_create_temp_fifo (const char *name, char **fifoname);
+
/* Create a temporary directory and schedule it for deletion. BASE is
used as a prefix for the unique directory name, which the function
returns. The caller should free this string. */
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/7] x86: Remove wrong THREAD_ATOMIC_* macros
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
` (5 preceding siblings ...)
2018-09-04 20:46 ` [PATCH 3/7] nptl: Fix testcases for new pthread cancellation mechanism Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-25 21:13 ` [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
7 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
The x86 defines optimized THREAD_ATOMIC_* macros where reference always
the current thread instead of the one indicated by input 'descr' argument.
It work as long the input is the self thread pointer, however it generates
wrong code is the semantic is to set a bit atomicialy from another thread.
This is not an issue for current GLIBC usage, however the new cancellation
code expects that some synchronization code to atomically set bits from
different threads.
The generic code generates an additional load to reference to TLS segment,
for instance the code:
THREAD_ATOMIC_BIT_SET (THREAD_SELF, cancelhandling, CANCELED_BIT);
Compiles to:
lock;orl $4, %fs:776
Where with patch changes it now compiles to:
mov %fs:16,%rax
lock;orl $4, 776(%rax)
If some usage indeed proves to be a hotspot we can add an extra macro
with a more descriptive name (THREAD_ATOMIC_BIT_SET_SELF for instance)
where x86_64 might optimize it.
Checked on x86_64-linux-gnu.
* sysdeps/x86_64/nptl/tls.h (THREAD_ATOMIC_CMPXCHG_VAL,
THREAD_ATOMIC_AND, THREAD_ATOMIC_BIT_SET): Remove macros.
---
ChangeLog | 3 +++
sysdeps/x86_64/nptl/tls.h | 37 -------------------------------------
2 files changed, 3 insertions(+), 37 deletions(-)
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index e88561c934..835a0d3deb 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -306,43 +306,6 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
}})
-/* Atomic compare and exchange on TLS, returning old value. */
-# define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \
- ({ __typeof (descr->member) __ret; \
- __typeof (oldval) __old = (oldval); \
- if (sizeof (descr->member) == 4) \
- asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \
- : "=a" (__ret) \
- : "0" (__old), "r" (newval), \
- "i" (offsetof (struct pthread, member))); \
- else \
- /* Not necessary for other sizes in the moment. */ \
- abort (); \
- __ret; })
-
-
-/* Atomic logical and. */
-# define THREAD_ATOMIC_AND(descr, member, val) \
- (void) ({ if (sizeof ((descr)->member) == 4) \
- asm volatile (LOCK_PREFIX "andl %1, %%fs:%P0" \
- :: "i" (offsetof (struct pthread, member)), \
- "ir" (val)); \
- else \
- /* Not necessary for other sizes in the moment. */ \
- abort (); })
-
-
-/* Atomic set bit. */
-# define THREAD_ATOMIC_BIT_SET(descr, member, bit) \
- (void) ({ if (sizeof ((descr)->member) == 4) \
- asm volatile (LOCK_PREFIX "orl %1, %%fs:%P0" \
- :: "i" (offsetof (struct pthread, member)), \
- "ir" (1 << (bit))); \
- else \
- /* Not necessary for other sizes in the moment. */ \
- abort (); })
-
-
/* Set the stack guard field in TCB head. */
# define THREAD_SET_STACK_GUARD(value) \
THREAD_SETMEM (THREAD_SELF, header.stack_guard, value)
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/7] nptl: Remove tst-cancel-wrappers test and related macros
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 5/7] i386: Remove bogus THREAD_ATOMIC_* macros Adhemerval Zanella
@ 2018-09-04 20:46 ` Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 6/7] nptl: Cleanup cancellation macros Adhemerval Zanella
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-04 20:46 UTC (permalink / raw)
To: libc-alpha
With upcoming BZ#12683 fix, syscall cancellation is not more handled
by {libc,pthread,librt}_{enable,disable}_asynccancel symbols. This renders
both LIBC_CANCEL_HANDLED and empty declaration and tst-cancel-wrappers.sh
unrequired. This patch removes both the macro and the nptl test.
Checked on x86_64-linux-gnu.
* io/creat.c (LIBC_CANCEL_HANDLED): Remove macro.
* io/ppoll.c (LIBC_CANCEL_HANDLED): Likewise.
* misc/pselect.c (LIBC_CANCEL_HANDLED): Likewise.
* nptl/pthreadP.h (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/generic/sysdep-cancel.h (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/mach/hurd/sysdep-cancel.h (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/posix/pause.c (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/posix/sigpause.c (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/unix/sysv/linux/creat.c (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/unix/sysv/linux/creat64.c (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/unix/sysv/linux/sigwait.c (LIBC_CANCEL_HANDLED): Likewise.
* sysdeps/unix/sysv/linux/sigwaitinfo.c (LIBC_CANCEL_HANDLED):
Likewise.
* nptl/Makefile [$(run-built-tests) = yes] (tests-special): Remove
tst-cancel-wrappers.sh.
(generated): Remove tst-cancel-wrappers.out.
(tst-cancel-wrappers.out): Remove rule.
* nptl/tst-cancel-wrappers.sh: Remove file.
---
ChangeLog | 19 ++++++
io/creat.c | 3 -
io/ppoll.c | 2 -
misc/pselect.c | 2 -
nptl/Makefile | 18 +-----
nptl/pthreadP.h | 10 ---
nptl/tst-cancel-wrappers.sh | 92 ---------------------------
sysdeps/generic/sysdep-cancel.h | 1 -
sysdeps/mach/hurd/sysdep-cancel.h | 1 -
sysdeps/posix/pause.c | 2 -
sysdeps/posix/sigpause.c | 3 -
sysdeps/unix/sysv/linux/creat.c | 2 -
sysdeps/unix/sysv/linux/creat64.c | 2 -
sysdeps/unix/sysv/linux/sigwait.c | 3 -
sysdeps/unix/sysv/linux/sigwaitinfo.c | 3 -
15 files changed, 21 insertions(+), 142 deletions(-)
delete mode 100644 nptl/tst-cancel-wrappers.sh
diff --git a/io/creat.c b/io/creat.c
index 21ee56ebc9..3d0afcab54 100644
--- a/io/creat.c
+++ b/io/creat.c
@@ -27,6 +27,3 @@ creat (const char *file, mode_t mode)
{
return __open (file, O_WRONLY|O_CREAT|O_TRUNC, mode);
}
-
-/* __open handles cancellation. */
-LIBC_CANCEL_HANDLED ();
diff --git a/io/ppoll.c b/io/ppoll.c
index ec26b99fee..5ccfdb9fda 100644
--- a/io/ppoll.c
+++ b/io/ppoll.c
@@ -70,7 +70,5 @@ ppoll (struct pollfd *fds, nfds_t nfds, const struct timespec *timeout,
}
#ifndef ppoll
-/* __poll handles cancellation. */
-LIBC_CANCEL_HANDLED ();
libc_hidden_def (ppoll);
#endif
diff --git a/misc/pselect.c b/misc/pselect.c
index 2c29230596..07d5ffcef0 100644
--- a/misc/pselect.c
+++ b/misc/pselect.c
@@ -73,6 +73,4 @@ __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
}
#ifndef __pselect
weak_alias (__pselect, pselect)
-/* __select handles cancellation. */
-LIBC_CANCEL_HANDLED ();
#endif
diff --git a/nptl/Makefile b/nptl/Makefile
index be8066524c..27f28d4295 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -461,8 +461,7 @@ tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-stack3-mem.out $(objpfx)tst-oddstacklimit.out
ifeq ($(build-shared),yes)
-tests-special += $(objpfx)tst-tls6.out $(objpfx)tst-cleanup0-cmp.out \
- $(objpfx)tst-cancel-wrappers.out
+tests-special += $(objpfx)tst-tls6.out $(objpfx)tst-cleanup0-cmp.out
endif
endif
@@ -665,8 +664,7 @@ $(objpfx)$(multidir)/crtn.o: $(objpfx)crtn.o $(objpfx)$(multidir)/
ln -f $< $@
endif
-generated += multidir.mk tst-atfork2.mtrace tst-cancel-wrappers.out \
- tst-tls6.out
+generated += multidir.mk tst-atfork2.mtrace tst-tls6.out
generated += $(objpfx)tst-atfork2.mtrace \
$(addsuffix .so,$(strip $(modules-names)))
@@ -677,18 +675,6 @@ LDFLAGS-pthread.so += -e __nptl_main
$(objpfx)pt-interp.os: $(common-objpfx)runtime-linker.h
endif
-ifeq ($(run-built-tests),yes)
-ifeq (yes,$(build-shared))
-$(objpfx)tst-cancel-wrappers.out: tst-cancel-wrappers.sh
- $(SHELL) $< '$(NM)' \
- $(common-objpfx)libc_pic.a \
- $(common-objpfx)libc.a \
- $(objpfx)libpthread_pic.a \
- $(objpfx)libpthread.a > $@; \
- $(evaluate-test)
-endif
-endif
-
tst-exec4-ARGS = $(host-test-program-cmd)
$(objpfx)tst-execstack: $(libdl)
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 13bdb11133..2d30555941 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -316,27 +316,17 @@ __do_cancel (void)
/* Same as CANCEL_RESET, but for use in libc.so. */
# define LIBC_CANCEL_RESET(oldtype) \
__libc_disable_asynccancel (oldtype)
-# define LIBC_CANCEL_HANDLED() \
- __asm (".globl " __SYMBOL_PREFIX "__libc_enable_asynccancel"); \
- __asm (".globl " __SYMBOL_PREFIX "__libc_disable_asynccancel")
#elif IS_IN (libpthread)
# define LIBC_CANCEL_ASYNC() CANCEL_ASYNC ()
# define LIBC_CANCEL_RESET(val) CANCEL_RESET (val)
-# define LIBC_CANCEL_HANDLED() \
- __asm (".globl " __SYMBOL_PREFIX "__pthread_enable_asynccancel"); \
- __asm (".globl " __SYMBOL_PREFIX "__pthread_disable_asynccancel")
#elif IS_IN (librt)
# define LIBC_CANCEL_ASYNC() \
__librt_enable_asynccancel ()
# define LIBC_CANCEL_RESET(val) \
__librt_disable_asynccancel (val)
-# define LIBC_CANCEL_HANDLED() \
- __asm (".globl " __SYMBOL_PREFIX "__librt_enable_asynccancel"); \
- __asm (".globl " __SYMBOL_PREFIX "__librt_disable_asynccancel")
#else
# define LIBC_CANCEL_ASYNC() 0 /* Just a dummy value. */
# define LIBC_CANCEL_RESET(val) ((void)(val)) /* Nothing, but evaluate it. */
-# define LIBC_CANCEL_HANDLED() /* Nothing. */
#endif
diff --git a/nptl/tst-cancel-wrappers.sh b/nptl/tst-cancel-wrappers.sh
deleted file mode 100644
index 0c5b614287..0000000000
--- a/nptl/tst-cancel-wrappers.sh
+++ /dev/null
@@ -1,92 +0,0 @@
-#!/bin/sh
-# Test whether all cancelable functions are cancelable.
-# Copyright (C) 2002-2018 Free Software Foundation, Inc.
-# This file is part of the GNU C Library.
-# Contributed by Jakub Jelinek <jakub@redhat.com>, 2002.
-
-# 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/>.
-
-NM="$1"; shift
-while [ $# -gt 0 ]; do
- ( $NM -P $1; echo 'end[end]:' ) | gawk ' BEGIN {
-C["accept"]=1
-C["close"]=1
-C["connect"]=1
-C["creat"]=1
-C["fcntl"]=1
-C["fdatasync"]=1
-C["fsync"]=1
-C["msgrcv"]=1
-C["msgsnd"]=1
-C["msync"]=1
-C["nanosleep"]=1
-C["open"]=1
-C["open64"]=1
-C["pause"]=1
-C["poll"]=1
-C["pread"]=1
-C["pread64"]=1
-C["pselect"]=1
-C["pwrite"]=1
-C["pwrite64"]=1
-C["read"]=1
-C["readv"]=1
-C["recv"]=1
-C["recvfrom"]=1
-C["recvmsg"]=1
-C["select"]=1
-C["send"]=1
-C["sendmsg"]=1
-C["sendto"]=1
-C["sigpause"]=1
-C["sigsuspend"]=1
-C["sigwait"]=1
-C["sigwaitinfo"]=1
-C["tcdrain"]=1
-C["wait"]=1
-C["waitid"]=1
-C["waitpid"]=1
-C["write"]=1
-C["writev"]=1
-C["__xpg_sigpause"]=1
-}
-/:$/ {
- if (seen)
- {
- if (!seen_enable || !seen_disable)
- {
- printf "in '$1'(%s) %s'\''s cancellation missing\n", object, seen
- ret = 1
- }
- }
- seen=""
- seen_enable=""
- seen_disable=""
- object=gensub(/^.*\[(.*)\]:$/, "\\1", 1, $0)
- next
-}
-{
- if (C[$1] && $2 ~ /^[TW]$/)
- seen=$1
- else if ($1 ~ /^([.]|)__(libc|pthread)_enable_asynccancel$/ && $2 == "U")
- seen_enable=1
- else if ($1 ~ /^([.]|)__(libc|pthread)_disable_asynccancel$/ && $2 == "U")
- seen_disable=1
-}
-END {
- exit ret
-}' || exit
- shift
-done
diff --git a/sysdeps/generic/sysdep-cancel.h b/sysdeps/generic/sysdep-cancel.h
index ba6a1e04ba..d22a786536 100644
--- a/sysdeps/generic/sysdep-cancel.h
+++ b/sysdeps/generic/sysdep-cancel.h
@@ -5,4 +5,3 @@
#define RTLD_SINGLE_THREAD_P (1)
#define LIBC_CANCEL_ASYNC() 0 /* Just a dummy value. */
#define LIBC_CANCEL_RESET(val) ((void)(val)) /* Nothing, but evaluate it. */
-#define LIBC_CANCEL_HANDLED() /* Nothing. */
diff --git a/sysdeps/mach/hurd/sysdep-cancel.h b/sysdeps/mach/hurd/sysdep-cancel.h
index ec55c7330f..f686a39024 100644
--- a/sysdeps/mach/hurd/sysdep-cancel.h
+++ b/sysdeps/mach/hurd/sysdep-cancel.h
@@ -6,4 +6,3 @@
#define RTLD_SINGLE_THREAD_P (0)
#define LIBC_CANCEL_ASYNC() 0 /* Just a dummy value. */
#define LIBC_CANCEL_RESET(val) ((void)(val)) /* Nothing, but evaluate it. */
-#define LIBC_CANCEL_HANDLED() /* Nothing. */
diff --git a/sysdeps/posix/pause.c b/sysdeps/posix/pause.c
index 2b9eca2192..879a8abd89 100644
--- a/sysdeps/posix/pause.c
+++ b/sysdeps/posix/pause.c
@@ -38,5 +38,3 @@ __libc_pause (void)
return __sigsuspend (&set);
}
weak_alias (__libc_pause, pause)
-
-LIBC_CANCEL_HANDLED (); /* sigsuspend handles our cancellation. */
diff --git a/sysdeps/posix/sigpause.c b/sysdeps/posix/sigpause.c
index db9df8eb6e..9a17d1b5d6 100644
--- a/sysdeps/posix/sigpause.c
+++ b/sysdeps/posix/sigpause.c
@@ -70,6 +70,3 @@ __xpg_sigpause (int sig)
return __sigpause (sig, 1);
}
strong_alias (__xpg_sigpause, __libc___xpg_sigpause)
-
-/* __sigsuspend handles cancellation. */
-LIBC_CANCEL_HANDLED ();
diff --git a/sysdeps/unix/sysv/linux/creat.c b/sysdeps/unix/sysv/linux/creat.c
index c996cbd3cc..089a8692f2 100644
--- a/sysdeps/unix/sysv/linux/creat.c
+++ b/sysdeps/unix/sysv/linux/creat.c
@@ -35,6 +35,4 @@ __creat (const char *file, mode_t mode)
}
weak_alias (__creat, creat)
-LIBC_CANCEL_HANDLED ();
-
#endif
diff --git a/sysdeps/unix/sysv/linux/creat64.c b/sysdeps/unix/sysv/linux/creat64.c
index d3ada38c9c..c9eba2ec5e 100644
--- a/sysdeps/unix/sysv/linux/creat64.c
+++ b/sysdeps/unix/sysv/linux/creat64.c
@@ -37,5 +37,3 @@ weak_alias (__creat64, creat64)
strong_alias (__creat64, __creat)
weak_alias (__creat64, creat)
#endif
-
-LIBC_CANCEL_HANDLED ();
diff --git a/sysdeps/unix/sysv/linux/sigwait.c b/sysdeps/unix/sysv/linux/sigwait.c
index 920c924c9c..18ef2cf574 100644
--- a/sysdeps/unix/sysv/linux/sigwait.c
+++ b/sysdeps/unix/sysv/linux/sigwait.c
@@ -37,6 +37,3 @@ __sigwait (const sigset_t *set, int *sig)
libc_hidden_def (__sigwait)
weak_alias (__sigwait, sigwait)
strong_alias (__sigwait, __libc_sigwait)
-
-/* __sigtimedwait handles cancellation. */
-LIBC_CANCEL_HANDLED ();
diff --git a/sysdeps/unix/sysv/linux/sigwaitinfo.c b/sysdeps/unix/sysv/linux/sigwaitinfo.c
index 55003fc516..71bdc943a0 100644
--- a/sysdeps/unix/sysv/linux/sigwaitinfo.c
+++ b/sysdeps/unix/sysv/linux/sigwaitinfo.c
@@ -28,6 +28,3 @@ __sigwaitinfo (const sigset_t *set, siginfo_t *info)
libc_hidden_def (__sigwaitinfo)
weak_alias (__sigwaitinfo, sigwaitinfo)
strong_alias (__sigwaitinfo, __libc_sigwaitinfo)
-
-/* __sigtimedwait handles cancellation. */
-LIBC_CANCEL_HANDLED ();
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/7] powerpc: Add CFI information on indirect syscall
2018-09-04 20:46 ` [PATCH 1/7] powerpc: Add CFI information on indirect syscall Adhemerval Zanella
@ 2018-09-06 15:18 ` Tulio Magno Quites Machado Filho
2018-09-25 21:13 ` Adhemerval Zanella
0 siblings, 1 reply; 16+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-09-06 15:18 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha; +Cc: Alan Modra
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> This patch the required CFI information on powerpc indirect syscall
> so backtrace works correctly on signal handler.
>
> Checked on powerpc-linux-gnu and powerpc64le-linux-gnu.
>
> * sysdeps/unix/sysv/linux/powerpc/syscall.S (syscall): Add CFI
> information.
> ---
> ChangeLog | 5 +++++
> sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall.S b/sysdeps/unix/sysv/linux/powerpc/syscall.S
> index 2da91721be..e30f461a17 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/syscall.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/syscall.S
> @@ -19,6 +19,7 @@
>
> ENTRY (syscall)
> ABORT_TRANSACTION
> + cfi_def_cfa_offset (0)
Why is this necessary in a function that is defined via ENTRY (or
ENTRY_TOCLESS) that already has cfi_startproc?
Does the error you found mean that parameter simple from cfi_startproc is being
set by mistake?
--
Tulio Magno
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] General fixes and refactor for BZ#12683
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
` (6 preceding siblings ...)
2018-09-04 20:46 ` [PATCH 4/7] x86: Remove wrong THREAD_ATOMIC_* macros Adhemerval Zanella
@ 2018-09-25 21:13 ` Adhemerval Zanella
2018-11-28 14:10 ` Adhemerval Zanella
7 siblings, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-25 21:13 UTC (permalink / raw)
To: libc-alpha
Ping.
On 04/09/2018 17:45, Adhemerval Zanella wrote:
> These patches are refactoring and fixes required for my proposal to
> BZ#12683 [1]. They can be applied independently of the cancellation
> refactor fix and the idea is to simplify the patch submission with
> changes only direct related to the issue.
>
> [1] https://sourceware.org/ml/libc-alpha/2018-02/msg00782.html
>
> Adhemerval Zanella (7):
> powerpc: Add CFI information on indirect syscall
> nptl: Remove tst-cancel-wrappers test and related macros
> nptl: Fix testcases for new pthread cancellation mechanism
> x86: Remove wrong THREAD_ATOMIC_* macros
> i386: Remove bogus THREAD_ATOMIC_* macros
> nptl: Cleanup cancellation macros
> support: Add support_create_temp_fifo
>
> ChangeLog | 70 ++++++++++++++
> debug/tst-backtrace5.c | 19 ++--
> io/creat.c | 3 -
> io/ppoll.c | 2 -
> misc/pselect.c | 2 -
> nptl/Makefile | 18 +---
> nptl/pthreadP.h | 10 --
> nptl/pthread_join_common.c | 9 +-
> nptl/sem_wait.c | 8 +-
> nptl/tst-cancel-wrappers.sh | 92 -------------------
> nptl/tst-cancel4.c | 8 ++
> support/temp_file.c | 23 +++++
> support/temp_file.h | 6 ++
> sysdeps/generic/sysdep-cancel.h | 1 -
> sysdeps/i386/nptl/tls.h | 37 --------
> sysdeps/mach/hurd/sysdep-cancel.h | 1 -
> sysdeps/nptl/aio_misc.h | 15 ++-
> sysdeps/nptl/gai_misc.h | 15 ++-
> sysdeps/nptl/lowlevellock.h | 40 ++++----
> sysdeps/posix/open64.c | 12 +--
> sysdeps/posix/pause.c | 2 -
> sysdeps/posix/sigpause.c | 3 -
> sysdeps/posix/sigwait.c | 12 +--
> sysdeps/posix/waitid.c | 12 +--
> sysdeps/unix/sysv/linux/clock_nanosleep.c | 23 ++---
> sysdeps/unix/sysv/linux/creat.c | 2 -
> sysdeps/unix/sysv/linux/creat64.c | 2 -
> sysdeps/unix/sysv/linux/futex-internal.h | 4 +-
> sysdeps/unix/sysv/linux/i386/lowlevellock.h | 42 ++++-----
> sysdeps/unix/sysv/linux/lowlevellock-futex.h | 11 +++
> sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
> sysdeps/unix/sysv/linux/sigwait.c | 3 -
> sysdeps/unix/sysv/linux/sigwaitinfo.c | 3 -
> sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 39 ++++----
> sysdeps/unix/sysv/linux/x86_64/lowlevellock.h | 40 ++++----
> sysdeps/x86_64/nptl/tls.h | 37 --------
> 36 files changed, 237 insertions(+), 390 deletions(-)
> delete mode 100644 nptl/tst-cancel-wrappers.sh
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/7] powerpc: Add CFI information on indirect syscall
2018-09-06 15:18 ` Tulio Magno Quites Machado Filho
@ 2018-09-25 21:13 ` Adhemerval Zanella
0 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-09-25 21:13 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho, libc-alpha; +Cc: Alan Modra
On 06/09/2018 12:18, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>> This patch the required CFI information on powerpc indirect syscall
>> so backtrace works correctly on signal handler.
>>
>> Checked on powerpc-linux-gnu and powerpc64le-linux-gnu.
>>
>> * sysdeps/unix/sysv/linux/powerpc/syscall.S (syscall): Add CFI
>> information.
>> ---
>> ChangeLog | 5 +++++
>> sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall.S b/sysdeps/unix/sysv/linux/powerpc/syscall.S
>> index 2da91721be..e30f461a17 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/syscall.S
>> +++ b/sysdeps/unix/sysv/linux/powerpc/syscall.S
>> @@ -19,6 +19,7 @@
>>
>> ENTRY (syscall)
>> ABORT_TRANSACTION
>> + cfi_def_cfa_offset (0)
>
> Why is this necessary in a function that is defined via ENTRY (or
> ENTRY_TOCLESS) that already has cfi_startproc?
>
> Does the error you found mean that parameter simple from cfi_startproc is being
> set by mistake?
>
Re-testing it on powerpc64le it seems that current cfi directives
provided by ENTRY indeed are suffice. The cfi_def_cfa_offset came
in fact from a previous iteration, which I initially added a frame
set/restore. So I withdraw this patch.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] General fixes and refactor for BZ#12683
2018-09-25 21:13 ` [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
@ 2018-11-28 14:10 ` Adhemerval Zanella
2018-12-19 22:15 ` Adhemerval Zanella
0 siblings, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2018-11-28 14:10 UTC (permalink / raw)
To: libc-alpha
I really want to move forward on this, since it is essentially some refactor
for the BZ#12683 (which I would like also to get in shape at least for next
version).
Does anyone have any concerns about this patchset (minus the
"powerpc: Add CFI information on indirect syscall" which from review seems
not required)?
On 25/09/2018 18:13, Adhemerval Zanella wrote:
> Ping.
>
> On 04/09/2018 17:45, Adhemerval Zanella wrote:
>> These patches are refactoring and fixes required for my proposal to
>> BZ#12683 [1]. They can be applied independently of the cancellation
>> refactor fix and the idea is to simplify the patch submission with
>> changes only direct related to the issue.
>>
>> [1] https://sourceware.org/ml/libc-alpha/2018-02/msg00782.html
>>
>> Adhemerval Zanella (7):
>> powerpc: Add CFI information on indirect syscall
>> nptl: Remove tst-cancel-wrappers test and related macros
>> nptl: Fix testcases for new pthread cancellation mechanism
>> x86: Remove wrong THREAD_ATOMIC_* macros
>> i386: Remove bogus THREAD_ATOMIC_* macros
>> nptl: Cleanup cancellation macros
>> support: Add support_create_temp_fifo
>>
>> ChangeLog | 70 ++++++++++++++
>> debug/tst-backtrace5.c | 19 ++--
>> io/creat.c | 3 -
>> io/ppoll.c | 2 -
>> misc/pselect.c | 2 -
>> nptl/Makefile | 18 +---
>> nptl/pthreadP.h | 10 --
>> nptl/pthread_join_common.c | 9 +-
>> nptl/sem_wait.c | 8 +-
>> nptl/tst-cancel-wrappers.sh | 92 -------------------
>> nptl/tst-cancel4.c | 8 ++
>> support/temp_file.c | 23 +++++
>> support/temp_file.h | 6 ++
>> sysdeps/generic/sysdep-cancel.h | 1 -
>> sysdeps/i386/nptl/tls.h | 37 --------
>> sysdeps/mach/hurd/sysdep-cancel.h | 1 -
>> sysdeps/nptl/aio_misc.h | 15 ++-
>> sysdeps/nptl/gai_misc.h | 15 ++-
>> sysdeps/nptl/lowlevellock.h | 40 ++++----
>> sysdeps/posix/open64.c | 12 +--
>> sysdeps/posix/pause.c | 2 -
>> sysdeps/posix/sigpause.c | 3 -
>> sysdeps/posix/sigwait.c | 12 +--
>> sysdeps/posix/waitid.c | 12 +--
>> sysdeps/unix/sysv/linux/clock_nanosleep.c | 23 ++---
>> sysdeps/unix/sysv/linux/creat.c | 2 -
>> sysdeps/unix/sysv/linux/creat64.c | 2 -
>> sysdeps/unix/sysv/linux/futex-internal.h | 4 +-
>> sysdeps/unix/sysv/linux/i386/lowlevellock.h | 42 ++++-----
>> sysdeps/unix/sysv/linux/lowlevellock-futex.h | 11 +++
>> sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
>> sysdeps/unix/sysv/linux/sigwait.c | 3 -
>> sysdeps/unix/sysv/linux/sigwaitinfo.c | 3 -
>> sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 39 ++++----
>> sysdeps/unix/sysv/linux/x86_64/lowlevellock.h | 40 ++++----
>> sysdeps/x86_64/nptl/tls.h | 37 --------
>> 36 files changed, 237 insertions(+), 390 deletions(-)
>> delete mode 100644 nptl/tst-cancel-wrappers.sh
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] General fixes and refactor for BZ#12683
2018-11-28 14:10 ` Adhemerval Zanella
@ 2018-12-19 22:15 ` Adhemerval Zanella
0 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-12-19 22:15 UTC (permalink / raw)
To: libc-alpha
Ping.
On 28/11/2018 12:10, Adhemerval Zanella wrote:
> I really want to move forward on this, since it is essentially some refactor
> for the BZ#12683 (which I would like also to get in shape at least for next
> version).
>
> Does anyone have any concerns about this patchset (minus the
> "powerpc: Add CFI information on indirect syscall" which from review seems
> not required)?
>
> On 25/09/2018 18:13, Adhemerval Zanella wrote:
>> Ping.
>>
>> On 04/09/2018 17:45, Adhemerval Zanella wrote:
>>> These patches are refactoring and fixes required for my proposal to
>>> BZ#12683 [1]. They can be applied independently of the cancellation
>>> refactor fix and the idea is to simplify the patch submission with
>>> changes only direct related to the issue.
>>>
>>> [1] https://sourceware.org/ml/libc-alpha/2018-02/msg00782.html
>>>
>>> Adhemerval Zanella (7):
>>> powerpc: Add CFI information on indirect syscall
>>> nptl: Remove tst-cancel-wrappers test and related macros
>>> nptl: Fix testcases for new pthread cancellation mechanism
>>> x86: Remove wrong THREAD_ATOMIC_* macros
>>> i386: Remove bogus THREAD_ATOMIC_* macros
>>> nptl: Cleanup cancellation macros
>>> support: Add support_create_temp_fifo
>>>
>>> ChangeLog | 70 ++++++++++++++
>>> debug/tst-backtrace5.c | 19 ++--
>>> io/creat.c | 3 -
>>> io/ppoll.c | 2 -
>>> misc/pselect.c | 2 -
>>> nptl/Makefile | 18 +---
>>> nptl/pthreadP.h | 10 --
>>> nptl/pthread_join_common.c | 9 +-
>>> nptl/sem_wait.c | 8 +-
>>> nptl/tst-cancel-wrappers.sh | 92 -------------------
>>> nptl/tst-cancel4.c | 8 ++
>>> support/temp_file.c | 23 +++++
>>> support/temp_file.h | 6 ++
>>> sysdeps/generic/sysdep-cancel.h | 1 -
>>> sysdeps/i386/nptl/tls.h | 37 --------
>>> sysdeps/mach/hurd/sysdep-cancel.h | 1 -
>>> sysdeps/nptl/aio_misc.h | 15 ++-
>>> sysdeps/nptl/gai_misc.h | 15 ++-
>>> sysdeps/nptl/lowlevellock.h | 40 ++++----
>>> sysdeps/posix/open64.c | 12 +--
>>> sysdeps/posix/pause.c | 2 -
>>> sysdeps/posix/sigpause.c | 3 -
>>> sysdeps/posix/sigwait.c | 12 +--
>>> sysdeps/posix/waitid.c | 12 +--
>>> sysdeps/unix/sysv/linux/clock_nanosleep.c | 23 ++---
>>> sysdeps/unix/sysv/linux/creat.c | 2 -
>>> sysdeps/unix/sysv/linux/creat64.c | 2 -
>>> sysdeps/unix/sysv/linux/futex-internal.h | 4 +-
>>> sysdeps/unix/sysv/linux/i386/lowlevellock.h | 42 ++++-----
>>> sysdeps/unix/sysv/linux/lowlevellock-futex.h | 11 +++
>>> sysdeps/unix/sysv/linux/powerpc/syscall.S | 1 +
>>> sysdeps/unix/sysv/linux/sigwait.c | 3 -
>>> sysdeps/unix/sysv/linux/sigwaitinfo.c | 3 -
>>> sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 39 ++++----
>>> sysdeps/unix/sysv/linux/x86_64/lowlevellock.h | 40 ++++----
>>> sysdeps/x86_64/nptl/tls.h | 37 --------
>>> 36 files changed, 237 insertions(+), 390 deletions(-)
>>> delete mode 100644 nptl/tst-cancel-wrappers.sh
>>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/7] nptl: Fix testcases for new pthread cancellation, mechanism
2014-09-26 19:56 ` Joseph S. Myers
@ 2014-09-27 16:17 ` Adhemerval Zanella
0 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2014-09-27 16:17 UTC (permalink / raw)
To: libc-alpha
On 26-09-2014 16:56, Joseph S. Myers wrote:
> On Fri, 26 Sep 2014, Adhemerval Zanella wrote:
>
>> This patches fixes the NPTL testcase that assumes the old behavior and
>> also remove the tst-cancel-wrappers.sh test (which checks for symbols
>> that does not exist anymore).
> I don't think there's any clear answer to how it's now tested that all
> these functions are cancellable (e.g. a list of tests in the testsuite,
> for every function that was covered by that test, that test that
> requirement).
>
> For example, where is this tested for pread64?
>
Indeed, it will require a similar test (although I see tst-cancel-wrapper.sh
does not really exercise the code itself). I think add more test like the
ones in tst-cancel4 would be better to check for pread64 and similar functions.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/7] nptl: Fix testcases for new pthread cancellation, mechanism
2014-09-26 19:49 ` [PATCH 3/7] nptl: Fix testcases for new pthread cancellation, mechanism Adhemerval Zanella
@ 2014-09-26 19:56 ` Joseph S. Myers
2014-09-27 16:17 ` Adhemerval Zanella
0 siblings, 1 reply; 16+ messages in thread
From: Joseph S. Myers @ 2014-09-26 19:56 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C. Library
On Fri, 26 Sep 2014, Adhemerval Zanella wrote:
> This patches fixes the NPTL testcase that assumes the old behavior and
> also remove the tst-cancel-wrappers.sh test (which checks for symbols
> that does not exist anymore).
I don't think there's any clear answer to how it's now tested that all
these functions are cancellable (e.g. a list of tests in the testsuite,
for every function that was covered by that test, that test that
requirement).
For example, where is this tested for pread64?
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] nptl: Fix testcases for new pthread cancellation, mechanism
[not found] <5425BDDA.5080807@linux.vnet.ibm.com>
@ 2014-09-26 19:49 ` Adhemerval Zanella
2014-09-26 19:56 ` Joseph S. Myers
0 siblings, 1 reply; 16+ messages in thread
From: Adhemerval Zanella @ 2014-09-26 19:49 UTC (permalink / raw)
To: GNU C. Library
With upcoming fix for BZ#12683, pthread cancellation does not act for:
1. If syscall is blocked but with some side effects already having taken
place (e.g. a partial read or write)
2. After the syscall has returned.
It is because program need to act on such cases (for instance, to avoid
leak of allocated resources our handling partial read/write).
This patches fixes the NPTL testcase that assumes the old behavior and
also remove the tst-cancel-wrappers.sh test (which checks for symbols
that does not exist anymore).
Tested on x86_64, powerpc64, and powerpc32.
--
* nptl/Makefile [test-special]: Remove tst-cancel-wrappers rule.
[generated]: Likewise.
* nptl/tst-cancel-wrappers.sh: Remove file.
* nptl/tst-cancel2.c (tf): Add pthread_cancel checks for partial
read/write due cancel signal.
* nptl/tst-cancel20.c (sh_body): Likewise.
(tf_body): Likewise.
* nptl/tst-cancel21.c (sh_body): Likewise.
(tf_body): Likewise.
* nptl/tst-cancel4.c (tf_write): Likewise.
(tf_send): Likewise.
--
diff --git a/nptl/Makefile b/nptl/Makefile
index 157fe62..cb4bdd7 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -377,8 +377,7 @@ tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-stack3-mem.out $(objpfx)tst-oddstacklimit.out
ifeq ($(build-shared),yes)
-tests-special += $(objpfx)tst-tls6.out $(objpfx)tst-cleanup0-cmp.out \
- $(objpfx)tst-cancel-wrappers.out
+tests-special += $(objpfx)tst-tls6.out $(objpfx)tst-cleanup0-cmp.out
endif
endif
@@ -572,7 +571,7 @@ $(objpfx)$(multidir)/crtn.o: $(objpfx)crtn.o $(objpfx)$(multidir)/
endif
generated += libpthread_nonshared.a \
- multidir.mk tst-atfork2.mtrace tst-cancel-wrappers.out \
+ multidir.mk tst-atfork2.mtrace \
tst-tls6.out
generated += $(objpfx)tst-atfork2.mtrace \
@@ -587,18 +586,6 @@ generated += banner.h
LDFLAGS-pthread.so += -e __nptl_main
endif
-ifeq ($(run-built-tests),yes)
-ifeq (yes,$(build-shared))
-$(objpfx)tst-cancel-wrappers.out: tst-cancel-wrappers.sh
- $(SHELL) $< '$(NM)' \
- $(common-objpfx)libc_pic.a \
- $(common-objpfx)libc.a \
- $(objpfx)libpthread_pic.a \
- $(objpfx)libpthread.a > $@; \
- $(evaluate-test)
-endif
-endif
-
tst-exec4-ARGS = $(host-test-program-cmd)
$(objpfx)tst-execstack: $(libdl)
diff --git a/nptl/tst-cancel-wrappers.sh b/nptl/tst-cancel-wrappers.sh
deleted file mode 100644
index e41ed51..0000000
--- a/nptl/tst-cancel-wrappers.sh
+++ /dev/null
@@ -1,92 +0,0 @@
-#! /bin/sh
-# Test whether all cancelable functions are cancelable.
-# Copyright (C) 2002-2014 Free Software Foundation, Inc.
-# This file is part of the GNU C Library.
-# Contributed by Jakub Jelinek <jakub@redhat.com>, 2002.
-
-# 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/>.
-
-NM="$1"; shift
-while [ $# -gt 0 ]; do
- ( $NM -P $1; echo 'end[end]:' ) | gawk ' BEGIN {
-C["accept"]=1
-C["close"]=1
-C["connect"]=1
-C["creat"]=1
-C["fcntl"]=1
-C["fdatasync"]=1
-C["fsync"]=1
-C["msgrcv"]=1
-C["msgsnd"]=1
-C["msync"]=1
-C["nanosleep"]=1
-C["open"]=1
-C["open64"]=1
-C["pause"]=1
-C["poll"]=1
-C["pread"]=1
-C["pread64"]=1
-C["pselect"]=1
-C["pwrite"]=1
-C["pwrite64"]=1
-C["read"]=1
-C["readv"]=1
-C["recv"]=1
-C["recvfrom"]=1
-C["recvmsg"]=1
-C["select"]=1
-C["send"]=1
-C["sendmsg"]=1
-C["sendto"]=1
-C["sigpause"]=1
-C["sigsuspend"]=1
-C["sigwait"]=1
-C["sigwaitinfo"]=1
-C["tcdrain"]=1
-C["wait"]=1
-C["waitid"]=1
-C["waitpid"]=1
-C["write"]=1
-C["writev"]=1
-C["__xpg_sigpause"]=1
-}
-/:$/ {
- if (seen)
- {
- if (!seen_enable || !seen_disable)
- {
- printf "in '$1'(%s) %s'\''s cancellation missing\n", object, seen
- ret = 1
- }
- }
- seen=""
- seen_enable=""
- seen_disable=""
- object=gensub(/^.*\[(.*)\]:$/,"\\1","",$0)
- next
-}
-{
- if (C[$1] && $2 ~ /^[TW]$/)
- seen=$1
- else if ($1 ~ /^([.]|)__(libc|pthread)_enable_asynccancel$/ && $2 == "U")
- seen_enable=1
- else if ($1 ~ /^([.]|)__(libc|pthread)_disable_asynccancel$/ && $2 == "U")
- seen_disable=1
-}
-END {
- exit ret
-}' || exit
- shift
-done
diff --git a/nptl/tst-cancel2.c b/nptl/tst-cancel2.c
index 2d834de..cb807c4 100644
--- a/nptl/tst-cancel2.c
+++ b/nptl/tst-cancel2.c
@@ -33,6 +33,9 @@ tf (void *arg)
char buf[100000];
while (write (fd[1], buf, sizeof (buf)) > 0);
+ /* The write can return a value higher than 0 (meaning partial write)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
return (void *) 42l;
}
diff --git a/nptl/tst-cancel20.c b/nptl/tst-cancel20.c
index 703e558..0d54b21 100644
--- a/nptl/tst-cancel20.c
+++ b/nptl/tst-cancel20.c
@@ -49,6 +49,9 @@ sh_body (void)
puts ("read succeeded");
exit (1);
}
+ /* The read can return a value higher than 0 (meaning partial reads)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
}
@@ -84,7 +87,8 @@ tf_body (void)
puts ("read succeeded");
exit (1);
}
-
+ /* Check for partial read. */
+ pthread_testcancel ();
read (fd[0], &c, 1);
pthread_cleanup_pop (0);
diff --git a/nptl/tst-cancel21.c b/nptl/tst-cancel21.c
index ddcea90..35a56dc 100644
--- a/nptl/tst-cancel21.c
+++ b/nptl/tst-cancel21.c
@@ -50,6 +50,9 @@ sh_body (void)
puts ("read succeeded");
exit (1);
}
+ /* The write can return a value higher than 0 (meaning partial write)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
}
@@ -85,6 +88,8 @@ tf_body (void)
puts ("read succeeded");
exit (1);
}
+ /* Check partial read. */
+ pthread_testcancel ();
read (fd[0], &c, 1);
diff --git a/nptl/tst-cancel4.c b/nptl/tst-cancel4.c
index 93080b2..fd4ea42 100644
--- a/nptl/tst-cancel4.c
+++ b/nptl/tst-cancel4.c
@@ -38,8 +38,10 @@
#include <sys/un.h>
#include <sys/wait.h>
-#include "pthreadP.h"
-
+/* The signal used for asynchronous cancelation. */
+#ifndef SIGCANCEL
+# define SIGCANCEL __SIGRTMIN
+#endif
/* Since STREAMS are not supported in the standard Linux kernel and
there we don't advertise STREAMS as supported is no need to test
@@ -247,6 +249,9 @@ tf_write (void *arg)
char buf[WRITE_BUFFER_SIZE];
memset (buf, '\0', sizeof (buf));
s = write (fd, buf, sizeof (buf));
+ /* The write can return a value higher than 0 (meaning partial write)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
@@ -1139,6 +1144,8 @@ tf_send (void *arg)
char mem[700000];
send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0);
+ /* Check for partial send. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-12-19 22:14 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-04 20:46 [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 5/7] i386: Remove bogus THREAD_ATOMIC_* macros Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 2/7] nptl: Remove tst-cancel-wrappers test and related macros Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 6/7] nptl: Cleanup cancellation macros Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 7/7] support: Add support_create_temp_fifo Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 1/7] powerpc: Add CFI information on indirect syscall Adhemerval Zanella
2018-09-06 15:18 ` Tulio Magno Quites Machado Filho
2018-09-25 21:13 ` Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 3/7] nptl: Fix testcases for new pthread cancellation mechanism Adhemerval Zanella
2018-09-04 20:46 ` [PATCH 4/7] x86: Remove wrong THREAD_ATOMIC_* macros Adhemerval Zanella
2018-09-25 21:13 ` [PATCH 0/7] General fixes and refactor for BZ#12683 Adhemerval Zanella
2018-11-28 14:10 ` Adhemerval Zanella
2018-12-19 22:15 ` Adhemerval Zanella
[not found] <5425BDDA.5080807@linux.vnet.ibm.com>
2014-09-26 19:49 ` [PATCH 3/7] nptl: Fix testcases for new pthread cancellation, mechanism Adhemerval Zanella
2014-09-26 19:56 ` Joseph S. Myers
2014-09-27 16:17 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).