From afc35d5efde48b82a7786d9c89b115965da6b637 Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Thu, 9 Feb 2012 02:33:08 -0500 Subject: [PATCH] replace bad cancellation cleanup abi with a sane one the old abi was intended to duplicate glibc's abi at the expense of being ugly and slow, but it turns out glib was not even using that abi except on non-gcc-compatible compilers (which it doesn't even support) and was instead using an exceptions-in-c/unwind-based approach whose abi we could not duplicate anyway without nasty dwarf2/unwind integration. the new abi is copied from a very old glibc abi, which seems to still be supported/present in current glibc. it avoids all unwinding, whether by sjlj or exceptions, and merely maintains a linked list of cleanup functions to be called from the context of pthread_exit. i've made some care to ensure that longjmp out of a cleanup function should work, even though it is not required to. this change breaks abi compatibility with programs which were using pthread cancellation, which is unfortunate, but that's why i'm making the change now rather than later. considering that most pthread features have not been usable until recently anyway, i don't see it as a major issue at this point. --- arch/arm/bits/pthread.h | 6 ------ arch/i386/bits/pthread.h | 25 ------------------------- arch/x86_64/bits/pthread.h | 6 ------ include/pthread.h | 25 +++++++++---------------- src/thread/cancellation.c | 21 +++++++++------------ src/thread/i386/cancellation.s | 34 ---------------------------------- src/thread/pthread_create.c | 30 ++++++++++++++---------------- 7 files changed, 32 insertions(+), 115 deletions(-) delete mode 100644 arch/arm/bits/pthread.h delete mode 100644 arch/i386/bits/pthread.h delete mode 100644 arch/x86_64/bits/pthread.h delete mode 100644 src/thread/i386/cancellation.s diff --git a/arch/arm/bits/pthread.h b/arch/arm/bits/pthread.h deleted file mode 100644 index 9cd15f25..00000000 --- a/arch/arm/bits/pthread.h +++ /dev/null @@ -1,6 +0,0 @@ -struct __ptcb { - long __jb[64]; - int __dummy; - struct __ptcb *__next; - void *__ptrs[3]; -}; diff --git a/arch/i386/bits/pthread.h b/arch/i386/bits/pthread.h deleted file mode 100644 index c119dc8a..00000000 --- a/arch/i386/bits/pthread.h +++ /dev/null @@ -1,25 +0,0 @@ -struct __ptcb { - long __jb[7]; - int __dummy; - struct __ptcb *__next; - void *__ptrs[3]; -}; - -static inline void __pthread_register_cancel_2(struct __ptcb *__cb) -{ - __asm__ __volatile__( "call __pthread_register_cancel" : : "a"(__cb) : "ecx", "edx", "memory" ); -} - -static inline void __pthread_unregister_cancel_2(struct __ptcb *__cb) -{ - __asm__ __volatile__( "call __pthread_unregister_cancel" : : "a"(__cb) : "ecx", "edx", "memory" ); -} - -static inline void __pthread_unwind_next_2(struct __ptcb *__cb) -{ - __asm__ __volatile__( "call __pthread_unwind_next" : : "a"(__cb) : "ecx", "edx", "memory" ); -} - -#define __pthread_register_cancel __pthread_register_cancel_2 -#define __pthread_unregister_cancel __pthread_unregister_cancel_2 -#define __pthread_unwind_next __pthread_unwind_next_2 diff --git a/arch/x86_64/bits/pthread.h b/arch/x86_64/bits/pthread.h deleted file mode 100644 index 7d19065d..00000000 --- a/arch/x86_64/bits/pthread.h +++ /dev/null @@ -1,6 +0,0 @@ -struct __ptcb { - long __jb[7]; - int __dummy; - struct __ptcb *__next; - void *__ptrs[3]; -}; diff --git a/include/pthread.h b/include/pthread.h index 44a71019..1b7f9fd3 100644 --- a/include/pthread.h +++ b/include/pthread.h @@ -186,24 +186,17 @@ int pthread_atfork(void (*)(void), void (*)(void), void (*)(void)); int pthread_getconcurrency(void); int pthread_setconcurrency(int); -#include +struct __ptcb { + void (*__f)(void *); + void *__x; + struct __ptcb *__next; +}; -int __setjmp(void *); -#ifndef __pthread_register_cancel -void __pthread_register_cancel(struct __ptcb *); -void __pthread_unregister_cancel(struct __ptcb *); -void __pthread_unwind_next(struct __ptcb *); -#endif - -#define pthread_cleanup_push(f, x) \ -do { struct __ptcb __cb; void (*__f)(void *) = (f); void *__x = (x); \ -if (__setjmp(__cb.__jb)) __f(__x), __pthread_unwind_next(&__cb); \ -__pthread_register_cancel(&__cb); { - -#define pthread_cleanup_pop(r) ; } \ -__pthread_unregister_cancel(&__cb); \ -if (r) __f(__x); } while (0) +void _pthread_cleanup_push(struct __ptcb *, void (*)(void *), void *); +void _pthread_cleanup_pop(struct __ptcb *, int); +#define pthread_cleanup_push(f, x) do { struct __ptcb __cb; _pthread_cleanup_push(&__cb, f, x); +#define pthread_cleanup_pop(r) _pthread_cleanup_pop(&__cb, (r)); } while(0) #ifdef __cplusplus } diff --git a/src/thread/cancellation.c b/src/thread/cancellation.c index b02cdfb0..967705a9 100644 --- a/src/thread/cancellation.c +++ b/src/thread/cancellation.c @@ -1,23 +1,20 @@ #include "pthread_impl.h" -static void dummy(struct __ptcb *cb) -{ -} -weak_alias(dummy, __pthread_do_unwind); -weak_alias(dummy, __pthread_do_register); -weak_alias(dummy, __pthread_do_unregister); +void __do_cleanup_push(); +void __do_cleanup_pop(); -void __pthread_unwind_next(struct __ptcb *cb) +void _pthread_cleanup_push(struct __ptcb *cb, void (*f)(void *), void *x) { - __pthread_do_unwind(cb); + __do_cleanup_push(cb, f, x); } -void __pthread_register_cancel(struct __ptcb *cb) +void _pthread_cleanup_pop(struct __ptcb *cb, int run) { - __pthread_do_register(cb); + __do_cleanup_pop(cb, run); } -void __pthread_unregister_cancel(struct __ptcb *cb) +static void dummy() { - __pthread_do_unregister(cb); } +weak_alias(dummy, __do_cleanup_push); +weak_alias(dummy, __do_cleanup_pop); diff --git a/src/thread/i386/cancellation.s b/src/thread/i386/cancellation.s deleted file mode 100644 index 75dc6091..00000000 --- a/src/thread/i386/cancellation.s +++ /dev/null @@ -1,34 +0,0 @@ -.text -.global __pthread_register_cancel -.type __pthread_register_cancel,@function -__pthread_register_cancel: - pushl %eax - call __pthread_do_register - popl %eax - ret - -.global __pthread_unregister_cancel -.type __pthread_unregister_cancel,@function -__pthread_unregister_cancel: - pushl %eax - call __pthread_do_unregister - popl %eax - ret - -.global __pthread_unwind_next -.type __pthread_unwind_next,@function -__pthread_unwind_next: - pushl %eax - call __pthread_do_unwind - popl %eax -__pthread_do_unwind: -__pthread_do_register: -__pthread_do_unregister: - ret - -.weak __pthread_do_unwind -.weak __pthread_do_register -.weak __pthread_do_unregister -.type __pthread_do_unwind,@function -.type __pthread_do_register,@function -.type __pthread_do_unregister,@function diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c index 0189f028..87bf8166 100644 --- a/src/thread/pthread_create.c +++ b/src/thread/pthread_create.c @@ -8,14 +8,18 @@ weak_alias(dummy_0, __synccall_lock); weak_alias(dummy_0, __synccall_unlock); weak_alias(dummy_0, __pthread_tsd_run_dtors); -void __pthread_do_unwind(struct __ptcb *cb) +void pthread_exit(void *result) { pthread_t self = pthread_self(); int n; - if (cb->__next) { - self->cancelbuf = cb->__next->__next; - longjmp((void *)cb->__next->__jb, 1); + self->result = result; + + while (self->cancelbuf) { + void (*f)(void *) = self->cancelbuf->__f; + void *x = self->cancelbuf->__x; + self->cancelbuf = self->cancelbuf->__next; + f(x); } __pthread_tsd_run_dtors(); @@ -39,17 +43,19 @@ void __pthread_do_unwind(struct __ptcb *cb) __syscall(SYS_exit, 0); } -void __pthread_do_register(struct __ptcb *cb) +void __do_cleanup_push(struct __ptcb *cb, void (*f)(void *), void *x) { struct pthread *self = pthread_self(); + cb->__f = f; + cb->__x = x; cb->__next = self->cancelbuf; self->cancelbuf = cb; } -void __pthread_do_unregister(struct __ptcb *cb) +void __do_cleanup_pop(struct __ptcb *cb, int run) { - struct pthread *self = __pthread_self(); - self->cancelbuf = self->cancelbuf->__next; + __pthread_self()->cancelbuf = cb->__next; + if (run) cb->__f(cb->__x); } static int start(void *p) @@ -134,11 +140,3 @@ int pthread_create(pthread_t *res, const pthread_attr_t *attr, void *(*entry)(vo *res = new; return 0; } - -void pthread_exit(void *result) -{ - struct pthread *self = pthread_self(); - struct __ptcb cb = { .__next = self->cancelbuf }; - self->result = result; - __pthread_do_unwind(&cb); -} -- GitLab