From 2a0b873065edb304fa2d1c54f8de663ea638b8ab Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 8 Oct 2013 18:50:24 -0700 Subject: [PATCH] Fix __errno for LP64 and clean up __get_tls. If __get_tls has the right type, a lot of confusing casting can disappear. It was probably a mistake that __get_tls was exposed as a function for mips and x86 (but not arm), so let's (a) ensure that the __get_tls function always matches the macro, (b) that we have the function for arm too, and (c) that we don't have the function for any 64-bit architecture. Change-Id: Ie9cb989b66e2006524ad7733eb6e1a65055463be --- libc/Android.mk | 3 +- libc/arch-mips/mips.mk | 1 - libc/arch-x86/bionic/__get_tls.c | 37 ---------------- libc/arch-x86/x86.mk | 1 - libc/arch-x86_64/x86_64.mk | 1 - libc/bionic/{__errno.c => __errno.cpp} | 7 +-- .../__get_tls.c => bionic/__get_tls.cpp} | 9 ++-- libc/bionic/libc_init_dynamic.cpp | 2 +- libc/bionic/pthread_create.cpp | 2 + libc/bionic/pthread_internals.cpp | 3 +- libc/bionic/pthread_key.cpp | 7 ++- .../__get_tls.c => private/__get_tls.h} | 43 ++++++++++++++----- libc/private/bionic_tls.h | 41 +++--------------- linker/dlfcn.cpp | 3 +- 14 files changed, 56 insertions(+), 104 deletions(-) delete mode 100644 libc/arch-x86/bionic/__get_tls.c rename libc/bionic/{__errno.c => __errno.cpp} (92%) rename libc/{arch-x86_64/bionic/__get_tls.c => bionic/__get_tls.cpp} (90%) rename libc/{arch-mips/bionic/__get_tls.c => private/__get_tls.h} (59%) diff --git a/libc/Android.mk b/libc/Android.mk index f4fdb4c37..72a655ca9 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -185,6 +185,7 @@ ifeq ($(TARGET_ARCH),$(filter $(TARGET_ARCH),arm mips x86)) libc_common_src_files += \ bionic/fcntl.c \ bionic/fstatfs.c \ + bionic/__get_tls.cpp \ bionic/lseek64.c \ bionic/sigsuspend.c \ bionic/statfs.c \ @@ -217,7 +218,7 @@ libc_bionic_src_files := \ bionic/assert.cpp \ bionic/brk.cpp \ bionic/dirent.cpp \ - bionic/__errno.c \ + bionic/__errno.cpp \ bionic/eventfd_read.cpp \ bionic/eventfd_write.cpp \ bionic/futimens.cpp \ diff --git a/libc/arch-mips/mips.mk b/libc/arch-mips/mips.mk index fe57ee791..7a3c9789e 100644 --- a/libc/arch-mips/mips.mk +++ b/libc/arch-mips/mips.mk @@ -1,6 +1,5 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ arch-mips/bionic/__get_sp.S \ - arch-mips/bionic/__get_tls.c \ arch-mips/bionic/__set_tls.c \ arch-mips/bionic/_exit_with_stack_teardown.S \ arch-mips/bionic/_setjmp.S \ diff --git a/libc/arch-x86/bionic/__get_tls.c b/libc/arch-x86/bionic/__get_tls.c deleted file mode 100644 index 5ac6e448b..000000000 --- a/libc/arch-x86/bionic/__get_tls.c +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (C) 2008 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ -/* see the implementation of __set_tls and pthread.c to understand this - * code. Basically, the content of gs:[0] always is a pointer to the base - * address of the tls region - */ -void* __get_tls(void) -{ - void* tls; - asm ( " movl %%gs:0, %0" : "=r"(tls) ); - return tls; -} diff --git a/libc/arch-x86/x86.mk b/libc/arch-x86/x86.mk index 395f0d4f9..4a63f5ed0 100644 --- a/libc/arch-x86/x86.mk +++ b/libc/arch-x86/x86.mk @@ -3,7 +3,6 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ arch-x86/bionic/_exit_with_stack_teardown.S \ arch-x86/bionic/futex_x86.S \ arch-x86/bionic/__get_sp.S \ - arch-x86/bionic/__get_tls.c \ arch-x86/bionic/_setjmp.S \ arch-x86/bionic/setjmp.S \ arch-x86/bionic/__set_tls.c \ diff --git a/libc/arch-x86_64/x86_64.mk b/libc/arch-x86_64/x86_64.mk index d9c8f12d8..a1c170541 100644 --- a/libc/arch-x86_64/x86_64.mk +++ b/libc/arch-x86_64/x86_64.mk @@ -3,7 +3,6 @@ _LIBC_ARCH_COMMON_SRC_FILES := \ arch-x86_64/bionic/_exit_with_stack_teardown.S \ arch-x86_64/bionic/futex_x86_64.S \ arch-x86_64/bionic/__get_sp.S \ - arch-x86_64/bionic/__get_tls.c \ arch-x86_64/bionic/__rt_sigreturn.S \ arch-x86_64/bionic/_setjmp.S \ arch-x86_64/bionic/setjmp.S \ diff --git a/libc/bionic/__errno.c b/libc/bionic/__errno.cpp similarity index 92% rename from libc/bionic/__errno.c rename to libc/bionic/__errno.cpp index 8f33ccedd..9caa61812 100644 --- a/libc/bionic/__errno.c +++ b/libc/bionic/__errno.cpp @@ -25,10 +25,11 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ + #include #include +#include -volatile int* __errno( void ) -{ - return &((volatile int*)__get_tls())[TLS_SLOT_ERRNO]; +volatile int* __errno() { + return reinterpret_cast(&(__get_tls()[TLS_SLOT_ERRNO])); } diff --git a/libc/arch-x86_64/bionic/__get_tls.c b/libc/bionic/__get_tls.cpp similarity index 90% rename from libc/arch-x86_64/bionic/__get_tls.c rename to libc/bionic/__get_tls.cpp index f21b6b8c8..d01e2aa49 100644 --- a/libc/arch-x86_64/bionic/__get_tls.c +++ b/libc/bionic/__get_tls.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013 The Android Open Source Project + * Copyright (C) 2008 The Android Open Source Project * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,8 +26,7 @@ * SUCH DAMAGE. */ -void* __get_tls(void) { - void* tls; - asm ("mov %%fs:0, %0" : "=r"(tls)); - return tls; +extern "C" void** __get_tls() { +#include "private/__get_tls.h" + return __get_tls(); } diff --git a/libc/bionic/libc_init_dynamic.cpp b/libc/bionic/libc_init_dynamic.cpp index 88e87a712..4e1374e3c 100644 --- a/libc/bionic/libc_init_dynamic.cpp +++ b/libc/bionic/libc_init_dynamic.cpp @@ -65,7 +65,7 @@ extern "C" { // as soon as the shared library is loaded. __attribute__((constructor)) static void __libc_preinit() { // Read the kernel argument block pointer from TLS. - void* tls = const_cast(__get_tls()); + void** tls = __get_tls(); KernelArgumentBlock** args_slot = &reinterpret_cast(tls)[TLS_SLOT_BIONIC_PREINIT]; KernelArgumentBlock* args = *args_slot; diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp index 63695d37c..9e06afc9e 100644 --- a/libc/bionic/pthread_create.cpp +++ b/libc/bionic/pthread_create.cpp @@ -50,6 +50,8 @@ extern "C" int __pthread_clone(void* (*fn)(void*), void* child_stack, int flags, extern "C" void ATTRIBUTES _thread_created_hook(pid_t thread_id); +extern "C" int __set_tls(void* ptr); + static const int kPthreadInitFailed = 1; static pthread_mutex_t gPthreadStackCreationLock = PTHREAD_MUTEX_INITIALIZER; diff --git a/libc/bionic/pthread_internals.cpp b/libc/bionic/pthread_internals.cpp index 66bc5b75d..59c6e4819 100644 --- a/libc/bionic/pthread_internals.cpp +++ b/libc/bionic/pthread_internals.cpp @@ -64,6 +64,5 @@ __LIBC_ABI_PRIVATE__ void _pthread_internal_add(pthread_internal_t* thread) { } __LIBC_ABI_PRIVATE__ pthread_internal_t* __get_thread(void) { - void** tls = reinterpret_cast(const_cast(__get_tls())); - return reinterpret_cast(tls[TLS_SLOT_THREAD_ID]); + return reinterpret_cast(__get_tls()[TLS_SLOT_THREAD_ID]); } diff --git a/libc/bionic/pthread_key.cpp b/libc/bionic/pthread_key.cpp index 7e8b4cdbf..706758b58 100644 --- a/libc/bionic/pthread_key.cpp +++ b/libc/bionic/pthread_key.cpp @@ -133,7 +133,7 @@ class ScopedTlsMapAccess { // from this thread's TLS area. This must call the destructor of all keys // that have a non-NULL data value and a non-NULL destructor. void CleanAll() { - void** tls = (void**)__get_tls(); + void** tls = __get_tls(); // Because destructors can do funky things like deleting/creating other // keys, we need to implement this in a loop. @@ -239,8 +239,7 @@ void* pthread_getspecific(pthread_key_t key) { // to check that the key is properly allocated. If the key was not // allocated, the value read from the TLS should always be NULL // due to pthread_key_delete() clearing the values for all threads. - uintptr_t address = reinterpret_cast(__get_tls())[key]; - return reinterpret_cast(address); + return __get_tls()[key]; } int pthread_setspecific(pthread_key_t key, const void* ptr) { @@ -250,6 +249,6 @@ int pthread_setspecific(pthread_key_t key, const void* ptr) { return EINVAL; } - reinterpret_cast(__get_tls())[key] = reinterpret_cast(ptr); + __get_tls()[key] = const_cast(ptr); return 0; } diff --git a/libc/arch-mips/bionic/__get_tls.c b/libc/private/__get_tls.h similarity index 59% rename from libc/arch-mips/bionic/__get_tls.c rename to libc/private/__get_tls.h index d1cdf0e54..5f9451d28 100644 --- a/libc/arch-mips/bionic/__get_tls.c +++ b/libc/private/__get_tls.h @@ -25,13 +25,36 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ -void* __get_tls(void) -{ - register void *tls asm("v1"); - asm (".set push\n\t" - ".set mips32r2\n\t" - "rdhwr %0,$29\n\t" - ".set pop" - : "=r"(tls)); - return tls; -} + +#ifndef __BIONIC_PRIVATE_GET_TLS_H_ +#define __BIONIC_PRIVATE_GET_TLS_H_ + +#if defined(__arm__) +# define __get_tls() \ + ({ void** __val; \ + __asm__("mrc p15, 0, %0, c13, c0, 3" : "=r"(__val)); \ + __val; }) +#elif defined(__mips__) +# define __get_tls() \ + /* On mips32r1, this goes via a kernel illegal instruction trap that's optimized for v1. */ \ + ({ register void** __val asm("v1"); \ + __asm__(".set push\n" \ + ".set mips32r2\n" \ + "rdhwr %0,$29\n" \ + ".set pop\n" : "=r"(__val)); \ + __val; }) +#elif defined(__i386__) +# define __get_tls() \ + ({ void** __val; \ + __asm__("movl %%gs:0, %0" : "=r"(__val)); \ + __val; }) +#elif defined(__x86_64__) +# define __get_tls() \ + ({ void** __val; \ + __asm__("mov %%fs:0, %0" : "=r"(__val)); \ + __val; }) +#else +#error unsupported architecture +#endif + +#endif /* __BIONIC_PRIVATE_GET_TLS_H_ */ diff --git a/libc/private/bionic_tls.h b/libc/private/bionic_tls.h index 2e203648e..a14bd3c38 100644 --- a/libc/private/bionic_tls.h +++ b/libc/private/bionic_tls.h @@ -25,10 +25,12 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ -#ifndef _SYS_TLS_H -#define _SYS_TLS_H + +#ifndef __BIONIC_PRIVATE_BIONIC_TLS_H_ +#define __BIONIC_PRIVATE_BIONIC_TLS_H_ #include +#include "__get_tls.h" __BEGIN_DECLS @@ -83,39 +85,6 @@ enum { #define BIONIC_ALIGN(x, a) (((x) + (a - 1)) & ~(a - 1)) #define BIONIC_TLS_SLOTS BIONIC_ALIGN(128 + TLS_SLOT_FIRST_USER_SLOT + GLOBAL_INIT_THREAD_LOCAL_BUFFER_COUNT, 4) -/* syscall only, do not call directly */ -extern int __set_tls(void* ptr); - -/* get the TLS */ -#if defined(__arm__) -# define __get_tls() \ - ({ unsigned int __val; \ - asm ("mrc p15, 0, %0, c13, c0, 3" : "=r"(__val)); \ - (volatile void*) __val; }) -#elif defined(__mips__) -# define __get_tls() \ - /* On mips32r1, this goes via a kernel illegal instruction trap that's optimized for v1. */ \ - ({ register unsigned int __val asm("v1"); \ - asm (" .set push\n" \ - " .set mips32r2\n" \ - " rdhwr %0,$29\n" \ - " .set pop\n" : "=r"(__val)); \ - (volatile void*) __val; }) -#elif defined(__i386__) -# define __get_tls() \ - ({ void* __val; \ - asm ("movl %%gs:0, %0" : "=r"(__val)); \ - (volatile void*) __val; }) - -#elif defined(__x86_64__) -# define __get_tls() \ - ({ void* __val; \ - asm ("mov %%fs:0, %0" : "=r"(__val)); \ - (volatile void*) __val; }) -#else -#error unsupported architecture -#endif - __END_DECLS #if defined(__cplusplus) @@ -123,4 +92,4 @@ class KernelArgumentBlock; extern __LIBC_HIDDEN__ void __libc_init_tls(KernelArgumentBlock& args); #endif -#endif /* _SYS_TLS_H */ +#endif /* __BIONIC_PRIVATE_BIONIC_TLS_H_ */ diff --git a/linker/dlfcn.cpp b/linker/dlfcn.cpp index a61cfa8f6..3553af7d8 100644 --- a/linker/dlfcn.cpp +++ b/linker/dlfcn.cpp @@ -31,8 +31,7 @@ static pthread_mutex_t gDlMutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER; static const char* __bionic_set_dlerror(char* new_value) { - void* tls = const_cast(__get_tls()); - char** dlerror_slot = &reinterpret_cast(tls)[TLS_SLOT_DLERROR]; + char** dlerror_slot = &reinterpret_cast(__get_tls())[TLS_SLOT_DLERROR]; const char* old_value = *dlerror_slot; *dlerror_slot = new_value;