From 6170693e28dd72a1517c267f3f62b3f37477b8bb Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 31 Mar 2015 10:56:58 -0700 Subject: [PATCH] Make ThreadLocalBuffer a class rather than a macro. Bug: 19995392 Change-Id: I497c512648fbe66257da3fb3bcd5c9911f983705 --- libc/bionic/libgen.cpp | 18 +++++++-------- libc/bionic/mntent.cpp | 11 +++++---- libc/bionic/pty.cpp | 16 +++++++------- libc/bionic/strerror.cpp | 10 ++++----- libc/bionic/strsignal.cpp | 6 ++--- libc/bionic/stubs.cpp | 30 ++++++++++--------------- libc/dns/resolv/res_state.c | 8 ++++--- libc/private/ThreadLocalBuffer.h | 48 +++++++++++++++++++--------------------- libc/private/bionic_tls.h | 30 ++++++++++++++----------- tests/pthread_test.cpp | 3 +-- 10 files changed, 88 insertions(+), 92 deletions(-) diff --git a/libc/bionic/libgen.cpp b/libc/bionic/libgen.cpp index b98f504b0..2f29d7b63 100644 --- a/libc/bionic/libgen.cpp +++ b/libc/bionic/libgen.cpp @@ -36,6 +36,9 @@ #include "private/ThreadLocalBuffer.h" +static ThreadLocalBuffer g_basename_tls_buffer; +static ThreadLocalBuffer g_dirname_tls_buffer; + __LIBC64_HIDDEN__ int basename_r(const char* path, char* buffer, size_t buffer_size) { const char* startp = NULL; const char* endp = NULL; @@ -147,17 +150,14 @@ __LIBC64_HIDDEN__ int dirname_r(const char* path, char* buffer, size_t buffer_si return result; } -GLOBAL_INIT_THREAD_LOCAL_BUFFER(basename); -GLOBAL_INIT_THREAD_LOCAL_BUFFER(dirname); - char* basename(const char* path) { - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, basename, MAXPATHLEN); - int rc = basename_r(path, basename_tls_buffer, basename_tls_buffer_size); - return (rc < 0) ? NULL : basename_tls_buffer; + char* buf = g_basename_tls_buffer.get(); + int rc = basename_r(path, buf, g_basename_tls_buffer.size()); + return (rc < 0) ? NULL : buf; } char* dirname(const char* path) { - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, dirname, MAXPATHLEN); - int rc = dirname_r(path, dirname_tls_buffer, dirname_tls_buffer_size); - return (rc < 0) ? NULL : dirname_tls_buffer; + char* buf = g_dirname_tls_buffer.get(); + int rc = dirname_r(path, buf, g_dirname_tls_buffer.size()); + return (rc < 0) ? NULL : buf; } diff --git a/libc/bionic/mntent.cpp b/libc/bionic/mntent.cpp index 4afacda60..d169e2930 100644 --- a/libc/bionic/mntent.cpp +++ b/libc/bionic/mntent.cpp @@ -31,14 +31,13 @@ #include "private/ThreadLocalBuffer.h" -GLOBAL_INIT_THREAD_LOCAL_BUFFER(getmntent_mntent); -GLOBAL_INIT_THREAD_LOCAL_BUFFER(getmntent_strings); +static ThreadLocalBuffer g_getmntent_mntent_tls_buffer; +static ThreadLocalBuffer g_getmntent_strings_tls_buffer; mntent* getmntent(FILE* fp) { - LOCAL_INIT_THREAD_LOCAL_BUFFER(mntent*, getmntent_mntent, sizeof(mntent)); - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, getmntent_strings, BUFSIZ); - return getmntent_r(fp, getmntent_mntent_tls_buffer, - getmntent_strings_tls_buffer, getmntent_strings_tls_buffer_size); + return getmntent_r(fp, g_getmntent_mntent_tls_buffer.get(), + g_getmntent_strings_tls_buffer.get(), + g_getmntent_strings_tls_buffer.size()); } mntent* getmntent_r(FILE* fp, struct mntent* e, char* buf, int buf_len) { diff --git a/libc/bionic/pty.cpp b/libc/bionic/pty.cpp index 884714799..1a3784753 100644 --- a/libc/bionic/pty.cpp +++ b/libc/bionic/pty.cpp @@ -38,8 +38,8 @@ #include "private/ThreadLocalBuffer.h" -GLOBAL_INIT_THREAD_LOCAL_BUFFER(ptsname); -GLOBAL_INIT_THREAD_LOCAL_BUFFER(ttyname); +static ThreadLocalBuffer g_ptsname_tls_buffer; +static ThreadLocalBuffer g_ttyname_tls_buffer; int getpt() { return posix_openpt(O_RDWR|O_NOCTTY); @@ -54,9 +54,9 @@ int posix_openpt(int flags) { } char* ptsname(int fd) { - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, ptsname, 32); - int error = ptsname_r(fd, ptsname_tls_buffer, ptsname_tls_buffer_size); - return (error == 0) ? ptsname_tls_buffer : NULL; + char* buf = g_ptsname_tls_buffer.get(); + int error = ptsname_r(fd, buf, g_ptsname_tls_buffer.size()); + return (error == 0) ? buf : NULL; } int ptsname_r(int fd, char* buf, size_t len) { @@ -80,9 +80,9 @@ int ptsname_r(int fd, char* buf, size_t len) { } char* ttyname(int fd) { - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, ttyname, 64); - int error = ttyname_r(fd, ttyname_tls_buffer, ttyname_tls_buffer_size); - return (error == 0) ? ttyname_tls_buffer : NULL; + char* buf = g_ttyname_tls_buffer.get(); + int error = ttyname_r(fd, buf, g_ttyname_tls_buffer.size()); + return (error == 0) ? buf : NULL; } int ttyname_r(int fd, char* buf, size_t len) { diff --git a/libc/bionic/strerror.cpp b/libc/bionic/strerror.cpp index d1518ffc4..f74194fd9 100644 --- a/libc/bionic/strerror.cpp +++ b/libc/bionic/strerror.cpp @@ -31,16 +31,16 @@ extern "C" const char* __strerror_lookup(int); -GLOBAL_INIT_THREAD_LOCAL_BUFFER(strerror); +static ThreadLocalBuffer g_strerror_tls_buffer; char* strerror(int error_number) { // Just return the original constant in the easy cases. char* result = const_cast(__strerror_lookup(error_number)); - if (result != NULL) { + if (result != nullptr) { return result; } - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, strerror, NL_TEXTMAX); - strerror_r(error_number, strerror_tls_buffer, strerror_tls_buffer_size); - return strerror_tls_buffer; + result = g_strerror_tls_buffer.get(); + strerror_r(error_number, result, g_strerror_tls_buffer.size()); + return result; } diff --git a/libc/bionic/strsignal.cpp b/libc/bionic/strsignal.cpp index 9f0193acf..c389ddd3b 100644 --- a/libc/bionic/strsignal.cpp +++ b/libc/bionic/strsignal.cpp @@ -32,7 +32,7 @@ extern "C" const char* __strsignal_lookup(int); extern "C" const char* __strsignal(int, char*, size_t); -GLOBAL_INIT_THREAD_LOCAL_BUFFER(strsignal); +static ThreadLocalBuffer g_strsignal_tls_buffer; char* strsignal(int signal_number) { // Just return the original constant in the easy cases. @@ -41,6 +41,6 @@ char* strsignal(int signal_number) { return result; } - LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, strsignal, NL_TEXTMAX); - return const_cast(__strsignal(signal_number, strsignal_tls_buffer, strsignal_tls_buffer_size)); + return const_cast(__strsignal(signal_number, g_strsignal_tls_buffer.get(), + g_strsignal_tls_buffer.size())); } diff --git a/libc/bionic/stubs.cpp b/libc/bionic/stubs.cpp index f9a31b9af..d4440c434 100644 --- a/libc/bionic/stubs.cpp +++ b/libc/bionic/stubs.cpp @@ -49,25 +49,12 @@ // functions to share state, but functions can't clobber // functions' state and vice versa. -GLOBAL_INIT_THREAD_LOCAL_BUFFER(group); - struct group_state_t { group group_; char* group_members_[2]; char group_name_buffer_[32]; }; -static group_state_t* __group_state() { - LOCAL_INIT_THREAD_LOCAL_BUFFER(group_state_t*, group, sizeof(group_state_t)); - if (group_tls_buffer != NULL) { - memset(group_tls_buffer, 0, sizeof(group_state_t)); - group_tls_buffer->group_.gr_mem = group_tls_buffer->group_members_; - } - return group_tls_buffer; -} - -GLOBAL_INIT_THREAD_LOCAL_BUFFER(passwd); - struct passwd_state_t { passwd passwd_; char name_buffer_[32]; @@ -75,9 +62,16 @@ struct passwd_state_t { char sh_buffer_[32]; }; -static passwd_state_t* __passwd_state() { - LOCAL_INIT_THREAD_LOCAL_BUFFER(passwd_state_t*, passwd, sizeof(passwd_state_t)); - return passwd_tls_buffer; +static ThreadLocalBuffer g_group_tls_buffer; +static ThreadLocalBuffer g_passwd_tls_buffer; + +static group_state_t* __group_state() { + group_state_t* result = g_group_tls_buffer.get(); + if (result != nullptr) { + memset(result, 0, sizeof(group_state_t)); + result->group_.gr_mem = result->group_members_; + } + return result; } static int do_getpw_r(int by_name, const char* name, uid_t uid, @@ -361,7 +355,7 @@ static group* app_id_to_group(gid_t gid, group_state_t* state) { } passwd* getpwuid(uid_t uid) { // NOLINT: implementing bad function. - passwd_state_t* state = __passwd_state(); + passwd_state_t* state = g_passwd_tls_buffer.get(); if (state == NULL) { return NULL; } @@ -374,7 +368,7 @@ passwd* getpwuid(uid_t uid) { // NOLINT: implementing bad function. } passwd* getpwnam(const char* login) { // NOLINT: implementing bad function. - passwd_state_t* state = __passwd_state(); + passwd_state_t* state = g_passwd_tls_buffer.get(); if (state == NULL) { return NULL; } diff --git a/libc/dns/resolv/res_state.c b/libc/dns/resolv/res_state.c index 459f07394..afccd9979 100644 --- a/libc/dns/resolv/res_state.c +++ b/libc/dns/resolv/res_state.c @@ -39,8 +39,6 @@ #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ #include -#include "private/ThreadLocalBuffer.h" - /* Set to 1 to enable debug traces */ #define DEBUG 0 @@ -105,7 +103,11 @@ _res_thread_free( void* _rt ) free(rt); } -BIONIC_PTHREAD_KEY_WITH_CONSTRUCTOR(_res_key, _res_thread_free); +static pthread_key_t _res_key; + +__attribute__((constructor)) static void __res_key_init() { + pthread_key_create(&_res_key, _res_thread_free); +} static _res_thread* _res_thread_get(void) diff --git a/libc/private/ThreadLocalBuffer.h b/libc/private/ThreadLocalBuffer.h index cc4731703..5e436659a 100644 --- a/libc/private/ThreadLocalBuffer.h +++ b/libc/private/ThreadLocalBuffer.h @@ -32,32 +32,30 @@ #include #include -// libstdc++ currently contains __cxa_guard_acquire and __cxa_guard_release, -// so we make do with macros instead of a C++ class. -// TODO: move __cxa_guard_acquire and __cxa_guard_release into libc. - -// We used to use pthread_once to initialize the keys, but life is more predictable -// if we allocate them all up front when the C library starts up, via __constructor__. -#define BIONIC_PTHREAD_KEY_WITH_CONSTRUCTOR(key_name, key_destructor) \ - static pthread_key_t key_name; \ - __attribute__((constructor)) static void __bionic_tls_ ## key_name ## _key_init() { \ - pthread_key_create(&key_name, key_destructor); \ +// TODO: use __thread instead? + +template +class ThreadLocalBuffer { + public: + ThreadLocalBuffer() { + // We used to use pthread_once to initialize the keys, but life is more predictable + // if we allocate them all up front when the C library starts up, via __constructor__. + pthread_key_create(&key_, free); + } + + T* get() { + T* result = reinterpret_cast(pthread_getspecific(key_)); + if (result == nullptr) { + result = reinterpret_cast(calloc(1, Size)); + pthread_setspecific(key_, result); + } + return result; } -#define GLOBAL_INIT_THREAD_LOCAL_BUFFER(name) \ - static void __bionic_tls_ ## name ## _key_destroy(void* buffer) { \ - free(buffer); \ - } \ - BIONIC_PTHREAD_KEY_WITH_CONSTRUCTOR(__bionic_tls_ ## name ## _key, __bionic_tls_ ## name ## _key_destroy) - -// Leaves "name_tls_buffer" and "name_tls_buffer_size" defined and initialized. -#define LOCAL_INIT_THREAD_LOCAL_BUFFER(type, name, byte_count) \ - type name ## _tls_buffer = \ - reinterpret_cast(pthread_getspecific(__bionic_tls_ ## name ## _key)); \ - if (name ## _tls_buffer == NULL) { \ - name ## _tls_buffer = reinterpret_cast(calloc(1, byte_count)); \ - pthread_setspecific(__bionic_tls_ ## name ## _key, name ## _tls_buffer); \ - } \ - const size_t name ## _tls_buffer_size __attribute__((unused)) = byte_count + size_t size() { return Size; } + + private: + pthread_key_t key_; +}; #endif // _BIONIC_THREAD_LOCAL_BUFFER_H_included diff --git a/libc/private/bionic_tls.h b/libc/private/bionic_tls.h index 1ab8d4a26..414d1711d 100644 --- a/libc/private/bionic_tls.h +++ b/libc/private/bionic_tls.h @@ -72,22 +72,26 @@ enum { /* * Bionic uses some pthread keys internally. All pthread keys used internally - * should be created in constructors, except for keys that may be used in or before constructors. + * should be created in constructors, except for keys that may be used in or + * before constructors. + * * We need to manually maintain the count of pthread keys used internally, but * pthread_test should fail if we forget. - * Following are current pthread keys used internally by libc: - * basename libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * dirname libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) + * + * These are the pthread keys currently used internally by libc: + * + * basename libc (ThreadLocalBuffer) + * dirname libc (ThreadLocalBuffer) * uselocale libc (can be used in constructors) - * getmntent_mntent libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * getmntent_strings libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * ptsname libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * ttyname libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * strerror libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * strsignal libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * passwd libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * group libc (GLOBAL_INIT_THREAD_LOCAL_BUFFER) - * _res_key libc (BIONIC_PTHREAD_KEY_WITH_CONSTRUCTOR) + * getmntent_mntent libc (ThreadLocalBuffer) + * getmntent_strings libc (ThreadLocalBuffer) + * ptsname libc (ThreadLocalBuffer) + * ttyname libc (ThreadLocalBuffer) + * strerror libc (ThreadLocalBuffer) + * strsignal libc (ThreadLocalBuffer) + * passwd libc (ThreadLocalBuffer) + * group libc (ThreadLocalBuffer) + * _res_key libc (constructor in BSD code) */ #define LIBC_PTHREAD_KEY_RESERVED_COUNT 12 diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index 13d743f87..16bf9c0cb 100644 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -68,8 +68,7 @@ TEST(pthread, pthread_key_many_distinct) { for (int i = 0; i < nkeys; ++i) { pthread_key_t key; - // If this fails, it's likely that GLOBAL_INIT_THREAD_LOCAL_BUFFER_COUNT is - // wrong. + // If this fails, it's likely that LIBC_PTHREAD_KEY_RESERVED_COUNT is wrong. ASSERT_EQ(0, pthread_key_create(&key, NULL)) << i << " of " << nkeys; keys.push_back(key); ASSERT_EQ(0, pthread_setspecific(key, reinterpret_cast(i))); -- 2.11.0