From 36d6188f8cd8b948fb797f11d9620d63d0c2215a Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 19 Nov 2013 13:31:58 -0800 Subject: [PATCH] Clean up forking and cloning. The kernel now maintains the pthread_internal_t::tid field for us, and __clone was only used in one place so let's inline it so we don't have to leave such a dangerous function lying around. Also rename files to match their content and remove some useless #includes. Change-Id: I24299fb4a940e394de75f864ee36fdabbd9438f9 --- libc/Android.mk | 2 +- libc/SYSCALLS.TXT | 5 +- libc/arch-aarch64/syscalls.mk | 1 - libc/arch-aarch64/syscalls/__clone.S | 22 --------- libc/arch-arm/arm.mk | 2 +- .../bionic/{clone.S => __bionic_clone.S} | 0 libc/arch-arm/syscalls.mk | 1 - libc/arch-arm/syscalls/__clone.S | 17 ------- .../bionic/{clone.S => __bionic_clone.S} | 0 libc/arch-mips/mips.mk | 2 +- libc/arch-mips/syscalls.mk | 1 - libc/arch-mips/syscalls/__clone.S | 23 ---------- .../bionic/{clone.S => __bionic_clone.S} | 0 libc/arch-x86/syscalls.mk | 1 - libc/arch-x86/syscalls/__clone.S | 32 ------------- libc/arch-x86/x86.mk | 2 +- .../bionic/{clone.S => __bionic_clone.S} | 5 ++ libc/arch-x86_64/syscalls.mk | 1 - libc/arch-x86_64/syscalls/__clone.S | 18 -------- libc/arch-x86_64/x86_64.mk | 2 +- libc/bionic/{bionic_clone.c => clone.cpp} | 46 +++++++++---------- libc/bionic/fork.cpp | 22 ++++----- libc/bionic/pthread_cond.cpp | 1 - libc/bionic/pthread_internal.h | 1 - libc/bionic/pthread_internals.cpp | 8 ---- libc/bionic/pthread_mutex.cpp | 1 - 26 files changed, 44 insertions(+), 172 deletions(-) delete mode 100644 libc/arch-aarch64/syscalls/__clone.S rename libc/arch-arm/bionic/{clone.S => __bionic_clone.S} (100%) delete mode 100644 libc/arch-arm/syscalls/__clone.S rename libc/arch-mips/bionic/{clone.S => __bionic_clone.S} (100%) delete mode 100644 libc/arch-mips/syscalls/__clone.S rename libc/arch-x86/bionic/{clone.S => __bionic_clone.S} (100%) delete mode 100644 libc/arch-x86/syscalls/__clone.S rename libc/arch-x86_64/bionic/{clone.S => __bionic_clone.S} (91%) delete mode 100644 libc/arch-x86_64/syscalls/__clone.S rename libc/bionic/{bionic_clone.c => clone.cpp} (61%) diff --git a/libc/Android.mk b/libc/Android.mk index ab0908911..551c6338d 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -77,7 +77,6 @@ libc_common_src_files := \ bionic/atol.c \ bionic/atoll.c \ bionic/bindresvport.c \ - bionic/bionic_clone.c \ bionic/clearenv.c \ bionic/daemon.c \ bionic/err.c \ @@ -207,6 +206,7 @@ libc_bionic_src_files := \ bionic/brk.cpp \ bionic/chmod.cpp \ bionic/chown.cpp \ + bionic/clone.cpp \ bionic/dirent.cpp \ bionic/dup2.cpp \ bionic/epoll_create.cpp \ diff --git a/libc/SYSCALLS.TXT b/libc/SYSCALLS.TXT index a02702405..c5dc4029c 100644 --- a/libc/SYSCALLS.TXT +++ b/libc/SYSCALLS.TXT @@ -275,9 +275,6 @@ int sysinfo(struct sysinfo*) all int personality(unsigned long) all long perf_event_open(struct perf_event_attr* attr_uptr, pid_t pid, int cpu, int group_fd, unsigned long flags) all -pid_t __clone:clone(int, void*, int*, void*, int*) all -int __set_tid_address:set_tid_address(int*) all - int epoll_create1(int) all int epoll_ctl(int, int op, int, struct epoll_event*) all int __epoll_pwait:epoll_pwait(int, struct epoll_event*, int, int, const sigset_t*, size_t) all @@ -296,6 +293,8 @@ int inotify_rm_watch(int, unsigned int) all int __pselect6:pselect6(int, fd_set*, fd_set*, fd_set*, timespec*, void*) all int __ppoll:ppoll(pollfd*, unsigned int, timespec*, const sigset_t*, size_t) all +int __set_tid_address:set_tid_address(int*) all + pid_t wait4(pid_t, int*, int, struct rusage*) all int __waitid:waitid(int, pid_t, struct siginfo_t*, int, void*) all diff --git a/libc/arch-aarch64/syscalls.mk b/libc/arch-aarch64/syscalls.mk index 890a6d82c..ab9c839d1 100644 --- a/libc/arch-aarch64/syscalls.mk +++ b/libc/arch-aarch64/syscalls.mk @@ -1,7 +1,6 @@ # Generated by gensyscalls.py. Do not edit. syscall_src := syscall_src += arch-aarch64/syscalls/__brk.S -syscall_src += arch-aarch64/syscalls/__clone.S syscall_src += arch-aarch64/syscalls/__epoll_pwait.S syscall_src += arch-aarch64/syscalls/__exit.S syscall_src += arch-aarch64/syscalls/__getcpu.S diff --git a/libc/arch-aarch64/syscalls/__clone.S b/libc/arch-aarch64/syscalls/__clone.S deleted file mode 100644 index 45c202249..000000000 --- a/libc/arch-aarch64/syscalls/__clone.S +++ /dev/null @@ -1,22 +0,0 @@ -/* Generated by gensyscalls.py. Do not edit. */ - -#include - -ENTRY(__clone) - stp x29, x30, [sp, #-16]! - mov x29, sp - str x8, [sp, #-16]! - - mov x8, __NR_clone - svc #0 - - ldr x8, [sp], #16 - ldp x29, x30, [sp], #16 - - cmn x0, #(MAX_ERRNO + 1) - cneg x0, x0, hi - b.hi __set_errno - - ret -END(__clone) -.hidden _C_LABEL(__clone) diff --git a/libc/arch-arm/arm.mk b/libc/arch-arm/arm.mk index dea303aff..0717306ce 100644 --- a/libc/arch-arm/arm.mk +++ b/libc/arch-arm/arm.mk @@ -1,7 +1,7 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ arch-arm/bionic/abort_arm.S \ arch-arm/bionic/atomics_arm.c \ - arch-arm/bionic/clone.S \ + arch-arm/bionic/__bionic_clone.S \ arch-arm/bionic/eabi.c \ arch-arm/bionic/_exit_with_stack_teardown.S \ arch-arm/bionic/futex_arm.S \ diff --git a/libc/arch-arm/bionic/clone.S b/libc/arch-arm/bionic/__bionic_clone.S similarity index 100% rename from libc/arch-arm/bionic/clone.S rename to libc/arch-arm/bionic/__bionic_clone.S diff --git a/libc/arch-arm/syscalls.mk b/libc/arch-arm/syscalls.mk index f8bb15a99..c0438b4fa 100644 --- a/libc/arch-arm/syscalls.mk +++ b/libc/arch-arm/syscalls.mk @@ -1,7 +1,6 @@ # Generated by gensyscalls.py. Do not edit. syscall_src := syscall_src += arch-arm/syscalls/__brk.S -syscall_src += arch-arm/syscalls/__clone.S syscall_src += arch-arm/syscalls/__epoll_pwait.S syscall_src += arch-arm/syscalls/__exit.S syscall_src += arch-arm/syscalls/__fcntl64.S diff --git a/libc/arch-arm/syscalls/__clone.S b/libc/arch-arm/syscalls/__clone.S deleted file mode 100644 index fb074bc26..000000000 --- a/libc/arch-arm/syscalls/__clone.S +++ /dev/null @@ -1,17 +0,0 @@ -/* Generated by gensyscalls.py. Do not edit. */ - -#include - -ENTRY(__clone) - mov ip, sp - .save {r4, r5, r6, r7} - stmfd sp!, {r4, r5, r6, r7} - ldmfd ip, {r4, r5, r6} - ldr r7, =__NR_clone - swi #0 - ldmfd sp!, {r4, r5, r6, r7} - cmn r0, #(MAX_ERRNO + 1) - bxls lr - neg r0, r0 - b __set_errno -END(__clone) diff --git a/libc/arch-mips/bionic/clone.S b/libc/arch-mips/bionic/__bionic_clone.S similarity index 100% rename from libc/arch-mips/bionic/clone.S rename to libc/arch-mips/bionic/__bionic_clone.S diff --git a/libc/arch-mips/mips.mk b/libc/arch-mips/mips.mk index c8953576b..1119a66be 100644 --- a/libc/arch-mips/mips.mk +++ b/libc/arch-mips/mips.mk @@ -1,7 +1,7 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ + arch-mips/bionic/__bionic_clone.S \ arch-mips/bionic/bzero.S \ arch-mips/bionic/cacheflush.cpp \ - arch-mips/bionic/clone.S \ arch-mips/bionic/_exit_with_stack_teardown.S \ arch-mips/bionic/futex_mips.S \ arch-mips/bionic/__get_sp.S \ diff --git a/libc/arch-mips/syscalls.mk b/libc/arch-mips/syscalls.mk index 6b72e704b..40932b712 100644 --- a/libc/arch-mips/syscalls.mk +++ b/libc/arch-mips/syscalls.mk @@ -1,7 +1,6 @@ # Generated by gensyscalls.py. Do not edit. syscall_src := syscall_src += arch-mips/syscalls/__brk.S -syscall_src += arch-mips/syscalls/__clone.S syscall_src += arch-mips/syscalls/__epoll_pwait.S syscall_src += arch-mips/syscalls/__exit.S syscall_src += arch-mips/syscalls/__fcntl64.S diff --git a/libc/arch-mips/syscalls/__clone.S b/libc/arch-mips/syscalls/__clone.S deleted file mode 100644 index 5ad14897f..000000000 --- a/libc/arch-mips/syscalls/__clone.S +++ /dev/null @@ -1,23 +0,0 @@ -/* Generated by gensyscalls.py. Do not edit. */ - -#include - .text - .globl __clone - .align 4 - .ent __clone - -__clone: - .set noreorder - .cpload $t9 - li $v0, __NR_clone - syscall - bnez $a3, 1f - move $a0, $v0 - j $ra - nop -1: - la $t9,__set_errno - j $t9 - nop - .set reorder - .end __clone diff --git a/libc/arch-x86/bionic/clone.S b/libc/arch-x86/bionic/__bionic_clone.S similarity index 100% rename from libc/arch-x86/bionic/clone.S rename to libc/arch-x86/bionic/__bionic_clone.S diff --git a/libc/arch-x86/syscalls.mk b/libc/arch-x86/syscalls.mk index b6a1e3807..780972f1f 100644 --- a/libc/arch-x86/syscalls.mk +++ b/libc/arch-x86/syscalls.mk @@ -1,7 +1,6 @@ # Generated by gensyscalls.py. Do not edit. syscall_src := syscall_src += arch-x86/syscalls/__brk.S -syscall_src += arch-x86/syscalls/__clone.S syscall_src += arch-x86/syscalls/__epoll_pwait.S syscall_src += arch-x86/syscalls/__exit.S syscall_src += arch-x86/syscalls/__fcntl64.S diff --git a/libc/arch-x86/syscalls/__clone.S b/libc/arch-x86/syscalls/__clone.S deleted file mode 100644 index 2ff8fcd15..000000000 --- a/libc/arch-x86/syscalls/__clone.S +++ /dev/null @@ -1,32 +0,0 @@ -/* Generated by gensyscalls.py. Do not edit. */ - -#include - -ENTRY(__clone) - pushl %ebx - pushl %ecx - pushl %edx - pushl %esi - pushl %edi - mov 24(%esp), %ebx - mov 28(%esp), %ecx - mov 32(%esp), %edx - mov 36(%esp), %esi - mov 40(%esp), %edi - movl $__NR_clone, %eax - int $0x80 - cmpl $-MAX_ERRNO, %eax - jb 1f - negl %eax - pushl %eax - call __set_errno - addl $4, %esp - orl $-1, %eax -1: - popl %edi - popl %esi - popl %edx - popl %ecx - popl %ebx - ret -END(__clone) diff --git a/libc/arch-x86/x86.mk b/libc/arch-x86/x86.mk index 5911ad9f1..9c7408496 100644 --- a/libc/arch-x86/x86.mk +++ b/libc/arch-x86/x86.mk @@ -1,5 +1,5 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ - arch-x86/bionic/clone.S \ + arch-x86/bionic/__bionic_clone.S \ arch-x86/bionic/_exit_with_stack_teardown.S \ arch-x86/bionic/futex_x86.S \ arch-x86/bionic/__get_sp.S \ diff --git a/libc/arch-x86_64/bionic/clone.S b/libc/arch-x86_64/bionic/__bionic_clone.S similarity index 91% rename from libc/arch-x86_64/bionic/clone.S rename to libc/arch-x86_64/bionic/__bionic_clone.S index b37416b44..309c36539 100644 --- a/libc/arch-x86_64/bionic/clone.S +++ b/libc/arch-x86_64/bionic/__bionic_clone.S @@ -40,8 +40,13 @@ ENTRY(__bionic_clone) movq %rax, -8(%rsi) # Write 'arg'. subq $16, %rsi + + # Translate to the kernel calling convention and swap the 'tls' and 'child_tid' arguments. + # They're flipped for x86-64 compared to all our other architectures and __bionic_clone. movq %r8, %r10 movq %rcx, %r8 + + # Make the system call. movl $__NR_clone, %eax syscall testl %eax, %eax diff --git a/libc/arch-x86_64/syscalls.mk b/libc/arch-x86_64/syscalls.mk index 50d9ab3a1..9e2bfccfe 100644 --- a/libc/arch-x86_64/syscalls.mk +++ b/libc/arch-x86_64/syscalls.mk @@ -2,7 +2,6 @@ syscall_src := syscall_src += arch-x86_64/syscalls/__arch_prctl.S syscall_src += arch-x86_64/syscalls/__brk.S -syscall_src += arch-x86_64/syscalls/__clone.S syscall_src += arch-x86_64/syscalls/__epoll_pwait.S syscall_src += arch-x86_64/syscalls/__exit.S syscall_src += arch-x86_64/syscalls/__getcpu.S diff --git a/libc/arch-x86_64/syscalls/__clone.S b/libc/arch-x86_64/syscalls/__clone.S deleted file mode 100644 index 4999762bf..000000000 --- a/libc/arch-x86_64/syscalls/__clone.S +++ /dev/null @@ -1,18 +0,0 @@ -/* Generated by gensyscalls.py. Do not edit. */ - -#include - -ENTRY(__clone) - movq %rcx, %r10 - movl $__NR_clone, %eax - syscall - cmpq $-MAX_ERRNO, %rax - jb 1f - negl %eax - movl %eax, %edi - call __set_errno - orq $-1, %rax -1: - ret -END(__clone) -.hidden _C_LABEL(__clone) diff --git a/libc/arch-x86_64/x86_64.mk b/libc/arch-x86_64/x86_64.mk index 4f833c15b..261453784 100644 --- a/libc/arch-x86_64/x86_64.mk +++ b/libc/arch-x86_64/x86_64.mk @@ -1,5 +1,5 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ - arch-x86_64/bionic/clone.S \ + arch-x86_64/bionic/__bionic_clone.S \ arch-x86_64/bionic/_exit_with_stack_teardown.S \ arch-x86_64/bionic/futex_x86_64.S \ arch-x86_64/bionic/__get_sp.S \ diff --git a/libc/bionic/bionic_clone.c b/libc/bionic/clone.cpp similarity index 61% rename from libc/bionic/bionic_clone.c rename to libc/bionic/clone.cpp index 518d99634..1d997fed7 100644 --- a/libc/bionic/bionic_clone.c +++ b/libc/bionic/clone.cpp @@ -25,41 +25,39 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ + #define __GNU_SOURCE 1 #include #include #include -#include -extern pid_t __bionic_clone(uint32_t flags, void* child_stack, int* parent_tid, void* tls, int* child_tid, int (*fn)(void*), void* arg); -extern void __exit(int status); +extern "C" pid_t __bionic_clone(uint32_t flags, void* child_stack, int* parent_tid, void* tls, int* child_tid, int (*fn)(void*), void* arg); +extern "C" void __exit(int status); -/* this function is called from the __bionic_clone - * assembly fragment to call the thread function - * then exit. */ -extern void __bionic_clone_entry(int (*fn)(void*), void* arg) { +// Called from the __bionic_clone assembler to call the thread function then exit. +extern "C" void __bionic_clone_entry(int (*fn)(void*), void* arg) { int status = (*fn)(arg); __exit(status); } int clone(int (*fn)(void*), void* child_stack, int flags, void* arg, ...) { - va_list args; - int *parent_tidptr = NULL; - void *new_tls = NULL; - int *child_tidptr = NULL; + int* parent_tid = NULL; + void* new_tls = NULL; + int* child_tid = NULL; - /* extract optional parameters - they are cumulative. */ - va_start(args, arg); - if (flags & (CLONE_PARENT_SETTID|CLONE_SETTLS|CLONE_CHILD_SETTID)) { - parent_tidptr = va_arg(args, int*); - } - if (flags & (CLONE_SETTLS|CLONE_CHILD_SETTID)) { - new_tls = va_arg(args, void*); - } - if (flags & CLONE_CHILD_SETTID) { - child_tidptr = va_arg(args, int*); - } - va_end(args); + // Extract any optional parameters required by the flags. + va_list args; + va_start(args, arg); + if ((flags & (CLONE_PARENT_SETTID|CLONE_SETTLS|CLONE_CHILD_SETTID|CLONE_CHILD_CLEARTID)) != 0) { + parent_tid = va_arg(args, int*); + } + if ((flags & (CLONE_SETTLS|CLONE_CHILD_SETTID|CLONE_CHILD_CLEARTID)) != 0) { + new_tls = va_arg(args, void*); + } + if ((flags & (CLONE_CHILD_SETTID|CLONE_CHILD_CLEARTID)) != 0) { + child_tid = va_arg(args, int*); + } + va_end(args); - return __bionic_clone(flags, child_stack, parent_tidptr, new_tls, child_tidptr, fn, arg); + return __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg); } diff --git a/libc/bionic/fork.cpp b/libc/bionic/fork.cpp index f7d1c11c3..9fa5fcf5f 100644 --- a/libc/bionic/fork.cpp +++ b/libc/bionic/fork.cpp @@ -27,12 +27,11 @@ */ #include +#include + +#include "private/libc_logging.h" #include "pthread_internal.h" -#include "private/bionic_pthread.h" - -extern "C" int __clone(int, void*, int*, void*, int*); - int fork() { // POSIX mandates that the timers of a fork child process be // disarmed, but not destroyed. To avoid a race condition, we're @@ -42,18 +41,17 @@ int fork() { __bionic_atfork_run_prepare(); pthread_internal_t* self = __get_thread(); -#if defined(__x86_64__) - int result = __clone(CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, NULL, NULL, &(self->tid), NULL); + int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD; +#if defined(__x86_64__) // sys_clone's last two arguments are flipped on x86-64. + int result = syscall(__NR_clone, flags, NULL, NULL, &(self->tid), NULL); #else - int result = __clone(CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, NULL, NULL, NULL, &(self->tid)); + int result = syscall(__NR_clone, flags, NULL, NULL, NULL, &(self->tid)); #endif - if (result != 0) { // Not a child process. + if (result == 0) { + __bionic_atfork_run_child(); + } else { __timer_table_start_stop(0); __bionic_atfork_run_parent(); - } else { - // Fix the tid in the pthread_internal_t struct after a fork. - __pthread_settid(pthread_self(), gettid()); - __bionic_atfork_run_child(); } return result; } diff --git a/libc/bionic/pthread_cond.cpp b/libc/bionic/pthread_cond.cpp index abd453e8f..7c229b5ec 100644 --- a/libc/bionic/pthread_cond.cpp +++ b/libc/bionic/pthread_cond.cpp @@ -38,7 +38,6 @@ #include "private/bionic_atomic_inline.h" #include "private/bionic_futex.h" -#include "private/bionic_pthread.h" #include "private/bionic_time_conversions.h" #include "private/bionic_tls.h" #include "private/thread_private.h" diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index de1ef26b9..bcd9b7104 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -97,6 +97,5 @@ __LIBC_HIDDEN__ extern void __timer_table_start_stop(int); __LIBC_HIDDEN__ extern void __bionic_atfork_run_prepare(); __LIBC_HIDDEN__ extern void __bionic_atfork_run_child(); __LIBC_HIDDEN__ extern void __bionic_atfork_run_parent(); -__LIBC_HIDDEN__ extern int __pthread_settid(pthread_t, pid_t); #endif /* _PTHREAD_INTERNAL_H_ */ diff --git a/libc/bionic/pthread_internals.cpp b/libc/bionic/pthread_internals.cpp index 4b1f6efb9..f8afeaf47 100644 --- a/libc/bionic/pthread_internals.cpp +++ b/libc/bionic/pthread_internals.cpp @@ -73,14 +73,6 @@ pid_t __pthread_gettid(pthread_t t) { return reinterpret_cast(t)->tid; } -int __pthread_settid(pthread_t t, pid_t tid) { - if (t == 0) { - return EINVAL; - } - reinterpret_cast(t)->tid = tid; - return 0; -} - // Initialize 'ts' with the difference between 'abstime' and the current time // according to 'clock'. Returns -1 if abstime already expired, or 0 otherwise. int __timespec_to_absolute(timespec* ts, const timespec* abstime, clockid_t clock) { diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp index 7e43cd841..1010f11d4 100644 --- a/libc/bionic/pthread_mutex.cpp +++ b/libc/bionic/pthread_mutex.cpp @@ -38,7 +38,6 @@ #include "private/bionic_atomic_inline.h" #include "private/bionic_futex.h" -#include "private/bionic_pthread.h" #include "private/bionic_tls.h" #include "private/thread_private.h"