* enabling caching for dl_iterate_phdr() @ 2003-12-16 19:55 David Mosberger 2003-12-23 22:24 ` Roland McGrath 0 siblings, 1 reply; 14+ messages in thread From: David Mosberger @ 2003-12-16 19:55 UTC (permalink / raw) To: drepper; +Cc: libc-hacker Would it be possible to add two atomic counters, dl_load_additions and dl_load_removals, which would get incremented whenever something is a phdr is added to/removed from the list that is traversed by dl_iterate_phdr()? The motivation for this is that this would let me solve a long-standing problem with libunwind where I'd like to cache unwind-info, but cannot do so safely because there is no way for libunwind to detect efficiently when a shared object may have gotten unloaded. Having both the "additions" and "removals" counters would make it possible to cache both negative and positive information, though I'd be quite happy if there was at least a "removals" counter. Any opinions? I guess the alternative that was mentioned in the past was to use callbacks, but that seems very dangerous in regards to deadlock etc. --david ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2003-12-16 19:55 enabling caching for dl_iterate_phdr() David Mosberger @ 2003-12-23 22:24 ` Roland McGrath 0 siblings, 0 replies; 14+ messages in thread From: Roland McGrath @ 2003-12-23 22:24 UTC (permalink / raw) To: davidm; +Cc: drepper, libc-hacker > Would it be possible to add two atomic counters, dl_load_additions and > dl_load_removals, which would get incremented whenever something is a > phdr is added to/removed from the list that is traversed by > dl_iterate_phdr()? What interface do you propose for exposing such counters? We won't be adding any published interfaces using variables. > Any opinions? I guess the alternative that was mentioned in the past > was to use callbacks, but that seems very dangerous in regards to > deadlock etc. If your callback functions do nothing but increment atomic counters of your own, then there is no problem. ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <16387.9755.753294.37588@napali.hpl.hp.com>]
* Re: enabling caching for dl_iterate_phdr() [not found] <16387.9755.753294.37588@napali.hpl.hp.com> @ 2004-01-17 0:57 ` Roland McGrath 2004-01-17 1:39 ` Jakub Jelinek 0 siblings, 1 reply; 14+ messages in thread From: Roland McGrath @ 2004-01-17 0:57 UTC (permalink / raw) To: davidm; +Cc: GNU libc hackers > >> Would it be possible to add two atomic counters, > >> dl_load_additions and dl_load_removals, which would get > >> incremented whenever something is a phdr is added to/removed from > >> the list that is traversed by dl_iterate_phdr()? > > Roland> What interface do you propose for exposing such counters? > Roland> We won't be adding any published interfaces using variables. > > I think that's OK. Do you think it would be OK to add a function > which returns the address of one of several "atomic counter" variables? That is a reasonable kind of interface in general. The trouble here is that glibc does not have any exported interfaces using atomic counters. The right type to use for these is machine-dependent; `int' is not the proper thing to be using in the interface. The types used in atomic.h are fine, but that is not an exported interface. We don't want to export it in that form. Unless you want to wait for that, this seems like a blocker for that style of interface. > enum { > AC_DL_LOAD_ADDITIONS, > AC_DL_LOAD_REMOVALS > } libc_atomic_counter_t; For an exported interface using a parameter like this does not seem like the best plan to me. If new flavors are added, this is not well-expressed for ABI compatibility. To me, the simplest way to do compatibility issues right is to use single-purpose entrypoints with separate names so symbol versioning will just take care of it. So, add a single entrypoint specifically for these two counters now. If you change it in the future, you can add entrypoints or change the old entrypoint signature under the same name using symbol versioning. > In any case, if a callback-based approach is preferred, I can live > with that, too. I don't claim it's the best interface. But that is the approach that doesn't have other blocking issues right now. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-17 0:57 ` Roland McGrath @ 2004-01-17 1:39 ` Jakub Jelinek 2004-01-17 1:40 ` Roland McGrath ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Jakub Jelinek @ 2004-01-17 1:39 UTC (permalink / raw) To: Roland McGrath; +Cc: davidm, GNU libc hackers On Fri, Jan 16, 2004 at 04:57:26PM -0800, Roland McGrath wrote: > > In any case, if a callback-based approach is preferred, I can live > > with that, too. > > I don't claim it's the best interface. But that is the approach that > doesn't have other blocking issues right now. How about extending struct dl_phdr_info and passing an counter in it to dl_iterate_phdr's callback? The structure can be extended at the end, because dl_iterate_phdr passes its size to the callback as well. The callback is called with dl_load_lock held, so the counter doesn't even have to be atomic. libgcc_s would just see if size includes the additional field in the first callback, if yes, it would record that counter and on subsequent dl_iterate_phdr call it would first check whether the counter did not change and if it is the same, it could use cached info. Jakub ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-17 1:39 ` Jakub Jelinek @ 2004-01-17 1:40 ` Roland McGrath 2004-01-17 1:51 ` David Mosberger 2004-01-22 0:35 ` David Mosberger 2 siblings, 0 replies; 14+ messages in thread From: Roland McGrath @ 2004-01-17 1:40 UTC (permalink / raw) To: Jakub Jelinek; +Cc: davidm, GNU libc hackers > How about extending struct dl_phdr_info and passing an counter in it > to dl_iterate_phdr's callback? That is easy enough. OTOH, it's arguably not much harder for the callback to just compare the results with saved results from the last run and decide that way no new work has to be done. That works with existing glibc's too. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-17 1:39 ` Jakub Jelinek 2004-01-17 1:40 ` Roland McGrath @ 2004-01-17 1:51 ` David Mosberger 2004-01-22 0:35 ` David Mosberger 2 siblings, 0 replies; 14+ messages in thread From: David Mosberger @ 2004-01-17 1:51 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Roland McGrath, davidm, GNU libc hackers >>>>> On Sat, 17 Jan 2004 00:29:31 +0100, Jakub Jelinek <jakub@redhat.com> said: Jakub> On Fri, Jan 16, 2004 at 04:57:26PM -0800, Roland McGrath wrote: >> > In any case, if a callback-based approach is preferred, I can live >> > with that, too. >> I don't claim it's the best interface. But that is the approach that >> doesn't have other blocking issues right now. Jakub> How about extending struct dl_phdr_info and passing an counter in it Jakub> to dl_iterate_phdr's callback? Jakub> The structure can be extended at the end, because dl_iterate_phdr Jakub> passes its size to the callback as well. Jakub> The callback is called with dl_load_lock held, so the counter doesn't Jakub> even have to be atomic. Jakub> libgcc_s would just see if size includes the additional field in the Jakub> first callback, if yes, it would record that counter and on subsequent Jakub> dl_iterate_phdr call it would first check whether the counter did not Jakub> change and if it is the same, it could use cached info. Sounds good to me. I assume it'd be OK to have separate "additions" and "removals" counters? Only the latter is needed for avoiding stale values in a cache, but the former helps performance, because dl_iterate_phdr() can be stopped early if we know that the list didn't change. I'd be happy to prototype the necessary code, but at the moment, glibc doesn't build for me on ia64 (it crashes in rpcgen). I assume that's due to the stuff Uli is working on. --david ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-17 1:39 ` Jakub Jelinek 2004-01-17 1:40 ` Roland McGrath 2004-01-17 1:51 ` David Mosberger @ 2004-01-22 0:35 ` David Mosberger 2004-01-22 8:39 ` Jakub Jelinek 2 siblings, 1 reply; 14+ messages in thread From: David Mosberger @ 2004-01-22 0:35 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Roland McGrath, davidm, GNU libc hackers >>>>> On Sat, 17 Jan 2004 00:29:31 +0100, Jakub Jelinek <jakub@redhat.com> said: Jakub> How about extending struct dl_phdr_info and passing an Jakub> counter in it to dl_iterate_phdr's callback? How about the attached patch? It seems to work for me at least for this simple test program: $ cat ~/tmp/t.c #include <link.h> #include <stdio.h> static int callback (struct dl_phdr_info *info, size_t size, void *ptr) { printf (" size = %Zu ", size); printf ("dlpi_adds = %u ", info->dlpi_adds); printf ("dlpi_subs = %u\n", info->dlpi_subs); return -1; } int main (int argc, char **argv) { void *handle; int ret; dl_iterate_phdr (callback, NULL); handle = dlopen ("libunwind.so", RTLD_LAZY); printf ("dlopen(libunwind.so)=%p\n", handle); dl_iterate_phdr (callback, NULL); ret = dlclose (handle); printf ("dlclose(libunwind.so)=%d\n", ret); dl_iterate_phdr (callback, NULL); return 0; } $ gcc -D_GNU_SOURCE -O -Wall t.c -I/tmp/davidm/usr/include/ -ldl $ ./a.out size = 40 dlpi_adds = 4 dlpi_subs = 0 dlopen(libunwind.so)=0x20000008002e0030 size = 40 dlpi_adds = 5 dlpi_subs = 0 dlclose(libunwind.so)=0 size = 40 dlpi_adds = 5 dlpi_subs = 1 In terms of performance, this is clearly more expensive than just checking a variable. For libunwind, the approach adds about 5% to the time required to initialize a stack unwind, but just hitting once in the cache during the actual unwind will easily make up for the extra cost, so I think it's OK. Comments? --david Index: elf/dl-close.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-close.c,v retrieving revision 1.98 diff -u -r1.98 dl-close.c --- elf/dl-close.c 27 Apr 2003 06:19:09 -0000 1.98 +++ elf/dl-close.c 22 Jan 2004 00:26:54 -0000 @@ -319,6 +319,7 @@ /* Notify the debugger we are about to remove some loaded objects. */ _r_debug.r_state = RT_DELETE; _dl_debug_state (); + ++GL(dl_load_subs); #ifdef USE_TLS size_t tls_free_start; Index: elf/dl-iteratephdr.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-iteratephdr.c,v retrieving revision 1.9 diff -u -r1.9 dl-iteratephdr.c --- elf/dl-iteratephdr.c 22 Oct 2003 07:09:41 -0000 1.9 +++ elf/dl-iteratephdr.c 22 Jan 2004 00:26:54 -0000 @@ -48,6 +48,8 @@ info.dlpi_name = l->l_name; info.dlpi_phdr = l->l_phdr; info.dlpi_phnum = l->l_phnum; + info.dlpi_adds = GL(dl_load_adds); + info.dlpi_subs = GL(dl_load_subs); ret = callback (&info, sizeof (struct dl_phdr_info), data); if (ret) break; @@ -84,6 +86,8 @@ info.dlpi_name = ""; info.dlpi_phdr = _dl_phdr; info.dlpi_phnum = _dl_phnum; + info.dlpi_adds = GL(dl_load_adds); + info.dlpi_subs = GL(dl_load_subs); ret = (*callback) (&info, sizeof (struct dl_phdr_info), data); if (ret) return ret; Index: elf/dl-object.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-object.c,v retrieving revision 1.36 diff -u -r1.36 dl-object.c --- elf/dl-object.c 25 Apr 2003 09:06:56 -0000 1.36 +++ elf/dl-object.c 22 Jan 2004 00:26:54 -0000 @@ -83,6 +83,7 @@ else GL(dl_loaded) = new; ++GL(dl_nloaded); + ++GL(dl_load_adds); /* If we have no loader the new object acts as it. */ if (loader == NULL) Index: elf/dl-support.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-support.c,v retrieving revision 1.79 diff -u -r1.79 dl-support.c --- elf/dl-support.c 13 Jan 2004 15:41:27 -0000 1.79 +++ elf/dl-support.c 22 Jan 2004 00:26:54 -0000 @@ -71,6 +71,11 @@ /* Number of object in the _dl_loaded list. */ unsigned int _dl_nloaded; +/* Incremented whenever something may have been added to dl_loaded. */ +unsigned int _dl_load_adds; +/* Incremented whenever something may have been removed from dl_loaded. */ +unsigned int _dl_load_subs; + /* Fake scope. In dynamically linked binaries this is the scope of the main application but here we don't have something like this. So create a fake scope containing nothing. */ Index: elf/link.h =================================================================== RCS file: /cvs/glibc/libc/elf/link.h,v retrieving revision 1.76 diff -u -r1.76 link.h --- elf/link.h 16 Sep 2003 05:48:04 -0000 1.76 +++ elf/link.h 22 Jan 2004 00:26:54 -0000 @@ -100,6 +100,12 @@ const char *dlpi_name; const ElfW(Phdr) *dlpi_phdr; ElfW(Half) dlpi_phnum; + + /* Note: older versions of libc do not provide the following + members. Check the SIZE argument pass to the dl_iterate_phdr() + callback to determine whether or not they areprovided. */ + unsigned int dlpi_adds; /* incremented when phdrs may have been added */ + unsigned int dlpi_subs; /* incremented when phdrs may have been removed */ }; __BEGIN_DECLS Index: include/link.h =================================================================== RCS file: /cvs/glibc/libc/include/link.h,v retrieving revision 1.29 diff -u -r1.29 link.h --- include/link.h 13 Jan 2004 08:27:52 -0000 1.29 +++ include/link.h 22 Jan 2004 00:26:55 -0000 @@ -293,6 +293,12 @@ const char *dlpi_name; const ElfW(Phdr) *dlpi_phdr; ElfW(Half) dlpi_phnum; + + /* Note: older versions of libc do not provide the following + members. Check the SIZE argument pass to the dl_iterate_phdr() + callback to determine whether or not they areprovided. */ + unsigned int dlpi_adds; /* incremented when phdrs may have been added */ + unsigned int dlpi_subs; /* incremented when phdrs may have been removed */ }; extern int dl_iterate_phdr (int (*callback) (struct dl_phdr_info *info, Index: sysdeps/generic/ldsodefs.h =================================================================== RCS file: /cvs/glibc/libc/sysdeps/generic/ldsodefs.h,v retrieving revision 1.87 diff -u -r1.87 ldsodefs.h --- sysdeps/generic/ldsodefs.h 15 Jan 2004 06:37:40 -0000 1.87 +++ sysdeps/generic/ldsodefs.h 22 Jan 2004 00:26:55 -0000 @@ -265,6 +265,11 @@ EXTERN const char *_dl_platform; EXTERN size_t _dl_platformlen; + /* Incremented whenever something may have been added to dl_loaded. */ + EXTERN unsigned int _dl_load_adds; + /* Incremented whenever something may have been removed from dl_loaded. */ + EXTERN unsigned int _dl_load_subs; + #ifndef MAP_ANON /* File descriptor referring to the zero-fill device. */ EXTERN int _dl_zerofd; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-22 0:35 ` David Mosberger @ 2004-01-22 8:39 ` Jakub Jelinek 2004-01-24 5:54 ` David Mosberger 0 siblings, 1 reply; 14+ messages in thread From: Jakub Jelinek @ 2004-01-22 8:39 UTC (permalink / raw) To: davidm; +Cc: Roland McGrath, GNU libc hackers On Wed, Jan 21, 2004 at 04:35:49PM -0800, David Mosberger wrote: > --- elf/link.h 16 Sep 2003 05:48:04 -0000 1.76 > +++ elf/link.h 22 Jan 2004 00:26:54 -0000 > @@ -100,6 +100,12 @@ > const char *dlpi_name; > const ElfW(Phdr) *dlpi_phdr; > ElfW(Half) dlpi_phnum; > + > + /* Note: older versions of libc do not provide the following > + members. Check the SIZE argument pass to the dl_iterate_phdr() > + callback to determine whether or not they areprovided. */ > + unsigned int dlpi_adds; /* incremented when phdrs may have been added */ > + unsigned int dlpi_subs; /* incremented when phdrs may have been removed */ I'd make the counters unsigned long long instead to avoid wrap around. Also, could you write a testcase for it? dl_iterate_phdr, then dlopen some shlib in elf, then dl_iterate_phdr again, check the counters, dlclose etc. Jakub ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-22 8:39 ` Jakub Jelinek @ 2004-01-24 5:54 ` David Mosberger 2004-01-24 20:27 ` Ulrich Drepper 0 siblings, 1 reply; 14+ messages in thread From: David Mosberger @ 2004-01-24 5:54 UTC (permalink / raw) To: Jakub Jelinek; +Cc: davidm, Roland McGrath, GNU libc hackers >>>>> On Thu, 22 Jan 2004 07:29:45 +0100, Jakub Jelinek <jakub@redhat.com> said: Jakub> I'd make the counters unsigned long long instead to avoid Jakub> wrap around. Fine by me. Jakub> Also, could you write a testcase for it? dl_iterate_phdr, Jakub> then dlopen some shlib in elf, then dl_iterate_phdr again, Jakub> check the counters, dlclose etc. Sure. Attached is an updated patch. "make check subdirs=elf" passes all tests (including the new tst-dlmodcount test). If it looks OK, please apply. --david ChangeLog 2004-01-23 David Mosberger <davidm@hpl.hp.com> * sysdeps/generic/ldsodefs.h (struct rtld_global): Add members _dl_load_adds and _dl_load_subs. * elf/dl-support.c (_dl_load_adds): New variable. (_dl_load_subs): Likewise. * elf/dl-object.c (_dl_new_object): Increment dl_load_adds. * elf/dl-close.c (_dl_close): Increment dl_load_subs. * elf/link.h (struct dl_phdr_info): Add members dlpi_adds and dlpi_subs. * include/link.h: Likewise. * elf/dl-iteratephdr.c (__dl_iterate_phdr): Initialize dlpi_adds and dlpi_subs members. (dl_iterate_phdr): Likewise. * elf/tst-dlmodcount.c: New file. * elf/Makefile (distribute): Mention tst-dlmodcount.c. (tests): If build-shared, mention tst-dlmodcount. ($(objpfx)tst-dlmodcount): If build-shared, build and run tst-dlmodcount. Index: elf/Makefile --- elf/Makefile +++ elf/Makefile @@ -80,7 +80,7 @@ reldep9.c reldep9mod1.c reldep9mod2.c reldep9mod3.c \ tst-array1.exp tst-array2.exp tst-array4.exp \ tst-array2dep.c \ - tst-execstack-mod.c \ + tst-execstack-mod.c tst-dlmodcount.c \ check-textrel.c dl-sysdep.h CFLAGS-dl-runtime.c = -fexceptions -fasynchronous-unwind-tables @@ -151,7 +151,7 @@ restest2 next dblload dblunload reldep5 reldep6 reldep7 reldep8 \ circleload1 tst-tls3 tst-tls4 tst-tls5 tst-tls6 tst-tls7 tst-tls8 \ tst-tls10 tst-tls11 tst-tls12 tst-tls13 tst-tls14 tst-align \ - $(tests-execstack-$(have-z-execstack)) + $(tests-execstack-$(have-z-execstack)) tst-dlmodcount # reldep9 test-srcs = tst-pathopt tests-vis-yes = vismain @@ -713,4 +713,8 @@ $(sort $(wildcard $(common-objpfx)*/lib*.so \ $(common-objpfx)iconvdata/*.so)) > $@ generated += check-textrel check-textrel.out + +$(objpfx)tst-dlmodcount: $(libdl) +$(objpfx)tst-dlmodcount.out: $(test-modules) + endif Index: elf/dl-close.c --- elf/dl-close.c +++ elf/dl-close.c @@ -319,6 +319,7 @@ /* Notify the debugger we are about to remove some loaded objects. */ _r_debug.r_state = RT_DELETE; _dl_debug_state (); + ++GL(dl_load_subs); #ifdef USE_TLS size_t tls_free_start; Index: elf/dl-iteratephdr.c --- elf/dl-iteratephdr.c +++ elf/dl-iteratephdr.c @@ -48,6 +48,8 @@ info.dlpi_name = l->l_name; info.dlpi_phdr = l->l_phdr; info.dlpi_phnum = l->l_phnum; + info.dlpi_adds = GL(dl_load_adds); + info.dlpi_subs = GL(dl_load_subs); ret = callback (&info, sizeof (struct dl_phdr_info), data); if (ret) break; @@ -84,6 +86,8 @@ info.dlpi_name = ""; info.dlpi_phdr = _dl_phdr; info.dlpi_phnum = _dl_phnum; + info.dlpi_adds = GL(dl_load_adds); + info.dlpi_subs = GL(dl_load_subs); ret = (*callback) (&info, sizeof (struct dl_phdr_info), data); if (ret) return ret; Index: elf/dl-object.c --- elf/dl-object.c +++ elf/dl-object.c @@ -83,6 +83,7 @@ else GL(dl_loaded) = new; ++GL(dl_nloaded); + ++GL(dl_load_adds); /* If we have no loader the new object acts as it. */ if (loader == NULL) Index: elf/dl-support.c --- elf/dl-support.c +++ elf/dl-support.c @@ -71,6 +71,11 @@ /* Number of object in the _dl_loaded list. */ unsigned int _dl_nloaded; +/* Incremented whenever something may have been added to dl_loaded. */ +unsigned long long _dl_load_adds; +/* Incremented whenever something may have been removed from dl_loaded. */ +unsigned long long _dl_load_subs; + /* Fake scope. In dynamically linked binaries this is the scope of the main application but here we don't have something like this. So create a fake scope containing nothing. */ Index: elf/link.h --- elf/link.h +++ elf/link.h @@ -100,6 +100,12 @@ const char *dlpi_name; const ElfW(Phdr) *dlpi_phdr; ElfW(Half) dlpi_phnum; + + /* Note: older versions of libc do not provide the following + members. Check the SIZE argument pass to the dl_iterate_phdr() + callback to determine whether or not they areprovided. */ + unsigned long long dlpi_adds; /* incr. when phdrs may have been added */ + unsigned long long dlpi_subs; /* incr. when phdrs may have been removed */ }; __BEGIN_DECLS Index: elf/tst-dlmodcount.c --- /dev/null +++ elf/tst-dlmodcount.c @@ -0,0 +1,107 @@ +/* Copyright (C) 2004 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by David Mosberger <davidm@hpl.hp.com>, 2004. + + 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, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + 02111-1307 USA. */ + +#include <link.h> +#include <stdio.h> + +#define SET 0 +#define ADD 1 +#define REMOVE 2 + +#define leq(l,r) (((r) - (l)) <= ~0ULL/2) + +static int +callback (struct dl_phdr_info *info, size_t size, void *ptr) +{ + static int last_adds = 0, last_subs = 0; + intptr_t cmd = (intptr_t) ptr; + + printf (" size = %Zu\n", size); + if (size < (offsetof (struct dl_phdr_info, dlpi_subs) + + sizeof (info->dlpi_subs))) + { + fprintf (stderr, "dl_iterate_phdr failed to pass dlpi_adds/dlpi_subs\n"); + exit (5); + } + + printf (" dlpi_adds = %Lu dlpi_subs = %Lu\n", + info->dlpi_adds, info->dlpi_subs); + + switch (cmd) + { + case SET: + break; + + case ADD: + if (leq (info->dlpi_adds, last_adds)) + { + fprintf (stderr, "dlpi_adds failed to get incremented!\n"); + exit (3); + } + break; + + case REMOVE: + if (leq (info->dlpi_subs, last_subs)) + { + fprintf (stderr, "dlpi_subs failed to get incremented!\n"); + exit (4); + } + break; + } + last_adds = info->dlpi_adds; + last_subs = info->dlpi_subs; + return -1; +} + +static void * +load (const char *path) +{ + void *handle; + + printf ("loading `%s'\n", path); + handle = dlopen (path, RTLD_LAZY); + if (!handle) + exit (1); + dl_iterate_phdr (callback, (void *)(intptr_t) ADD); + return handle; +} + +static void +unload (const char *path, void *handle) +{ + int ret; + + printf ("unloading `%s'\n", path); + if (dlclose (handle) < 0) + exit (2); + dl_iterate_phdr (callback, (void *)(intptr_t) REMOVE); +} + +int +main (int argc, char **argv) +{ + void *handle1, *handle2; + + dl_iterate_phdr (callback, (void *)(intptr_t) SET); + handle1 = load ("firstobj.so"); + handle2 = load ("globalmod1.so"); + unload ("firstobj.so", handle1); + unload ("globalmod1.so", handle2); + return 0; +} Index: include/link.h --- include/link.h +++ include/link.h @@ -293,6 +293,12 @@ const char *dlpi_name; const ElfW(Phdr) *dlpi_phdr; ElfW(Half) dlpi_phnum; + + /* Note: older versions of libc do not provide the following + members. Check the SIZE argument pass to the dl_iterate_phdr() + callback to determine whether or not they areprovided. */ + unsigned long long dlpi_adds; /* incr. when phdrs may have been added */ + unsigned long long dlpi_subs; /* incr. when phdrs may have been removed */ }; extern int dl_iterate_phdr (int (*callback) (struct dl_phdr_info *info, Index: sysdeps/generic/ldsodefs.h --- sysdeps/generic/ldsodefs.h +++ sysdeps/generic/ldsodefs.h @@ -265,6 +265,11 @@ EXTERN const char *_dl_platform; EXTERN size_t _dl_platformlen; + /* Incremented whenever something may have been added to dl_loaded. */ + EXTERN unsigned long long _dl_load_adds; + /* Incremented whenever something may have been removed from dl_loaded. */ + EXTERN unsigned long long _dl_load_subs; + #ifndef MAP_ANON /* File descriptor referring to the zero-fill device. */ EXTERN int _dl_zerofd; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-24 5:54 ` David Mosberger @ 2004-01-24 20:27 ` Ulrich Drepper 2004-01-26 21:48 ` David Mosberger 0 siblings, 1 reply; 14+ messages in thread From: Ulrich Drepper @ 2004-01-24 20:27 UTC (permalink / raw) To: davidm; +Cc: GNU libc hackers David Mosberger wrote: > Attached is an updated patch. "make check subdirs=elf" passes all > tests (including the new tst-dlmodcount test). I've applied the patch. -- ⧠Ulrich Drepper ⧠Red Hat, Inc. ⧠444 Castro St ⧠Mountain View, CA â ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-24 20:27 ` Ulrich Drepper @ 2004-01-26 21:48 ` David Mosberger 2004-01-26 22:39 ` Ulrich Drepper 0 siblings, 1 reply; 14+ messages in thread From: David Mosberger @ 2004-01-26 21:48 UTC (permalink / raw) To: Ulrich Drepper; +Cc: davidm, GNU libc hackers >>>>> On Sat, 24 Jan 2004 12:11:14 -0800, Ulrich Drepper <drepper@redhat.com> said: Uli> David Mosberger wrote: >> Attached is an updated patch. "make check subdirs=elf" passes >> all tests (including the new tst-dlmodcount test). Uli> I've applied the patch. Thanks! In the meantime, it occurred to me that we can get rid of one of the "unsigned long long" variables by taking advantage of the fact that "_dl_nloaded == _dl_load_adds - _dl_load_subs" (provided there are never more than 0xffffffff shared objects loaded). Apart from saving 8 bytes of global data, it also has the advantage that it makes code maintenance a bit easier. The rule is very simple now: wherever _dl_nloaded gets incremented, _dl_load_adds needs to be incremented as well. If it looks OK to you, please apply (it was "make subdirs=elf check" tested). Thanks, --david ChangeLog 2004-01-26 David Mosberger <davidm@hpl.hp.com> * elf/link.h (struct dl_phdr_info): Fix typo in comment. * elf/rtld.c (dl_main): After incrementing dl_nloaded, also increment dl_load_adds. * elf/dl-iteratephdr.c (__dl_iterate_phdr): Replace GL(dl_load_subs) with equivalent GL(dl_load_adds - GL(dl_nloaded). (dl_iterate_phdr): Likewise. * elf/dl-close.c (_dl_close): Delete increment of GL(dl_load_subs). * elf/dl-support.c: Remove global variable _dl_load_subs. * sysdeps/generic/ldsodefs.h (struct rtld_global): Remove _dl_load_subs member. Index: elf/dl-close.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-close.c,v retrieving revision 1.99 diff -u -r1.99 dl-close.c --- elf/dl-close.c 24 Jan 2004 20:07:38 -0000 1.99 +++ elf/dl-close.c 26 Jan 2004 21:42:35 -0000 @@ -319,7 +319,6 @@ /* Notify the debugger we are about to remove some loaded objects. */ _r_debug.r_state = RT_DELETE; _dl_debug_state (); - ++GL(dl_load_subs); #ifdef USE_TLS size_t tls_free_start; Index: elf/dl-iteratephdr.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-iteratephdr.c,v retrieving revision 1.10 diff -u -r1.10 dl-iteratephdr.c --- elf/dl-iteratephdr.c 24 Jan 2004 20:07:51 -0000 1.10 +++ elf/dl-iteratephdr.c 26 Jan 2004 21:42:35 -0000 @@ -49,7 +49,7 @@ info.dlpi_phdr = l->l_phdr; info.dlpi_phnum = l->l_phnum; info.dlpi_adds = GL(dl_load_adds); - info.dlpi_subs = GL(dl_load_subs); + info.dlpi_subs = GL(dl_load_adds) - GL(dl_nloaded); ret = callback (&info, sizeof (struct dl_phdr_info), data); if (ret) break; @@ -87,7 +87,7 @@ info.dlpi_phdr = _dl_phdr; info.dlpi_phnum = _dl_phnum; info.dlpi_adds = GL(dl_load_adds); - info.dlpi_subs = GL(dl_load_subs); + info.dlpi_subs = GL(dl_load_adds) - GL(dl_nloaded); ret = (*callback) (&info, sizeof (struct dl_phdr_info), data); if (ret) return ret; Index: elf/dl-support.c =================================================================== RCS file: /cvs/glibc/libc/elf/dl-support.c,v retrieving revision 1.80 diff -u -r1.80 dl-support.c --- elf/dl-support.c 24 Jan 2004 20:08:29 -0000 1.80 +++ elf/dl-support.c 26 Jan 2004 21:42:35 -0000 @@ -73,8 +73,6 @@ /* Incremented whenever something may have been added to dl_loaded. */ unsigned long long _dl_load_adds; -/* Incremented whenever something may have been removed from dl_loaded. */ -unsigned long long _dl_load_subs; /* Fake scope. In dynamically linked binaries this is the scope of the main application but here we don't have something like this. So Index: elf/link.h =================================================================== RCS file: /cvs/glibc/libc/elf/link.h,v retrieving revision 1.77 diff -u -r1.77 link.h --- elf/link.h 24 Jan 2004 20:08:50 -0000 1.77 +++ elf/link.h 26 Jan 2004 21:42:35 -0000 @@ -103,7 +103,7 @@ /* Note: the next two members were introduced after the first version of this structure was available. Check the SIZE - argument pass to the dl_iterate_phdr() callback to determine + argument passed to the dl_iterate_phdr() callback to determine whether or not they are provided. */ /* Incremented when a new object may have been added. */ Index: elf/rtld.c =================================================================== RCS file: /cvs/glibc/libc/elf/rtld.c,v retrieving revision 1.310 diff -u -r1.310 rtld.c --- elf/rtld.c 24 Jan 2004 08:33:44 -0000 1.310 +++ elf/rtld.c 26 Jan 2004 21:42:35 -0000 @@ -1020,6 +1020,7 @@ GL(dl_loaded)->l_next = &GL(dl_rtld_map); GL(dl_rtld_map).l_prev = GL(dl_loaded); ++GL(dl_nloaded); + ++GL(dl_load_adds); /* If LD_USE_LOAD_BIAS env variable has not been seen, default to not using bias for non-prelinked PIEs and libraries Index: sysdeps/generic/ldsodefs.h =================================================================== RCS file: /cvs/glibc/libc/sysdeps/generic/ldsodefs.h,v retrieving revision 1.89 diff -u -r1.89 ldsodefs.h --- sysdeps/generic/ldsodefs.h 24 Jan 2004 20:10:18 -0000 1.89 +++ sysdeps/generic/ldsodefs.h 26 Jan 2004 21:42:36 -0000 @@ -267,8 +267,6 @@ /* Incremented whenever something may have been added to dl_loaded. */ EXTERN unsigned long long _dl_load_adds; - /* Incremented whenever something may have been removed from dl_loaded. */ - EXTERN unsigned long long _dl_load_subs; #ifndef MAP_ANON /* File descriptor referring to the zero-fill device. */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-26 21:48 ` David Mosberger @ 2004-01-26 22:39 ` Ulrich Drepper 2004-01-26 22:58 ` Roland McGrath 2004-01-26 23:03 ` David Mosberger 0 siblings, 2 replies; 14+ messages in thread From: Ulrich Drepper @ 2004-01-26 22:39 UTC (permalink / raw) To: davidm; +Cc: GNU libc hackers David Mosberger wrote: > In the meantime, it occurred to me that we can get rid of one of the > "unsigned long long" variables by taking advantage of the fact that > "_dl_nloaded == _dl_load_adds - _dl_load_subs" _dl_nloaded is not available outside ld.so. -- ⧠Ulrich Drepper ⧠Red Hat, Inc. ⧠444 Castro St ⧠Mountain View, CA â ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-26 22:39 ` Ulrich Drepper @ 2004-01-26 22:58 ` Roland McGrath 2004-01-26 23:03 ` David Mosberger 1 sibling, 0 replies; 14+ messages in thread From: Roland McGrath @ 2004-01-26 22:58 UTC (permalink / raw) To: Ulrich Drepper; +Cc: davidm, GNU libc hackers > David Mosberger wrote: > > > In the meantime, it occurred to me that we can get rid of one of the > > "unsigned long long" variables by taking advantage of the fact that > > "_dl_nloaded == _dl_load_adds - _dl_load_subs" > > _dl_nloaded is not available outside ld.so. It's part of rtld_global, just like _dl_load_adds is. We are only talking about dl_iterate_phdr's implementation here. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: enabling caching for dl_iterate_phdr() 2004-01-26 22:39 ` Ulrich Drepper 2004-01-26 22:58 ` Roland McGrath @ 2004-01-26 23:03 ` David Mosberger 1 sibling, 0 replies; 14+ messages in thread From: David Mosberger @ 2004-01-26 23:03 UTC (permalink / raw) To: Ulrich Drepper; +Cc: davidm, GNU libc hackers >>>>> On Mon, 26 Jan 2004 14:26:07 -0800, Ulrich Drepper <drepper@redhat.com> said: Uli> David Mosberger wrote: >> In the meantime, it occurred to me that we can get rid of one of >> the "unsigned long long" variables by taking advantage of the >> fact that "_dl_nloaded == _dl_load_adds - _dl_load_subs" Uli> _dl_nloaded is not available outside ld.so. I'm probably missing something obvious, but why is this a problem? As far as I can see, _dl_nloaded has the same visibility as _dl_load_subs. --david ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2004-01-26 23:03 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-12-16 19:55 enabling caching for dl_iterate_phdr() David Mosberger 2003-12-23 22:24 ` Roland McGrath [not found] <16387.9755.753294.37588@napali.hpl.hp.com> 2004-01-17 0:57 ` Roland McGrath 2004-01-17 1:39 ` Jakub Jelinek 2004-01-17 1:40 ` Roland McGrath 2004-01-17 1:51 ` David Mosberger 2004-01-22 0:35 ` David Mosberger 2004-01-22 8:39 ` Jakub Jelinek 2004-01-24 5:54 ` David Mosberger 2004-01-24 20:27 ` Ulrich Drepper 2004-01-26 21:48 ` David Mosberger 2004-01-26 22:39 ` Ulrich Drepper 2004-01-26 22:58 ` Roland McGrath 2004-01-26 23:03 ` David Mosberger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).