Re: enabling caching for dl_iterate_phdr()

[David Mosberger <davidm at napali dot hpl dot hp dot com>]

>>>>> 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;