From 73d61e4f6c65da714c0f8a3a233725322553ceba Mon Sep 17 00:00:00 2001 From: Allan McRae Date: Wed, 5 Feb 2014 21:14:59 +1000 Subject: [PATCH] Revert "Async-signal safe TLS." This reverts commit 7f507ee17aee720fa423fa38502bc3caa0dd03d7. Conflicts: ChangeLog nptl/tst-tls7.c nptl/tst-tls7mod.c --- ChangeLog | 9 ---- elf/dl-open.c | 5 +- elf/dl-reloc.c | 48 ++--------------- elf/dl-tls.c | 100 ++++++++++++----------------------- nptl/ChangeLog | 9 +--- nptl/Makefile | 12 +---- nptl/allocatestack.c | 13 ++--- nptl/tst-tls7.c | 143 --------------------------------------------------- nptl/tst-tls7mod.c | 40 -------------- 9 files changed, 46 insertions(+), 333 deletions(-) delete mode 100644 nptl/tst-tls7.c delete mode 100644 nptl/tst-tls7mod.c diff --git a/ChangeLog b/ChangeLog index 9aa314e28f..7843e3b92f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -561,15 +561,6 @@ * sysdeps/powerpc/powerpc64/multiarch/strnlen-power7.S: Likewise. * sysdeps/powerpc/powerpc64/multiarch/wcscpy.c: Likewise. -2014-01-03 Andrew Hunter - - * elf/dl-open.c (dl_open_worker): New comment. - * elf/dl-reloc.c (_dl_try_allocate_static_tls): Use - atomic_compare_and_exchange_bool_acq - (_dl_allocate_static_tls): Block signals. - * elf/dl-tls.c (allocate_and_init): Return void. - (_dl_update_slotinfo): Block signals, use atomic update. - 2014-01-03 Joseph Myers * sysdeps/powerpc/nofpu/libm-test-ulps: Regenerated. diff --git a/elf/dl-open.c b/elf/dl-open.c index ea222d03e1..a9ca6b3b44 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -548,10 +548,7 @@ cannot load any more object with static TLS")); generation of the DSO we are allocating data for. */ _dl_update_slotinfo (imap->l_tls_modid); #endif - /* We do this iteration under a signal mask in dl-reloc; why not - here? Because these symbols are new and dlopen hasn't - returned yet. So we can't possibly be racing with a TLS - access to them from another thread. */ + GL(dl_init_static_tls) (imap); assert (imap->l_need_tls_init == 0); } diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c index 81ee47eb62..1f66fccee2 100644 --- a/elf/dl-reloc.c +++ b/elf/dl-reloc.c @@ -16,10 +16,8 @@ License along with the GNU C Library; if not, see . */ -#include #include #include -#include #include #include #include @@ -72,6 +70,8 @@ _dl_try_allocate_static_tls (struct link_map *map) size_t offset = GL(dl_tls_static_used) + (freebytes - n * map->l_tls_align - map->l_tls_firstbyte_offset); + + map->l_tls_offset = GL(dl_tls_static_used) = offset; #elif TLS_DTV_AT_TP /* dl_tls_static_used includes the TCB at the beginning. */ size_t offset = (((GL(dl_tls_static_used) @@ -83,36 +83,7 @@ _dl_try_allocate_static_tls (struct link_map *map) if (used > GL(dl_tls_static_size)) goto fail; -#else -# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" -#endif - /* We've computed the new value we want, now try to install it. */ - ptrdiff_t val; - if ((val = map->l_tls_offset) == NO_TLS_OFFSET) - { - /* l_tls_offset starts out at NO_TLS_OFFSET, and all attempts to - change it go from NO_TLS_OFFSET to some other value. We use - compare_and_exchange to ensure only one attempt succeeds. We - don't actually need any memory ordering here, but _acq is the - weakest available. */ - (void ) atomic_compare_and_exchange_bool_acq (&map->l_tls_offset, - offset, - NO_TLS_OFFSET); - val = map->l_tls_offset; - assert (val != NO_TLS_OFFSET); - } - if (val != offset) - { - /* We'd like to set a static offset for this section, but another - thread has already used a dynamic TLS block for it. Since we can - only use static offsets if everyone does (and it's not practical - to move that thread's dynamic block), we have to fail. */ - goto fail; - } - /* We installed the value; now update the globals. */ -#if TLS_TCB_AT_TP - GL(dl_tls_static_used) = offset; -#elif TLS_DTV_AT_TP + map->l_tls_offset = offset; map->l_tls_firstbyte_offset = GL(dl_tls_static_used); GL(dl_tls_static_used) = used; #else @@ -143,17 +114,8 @@ void internal_function __attribute_noinline__ _dl_allocate_static_tls (struct link_map *map) { - /* We wrap this in a signal mask because it has to iterate all threads - (including this one) and update this map's TLS entry. A signal handler - accessing TLS would try to do the same update and break. */ - sigset_t old; - _dl_mask_all_signals (&old); - int err = -1; - if (map->l_tls_offset != FORCED_DYNAMIC_TLS_OFFSET) - err = _dl_try_allocate_static_tls (map); - - _dl_unmask_signals (&old); - if (err != 0) + if (map->l_tls_offset == FORCED_DYNAMIC_TLS_OFFSET + || _dl_try_allocate_static_tls (map)) { _dl_signal_error (0, map->l_name, NULL, N_("\ cannot allocate memory in static TLS block")); diff --git a/elf/dl-tls.c b/elf/dl-tls.c index 50ec876eae..c1802e7d4e 100644 --- a/elf/dl-tls.c +++ b/elf/dl-tls.c @@ -17,7 +17,6 @@ . */ #include -#include #include #include #include @@ -534,21 +533,19 @@ rtld_hidden_def (_dl_deallocate_tls) # endif -static void -allocate_and_init (dtv_t *dtv, struct link_map *map) +static void * +allocate_and_init (struct link_map *map) { void *newp; newp = __signal_safe_memalign (map->l_tls_align, map->l_tls_blocksize); if (newp == NULL) oom (); - /* Initialize the memory. Since this is our thread's space, we are - under a signal mask, and no one has touched this section before, - we can safely just overwrite whatever's there. */ + /* Initialize the memory. */ memset (__mempcpy (newp, map->l_tls_initimage, map->l_tls_initimage_size), '\0', map->l_tls_blocksize - map->l_tls_initimage_size); - dtv->pointer.val = newp; + return newp; } @@ -590,15 +587,7 @@ _dl_update_slotinfo (unsigned long int req_modid) the entry we need. */ size_t new_gen = listp->slotinfo[idx].gen; size_t total = 0; - sigset_t old; - - _dl_mask_all_signals (&old); - /* We use the signal mask as a lock against reentrancy here. - Check that a signal taken before the lock didn't already - update us. */ - dtv = THREAD_DTV (); - if (dtv[0].counter >= listp->slotinfo[idx].gen) - goto out; + /* We have to look through the entire dtv slotinfo list. */ listp = GL(dl_tls_dtv_slotinfo_list); do @@ -710,8 +699,6 @@ _dl_update_slotinfo (unsigned long int req_modid) /* This will be the new maximum generation counter. */ dtv[0].counter = new_gen; - out: - _dl_unmask_signals (&old); } return the_map; @@ -737,60 +724,39 @@ tls_get_addr_tail (GET_ADDR_ARGS, dtv_t *dtv, struct link_map *the_map) the_map = listp->slotinfo[idx].map; } - sigset_t old; - _dl_mask_all_signals (&old); - - /* As with update_slotinfo, we use the sigmask as a check against - reentrancy. */ - if (dtv[GET_ADDR_MODULE].pointer.val != TLS_DTV_UNALLOCATED) - goto out; - - /* Synchronize against a parallel dlopen() forcing this variable - into static storage. If that happens, we have to be more careful - about initializing the area, as that dlopen() will be iterating - the threads to do so itself. */ - ptrdiff_t offset; - if ((offset = the_map->l_tls_offset) == NO_TLS_OFFSET) - { - /* l_tls_offset starts out at NO_TLS_OFFSET, and all attempts to - change it go from NO_TLS_OFFSET to some other value. We use - compare_and_exchange to ensure only one attempt succeeds. We - don't actually need any memory ordering here, but _acq is the - weakest available. */ - (void) atomic_compare_and_exchange_bool_acq (&the_map->l_tls_offset, - FORCED_DYNAMIC_TLS_OFFSET, - NO_TLS_OFFSET); - offset = the_map->l_tls_offset; - assert (offset != NO_TLS_OFFSET); - } - if (offset == FORCED_DYNAMIC_TLS_OFFSET) - { - allocate_and_init (&dtv[GET_ADDR_MODULE], the_map); - } - else + + again: + /* Make sure that, if a dlopen running in parallel forces the + variable into static storage, we'll wait until the address in the + static TLS block is set up, and use that. If we're undecided + yet, make sure we make the decision holding the lock as well. */ + if (__builtin_expect (the_map->l_tls_offset + != FORCED_DYNAMIC_TLS_OFFSET, 0)) { - void **pp = &dtv[GET_ADDR_MODULE].pointer.val; - while (atomic_forced_read (*pp) == TLS_DTV_UNALLOCATED) + __rtld_lock_lock_recursive (GL(dl_load_lock)); + if (__builtin_expect (the_map->l_tls_offset == NO_TLS_OFFSET, 1)) { - /* for lack of a better (safe) thing to do, just spin. - Someone else (not us; it's done under a signal mask) set - this map to a static TLS offset, and they'll iterate all - threads to initialize it. They'll eventually write - to pointer.val, at which point we know they've fully - completed initialization. */ - atomic_delay (); + the_map->l_tls_offset = FORCED_DYNAMIC_TLS_OFFSET; + __rtld_lock_unlock_recursive (GL(dl_load_lock)); + } + else + { + __rtld_lock_unlock_recursive (GL(dl_load_lock)); + if (__builtin_expect (the_map->l_tls_offset + != FORCED_DYNAMIC_TLS_OFFSET, 1)) + { + void *p = dtv[GET_ADDR_MODULE].pointer.val; + if (__builtin_expect (p == TLS_DTV_UNALLOCATED, 0)) + goto again; + + return (char *) p + GET_ADDR_OFFSET; + } } - /* Make sure we've picked up their initialization of the actual - block; this pairs against the write barrier in - init_one_static_tls, guaranteeing that we see their write of - the tls_initimage into the static region. */ - atomic_read_barrier (); } -out: - assert (dtv[GET_ADDR_MODULE].pointer.val != TLS_DTV_UNALLOCATED); - _dl_unmask_signals (&old); + void *p = dtv[GET_ADDR_MODULE].pointer.val = allocate_and_init (the_map); + dtv[GET_ADDR_MODULE].pointer.is_static = false; - return (char *) dtv[GET_ADDR_MODULE].pointer.val + GET_ADDR_OFFSET; + return (char *) p + GET_ADDR_OFFSET; } diff --git a/nptl/ChangeLog b/nptl/ChangeLog index 003e290bb1..a4d3f45a47 100644 --- a/nptl/ChangeLog +++ b/nptl/ChangeLog @@ -19,14 +19,7 @@ (do_test): Call it. * tst-tls7mod.c (action): Move sem_post to caller. -2014-01-03 Andrew Hunter - - * nptl/Makefile (tst-tls7): New test. - * nptl/tst-tls7.c: New file. - * nptl/tst-tls7mod.c: New file. - * nptl/allocatestack.c (init_one_static_tls): Use atomic barrier. - -2013-12-12 Adhemerval Zanella +2011-12-12 Adhemerval Zanella * sysdeps/powerpc/tls.h (struct tcbhead_t): Add DSO and TAR fields. * nptl/sysdeps/powerpc/tcb-offsets.sym: Likewise. diff --git a/nptl/Makefile b/nptl/Makefile index a43e740847..57cc8c69e4 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -293,7 +293,7 @@ tests += tst-cancelx2 tst-cancelx3 tst-cancelx4 tst-cancelx5 \ tst-oncex3 tst-oncex4 endif ifeq ($(build-shared),yes) -tests += tst-atfork2 tst-tls3 tst-tls4 tst-tls5 tst-tls7 tst-_res1 tst-fini1 \ +tests += tst-atfork2 tst-tls3 tst-tls4 tst-tls5 tst-_res1 tst-fini1 \ tst-stackguard1 tests-nolibpthread += tst-fini1 ifeq ($(have-z-execstack),yes) @@ -304,8 +304,7 @@ endif modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \ tst-tls5mod tst-tls5moda tst-tls5modb tst-tls5modc \ tst-tls5modd tst-tls5mode tst-tls5modf \ - tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod \ - tst-tls7mod + tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) tst-cleanup4aux.o test-extras += $(modules-names) tst-cleanup4aux test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names))) @@ -319,7 +318,6 @@ tst-tls5modc.so-no-z-defs = yes tst-tls5modd.so-no-z-defs = yes tst-tls5mode.so-no-z-defs = yes tst-tls5modf.so-no-z-defs = yes -tst-tls7mod.so-no-z-defs = yes ifeq ($(build-shared),yes) # Build all the modules even when not actually running test programs. @@ -482,12 +480,6 @@ $(objpfx)tst-tls5: $(objpfx)tst-tls5mod.so $(shared-thread-library) LDFLAGS-tst-tls5 = $(no-as-needed) LDFLAGS-tst-tls5mod.so = -Wl,-soname,tst-tls5mod.so -# ensure free(malloc()) isn't optimized out -CFLAGS-tst-tls7.c = -fno-builtin-malloc -fno-builtin-free -$(objpfx)tst-tls7: $(libdl) $(shared-thread-library) -$(objpfx)tst-tls7.out: $(objpfx)tst-tls7mod.so -$(objpfx)tst-tls7mod.so: $(shared-thread-library) - ifeq ($(build-shared),yes) ifeq ($(run-built-tests),yes) tests: $(objpfx)tst-tls6.out diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 2a5ac22117..7f6094ebb2 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -1173,18 +1173,13 @@ init_one_static_tls (struct pthread *curp, struct link_map *map) # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" # endif - /* Initialize the memory. */ - memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size), - '\0', map->l_tls_blocksize - map->l_tls_initimage_size); - /* Fill in the DTV slot so that a later LD/GD access will find it. */ - dtv[map->l_tls_modid].pointer.is_static = true; - /* Pairs against the read barrier in tls_get_attr_tail, guaranteeing - any thread waiting for an update to pointer.val sees the - initimage write. */ - atomic_write_barrier (); dtv[map->l_tls_modid].pointer.val = dest; + dtv[map->l_tls_modid].pointer.is_static = true; + /* Initialize the memory. */ + memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size), + '\0', map->l_tls_blocksize - map->l_tls_initimage_size); } void diff --git a/nptl/tst-tls7.c b/nptl/tst-tls7.c deleted file mode 100644 index 3e85a6ec85..0000000000 --- a/nptl/tst-tls7.c +++ /dev/null @@ -1,143 +0,0 @@ -/* Copyright (C) 2013 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - 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 - . */ - - -/* This test checks that TLS in a dlopened object works when first accessed - from a signal handler. */ - -#include -#include -#include -#include -#include -#include -#include -#include - -void * -spin (void *ignored) -{ - while (1) - { - /* busywork */ - free (malloc (128)); - } - - /* never reached */ - return NULL; -} - -static void (*tls7mod_action) (int, siginfo_t *, void *); - -static void -action (int signo, siginfo_t *info, void *ignored) -{ - sem_t *sem = info->si_value.sival_ptr; - - atomic_read_barrier (); - assert (tls7mod_action != NULL); - (*tls7mod_action) (signo, info, ignored); - - /* This sem_post may trigger dlclose, which will invalidate tls7mod_action. - It is important to do that only after tls7mod_action is no longer - active. */ - sem_post (sem); -} - -int -do_test (void) -{ - pthread_t th[10]; - - for (int i = 0; i < 10; ++i) - { - if (pthread_create (&th[i], NULL, spin, NULL)) - { - puts ("pthread_create failed"); - exit (1); - } - } -#define NITERS 75 - - for (int i = 0; i < NITERS; ++i) - { - void *h = dlopen ("tst-tls7mod.so", RTLD_LAZY); - if (h == NULL) - { - puts ("dlopen failed"); - exit (1); - } - - tls7mod_action = dlsym (h, "action"); - if (tls7mod_action == NULL) - { - puts ("dlsym for action failed"); - exit (1); - } - atomic_write_barrier (); - - struct sigaction sa; - sa.sa_sigaction = action; - sigemptyset (&sa.sa_mask); - sa.sa_flags = SA_SIGINFO; - if (sigaction (SIGUSR1, &sa, NULL)) - { - puts ("sigaction failed"); - exit (1); - } - - sem_t sem; - if (sem_init (&sem, 0, 0)) - { - puts ("sem_init failed"); - } - - sigval_t val; - val.sival_ptr = &sem; - for (int i = 0; i < 10; ++i) - { - if (pthread_sigqueue (th[i], SIGUSR1, val)) - { - puts ("pthread_sigqueue failed"); - } - } - - - for (int i = 0; i < 10; ++i) - { - if (sem_wait (&sem)) - { - puts ("sem_wait failed"); - } - } - - /* Paranoia. */ - tls7mod_action = NULL; - - if (dlclose (h)) - { - puts ("dlclose failed"); - exit (1); - } - } - return 0; -} - -#define TIMEOUT 8 - -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" diff --git a/nptl/tst-tls7mod.c b/nptl/tst-tls7mod.c deleted file mode 100644 index da5af56370..0000000000 --- a/nptl/tst-tls7mod.c +++ /dev/null @@ -1,40 +0,0 @@ -/* Copyright (C) 2013 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - 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 - . */ - -/* Dynamic module with TLS to be accessed by a signal handler to check safety - of that mode. */ - -#include -#include -#include - -/* This is an unlikely value to see in incorrectly initialized TLS - block -- make sure we're initialized properly. */ -static __thread intptr_t tls_data = 0xdeadbeef; - -void -action (int signo, siginfo_t *info, void *ignored) -{ - if (tls_data != 0xdeadbeef) - { - write (STDOUT_FILENO, "wrong TLS value\n", 17); - _exit (1); - } - - /* arbitrary choice, just write something unique-ish. */ - tls_data = (intptr_t) info; -} -- 2.11.4.GIT