diff --git a/ChangeLog b/ChangeLog index 9ef00a7a32..9bab5e44bb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2015-10-29 Florian Weimer + + * elf/dl-fini.c (_dl_fini): Rewrite to use variable-length array + instead of extend_alloca. Change control flow to avoid a goto. + Remove assert which is trivially always true. + 2015-10-28 Joseph Myers [BZ #16068] diff --git a/elf/dl-fini.c b/elf/dl-fini.c index 6cfe6519c4..0790b04d86 100644 --- a/elf/dl-fini.c +++ b/elf/dl-fini.c @@ -16,7 +16,6 @@ License along with the GNU C Library; if not, see . */ -#include #include #include #include @@ -140,8 +139,6 @@ _dl_fini (void) using `dlopen' there are possibly several other modules with its dependencies to be taken into account. Therefore we have to start determining the order of the modules once again from the beginning. */ - struct link_map **maps = NULL; - size_t maps_size = 0; /* We run the destructors of the main namespaces last. As for the other namespaces, we pick run the destructors in them in reverse @@ -155,7 +152,6 @@ _dl_fini (void) /* Protect against concurrent loads and unloads. */ __rtld_lock_lock_recursive (GL(dl_load_lock)); - unsigned int nmaps = 0; unsigned int nloaded = GL(dl_ns)[ns]._ns_nloaded; /* No need to do anything for empty namespaces or those used for auditing DSOs. */ @@ -164,118 +160,107 @@ _dl_fini (void) || GL(dl_ns)[ns]._ns_loaded->l_auditing != do_audit #endif ) - goto out; - - /* XXX Could it be (in static binaries) that there is no object - loaded? */ - assert (ns != LM_ID_BASE || nloaded > 0); - - /* Now we can allocate an array to hold all the pointers and copy - the pointers in. */ - if (maps_size < nloaded * sizeof (struct link_map *)) + __rtld_lock_unlock_recursive (GL(dl_load_lock)); + else { - if (maps_size == 0) + /* Now we can allocate an array to hold all the pointers and + copy the pointers in. */ + struct link_map *maps[nloaded]; + + unsigned int i; + struct link_map *l; + assert (nloaded != 0 || GL(dl_ns)[ns]._ns_loaded == NULL); + for (l = GL(dl_ns)[ns]._ns_loaded, i = 0; l != NULL; l = l->l_next) + /* Do not handle ld.so in secondary namespaces. */ + if (l == l->l_real) + { + assert (i < nloaded); + + maps[i] = l; + l->l_idx = i; + ++i; + + /* Bump l_direct_opencount of all objects so that they + are not dlclose()ed from underneath us. */ + ++l->l_direct_opencount; + } + assert (ns != LM_ID_BASE || i == nloaded); + assert (ns == LM_ID_BASE || i == nloaded || i == nloaded - 1); + unsigned int nmaps = i; + + /* Now we have to do the sorting. */ + _dl_sort_fini (maps, nmaps, NULL, ns); + + /* We do not rely on the linked list of loaded object anymore + from this point on. We have our own list here (maps). The + various members of this list cannot vanish since the open + count is too high and will be decremented in this loop. So + we release the lock so that some code which might be called + from a destructor can directly or indirectly access the + lock. */ + __rtld_lock_unlock_recursive (GL(dl_load_lock)); + + /* 'maps' now contains the objects in the right order. Now + call the destructors. We have to process this array from + the front. */ + for (i = 0; i < nmaps; ++i) { - maps_size = nloaded * sizeof (struct link_map *); - maps = (struct link_map **) alloca (maps_size); - } - else - maps = (struct link_map **) - extend_alloca (maps, maps_size, - nloaded * sizeof (struct link_map *)); - } + struct link_map *l = maps[i]; - unsigned int i; - struct link_map *l; - assert (nloaded != 0 || GL(dl_ns)[ns]._ns_loaded == NULL); - for (l = GL(dl_ns)[ns]._ns_loaded, i = 0; l != NULL; l = l->l_next) - /* Do not handle ld.so in secondary namespaces. */ - if (l == l->l_real) - { - assert (i < nloaded); - - maps[i] = l; - l->l_idx = i; - ++i; - - /* Bump l_direct_opencount of all objects so that they are - not dlclose()ed from underneath us. */ - ++l->l_direct_opencount; - } - assert (ns != LM_ID_BASE || i == nloaded); - assert (ns == LM_ID_BASE || i == nloaded || i == nloaded - 1); - nmaps = i; - - /* Now we have to do the sorting. */ - _dl_sort_fini (maps, nmaps, NULL, ns); - - /* We do not rely on the linked list of loaded object anymore from - this point on. We have our own list here (maps). The various - members of this list cannot vanish since the open count is too - high and will be decremented in this loop. So we release the - lock so that some code which might be called from a destructor - can directly or indirectly access the lock. */ - out: - __rtld_lock_unlock_recursive (GL(dl_load_lock)); - - /* 'maps' now contains the objects in the right order. Now call the - destructors. We have to process this array from the front. */ - for (i = 0; i < nmaps; ++i) - { - l = maps[i]; - - if (l->l_init_called) - { - /* Make sure nothing happens if we are called twice. */ - l->l_init_called = 0; - - /* Is there a destructor function? */ - if (l->l_info[DT_FINI_ARRAY] != NULL - || l->l_info[DT_FINI] != NULL) + if (l->l_init_called) { - /* When debugging print a message first. */ - if (__builtin_expect (GLRO(dl_debug_mask) - & DL_DEBUG_IMPCALLS, 0)) - _dl_debug_printf ("\ncalling fini: %s [%lu]\n\n", - DSO_FILENAME (l->l_name), - ns); - - /* First see whether an array is given. */ - if (l->l_info[DT_FINI_ARRAY] != NULL) + /* Make sure nothing happens if we are called twice. */ + l->l_init_called = 0; + + /* Is there a destructor function? */ + if (l->l_info[DT_FINI_ARRAY] != NULL + || l->l_info[DT_FINI] != NULL) { - ElfW(Addr) *array = - (ElfW(Addr) *) (l->l_addr - + l->l_info[DT_FINI_ARRAY]->d_un.d_ptr); - unsigned int i = (l->l_info[DT_FINI_ARRAYSZ]->d_un.d_val - / sizeof (ElfW(Addr))); - while (i-- > 0) - ((fini_t) array[i]) (); + /* When debugging print a message first. */ + if (__builtin_expect (GLRO(dl_debug_mask) + & DL_DEBUG_IMPCALLS, 0)) + _dl_debug_printf ("\ncalling fini: %s [%lu]\n\n", + DSO_FILENAME (l->l_name), + ns); + + /* First see whether an array is given. */ + if (l->l_info[DT_FINI_ARRAY] != NULL) + { + ElfW(Addr) *array = + (ElfW(Addr) *) (l->l_addr + + l->l_info[DT_FINI_ARRAY]->d_un.d_ptr); + unsigned int i = (l->l_info[DT_FINI_ARRAYSZ]->d_un.d_val + / sizeof (ElfW(Addr))); + while (i-- > 0) + ((fini_t) array[i]) (); + } + + /* Next try the old-style destructor. */ + if (l->l_info[DT_FINI] != NULL) + DL_CALL_DT_FINI + (l, l->l_addr + l->l_info[DT_FINI]->d_un.d_ptr); } - /* Next try the old-style destructor. */ - if (l->l_info[DT_FINI] != NULL) - DL_CALL_DT_FINI(l, l->l_addr + l->l_info[DT_FINI]->d_un.d_ptr); - } - #ifdef SHARED - /* Auditing checkpoint: another object closed. */ - if (!do_audit && __builtin_expect (GLRO(dl_naudit) > 0, 0)) - { - struct audit_ifaces *afct = GLRO(dl_audit); - for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt) + /* Auditing checkpoint: another object closed. */ + if (!do_audit && __builtin_expect (GLRO(dl_naudit) > 0, 0)) { - if (afct->objclose != NULL) - /* Return value is ignored. */ - (void) afct->objclose (&l->l_audit[cnt].cookie); - - afct = afct->next; + struct audit_ifaces *afct = GLRO(dl_audit); + for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt) + { + if (afct->objclose != NULL) + /* Return value is ignored. */ + (void) afct->objclose (&l->l_audit[cnt].cookie); + + afct = afct->next; + } } - } #endif - } + } - /* Correct the previous increment. */ - --l->l_direct_opencount; + /* Correct the previous increment. */ + --l->l_direct_opencount; + } } }