| Message ID | Rt9LNuZNJXNeI-mSgZ_YAja7MzVVLO7IYVyN2uyhHcnpBpJH9yzQATx-2GaiHZoMA6mScM6GvrheNFkdCQK4ZGmDbbXhVJ-RI1dp0AGlezA=@espindo.la |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Enable VDSO on statically linked programs. | expand |
On Fri, Sep 21, 2018 at 4:40 PM, Rafael Avila de Espindola <rafael@espindo.la> wrote: > The new version has fixed the indentation of preprocessor directives and changed a few tests to also be linked statically. The tests don't show that the VDSO is being used, but show that the functions now using the VDSO still work. > 2 comments: 1. Why isn't i386 enabled? 2. ChangeLog entries are missing.
* Rafael Avila de Espindola: > The new version has fixed the indentation of preprocessor directives > and changed a few tests to also be linked statically. The tests don't > show that the VDSO is being used, but show that the functions now > using the VDSO still work. What's the size impact on binaries which did not need rtld before? Thanks, Florian
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Fri, Sep 21, 2018 at 4:40 PM, Rafael Avila de Espindola > <rafael@espindo.la> wrote: >> The new version has fixed the indentation of preprocessor directives and changed a few tests to also be linked statically. The tests don't show that the VDSO is being used, but show that the functions now using the VDSO still work. >> > > 2 comments: > > 1. Why isn't i386 enabled? It is probably better to do other architectures as a follow up, no? I do volunteer to do at least i386. > 2. ChangeLog entries are missing. Sorry, it is 2018-09-23 Rafael Ávila de Espíndola <rafael@espindo.la> * nptl/Makefile: Add tst-cond11-static to tests-static and tests. * nptl/tst-cond11-static.c: New File. * sysdeps/unix/sysv/linux/sysdep-vdso.h: remove #ifdef SHARED. * sysdeps/unix/sysv/linux/x86/libc-vdso.h: remove #ifdef SHARED. * sysdeps/unix/sysv/linux/x86_64/init-first.c: remove #ifdef SHARED. * sysdeps/unix/sysv/linux/Makefile: Add tst-affinity-static to tests-static and tests * sysdeps/unix/sysv/linux/tst-affinity-static.c: New file. Cheers, Rafael
"Florian Weimer" <fweimer@redhat.com> writes: > * Rafael Avila de Espindola: > >> The new version has fixed the indentation of preprocessor directives >> and changed a few tests to also be linked statically. The tests don't >> show that the VDSO is being used, but show that the functions now >> using the VDSO still work. > > What's the size impact on binaries which did not need rtld before? In a trivial program that just calls clock_gettime and printf size reports: text data bss dec hex filename 641724 20940 6016 668680 a3408 t-glibc 642500 20940 6048 669488 a3730 t-glibc-vdso So it looks like most of the relevant code was already being linked in. Cheers, Rafael
* Rafael Avila de Espindola: > "Florian Weimer" <fweimer@redhat.com> writes: > >> * Rafael Avila de Espindola: >> >>> The new version has fixed the indentation of preprocessor directives >>> and changed a few tests to also be linked statically. The tests don't >>> show that the VDSO is being used, but show that the functions now >>> using the VDSO still work. >> >> What's the size impact on binaries which did not need rtld before? > > In a trivial program that just calls clock_gettime and printf size > reports: > > text data bss dec hex filename > 641724 20940 6016 668680 a3408 t-glibc > 642500 20940 6048 669488 a3730 t-glibc-vdso > > So it looks like most of the relevant code was already being linked in. Hmm, right. printf pulls in dlopen (via libio and gconv), so it is not the right symbol test. It currently seems to be impossible to link without dlopen because libio is always included. However, we want to remove the dependency of libio (and NSS) on dlopen in the static case, so eventually, that will change. I guess once this has happened, we can probably build a stripped-down version of rtld just for processing the vDSO. Thanks, Florian
On 24/09/18 02:24, Rafael Avila de Espindola wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >> On Fri, Sep 21, 2018 at 4:40 PM, Rafael Avila de Espindola >> <rafael@espindo.la> wrote: >>> The new version has fixed the indentation of preprocessor directives and changed a few tests to also be linked statically. The tests don't show that the VDSO is being used, but show that the functions now using the VDSO still work. >>> >> >> 2 comments: >> >> 1. Why isn't i386 enabled? > > It is probably better to do other architectures as a follow up, no? I do > volunteer to do at least i386. > > >> 2. ChangeLog entries are missing. > > Sorry, it is > > 2018-09-23 Rafael Ávila de Espíndola <rafael@espindo.la> > > * nptl/Makefile: Add tst-cond11-static to tests-static and tests. > * nptl/tst-cond11-static.c: New File. > * sysdeps/unix/sysv/linux/sysdep-vdso.h: remove #ifdef SHARED. > * sysdeps/unix/sysv/linux/x86/libc-vdso.h: remove #ifdef SHARED. > * sysdeps/unix/sysv/linux/x86_64/init-first.c: remove #ifdef SHARED. > * sysdeps/unix/sysv/linux/Makefile: Add tst-affinity-static to > tests-static and tests > * sysdeps/unix/sysv/linux/tst-affinity-static.c: New file. > this should include the bug number 19767 the title as well as the description should make it clear that only x86 behaviour is changed, this is a generic issue in glibc and ideally all targets should have the same behaviour eventually. it is also useful to mention if you tested the patch on non-x86 targets since the header changes seem to affect them.
On Sun, Sep 23, 2018 at 6:24 PM, Rafael Avila de Espindola <rafael@espindo.la> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >> On Fri, Sep 21, 2018 at 4:40 PM, Rafael Avila de Espindola >> <rafael@espindo.la> wrote: >>> The new version has fixed the indentation of preprocessor directives and changed a few tests to also be linked statically. The tests don't show that the VDSO is being used, but show that the functions now using the VDSO still work. >>> >> >> 2 comments: >> >> 1. Why isn't i386 enabled? > > It is probably better to do other architectures as a follow up, no? I do > volunteer to do at least i386. Your patch changed sysdeps/unix/sysv/linux/x86/libc-vdso.h which is used for both i386 and x86-64. You just need to make similar changes in sysdeps/unix/sysv/linux/i386/init-first.c. BTW, You can test i686 on x86-64. > >> 2. ChangeLog entries are missing. > > Sorry, it is > > 2018-09-23 Rafael Ávila de Espíndola <rafael@espindo.la> > > * nptl/Makefile: Add tst-cond11-static to tests-static and tests. > * nptl/tst-cond11-static.c: New File. > * sysdeps/unix/sysv/linux/sysdep-vdso.h: remove #ifdef SHARED. > * sysdeps/unix/sysv/linux/x86/libc-vdso.h: remove #ifdef SHARED. > * sysdeps/unix/sysv/linux/x86_64/init-first.c: remove #ifdef SHARED. > * sysdeps/unix/sysv/linux/Makefile: Add tst-affinity-static to > tests-static and tests > * sysdeps/unix/sysv/linux/tst-affinity-static.c: New file. > > Cheers, > Rafael >
From a4563236a090f7e02c82b720ca449f5ef7772771 Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola <rafael@espindo.la> Date: Thu, 20 Sep 2018 08:23:30 -0700 Subject: [PATCH] Enable VDSO on statically linked programs. All the required code already existed, and some of it was already running. AT_SYSINFO_EHDR is processed if NEED_DL_SYSINFO_DSO is defined, but it looks like it always is. The call to setup_vdso is also unconditional, so all that was left to do was setup the function pointers and use them. This patch just deletes some #ifdef to enable that. --- nptl/Makefile | 5 ++-- nptl/tst-cond11-static.c | 1 + sysdeps/unix/sysv/linux/Makefile | 3 +++ sysdeps/unix/sysv/linux/sysdep-vdso.h | 27 ++++++------------- sysdeps/unix/sysv/linux/tst-affinity-static.c | 1 + sysdeps/unix/sysv/linux/x86/libc-vdso.h | 6 +---- sysdeps/unix/sysv/linux/x86_64/init-first.c | 12 ++++----- 7 files changed, 22 insertions(+), 33 deletions(-) create mode 100644 nptl/tst-cond11-static.c create mode 100644 sysdeps/unix/sysv/linux/tst-affinity-static.c diff --git a/nptl/Makefile b/nptl/Makefile index be8066524c..1b384548a1 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -449,9 +449,10 @@ link-libc-static := $(common-objpfx)libc.a $(static-gnulib) \ tests-static += tst-locale1 tst-locale2 tst-stackguard1-static \ tst-cancel21-static tst-cancel24-static tst-cond8-static \ tst-mutex8-static tst-mutexpi8-static tst-sem11-static \ - tst-sem12-static + tst-sem12-static tst-cond11-static + tests += tst-cancel21-static tst-cancel24-static \ - tst-cond8-static + tst-cond8-static tst-cond11-static tests-internal += tst-sem11-static tst-sem12-static tst-stackguard1-static xtests-static += tst-setuid1-static diff --git a/nptl/tst-cond11-static.c b/nptl/tst-cond11-static.c new file mode 100644 index 0000000000..9bccb8ec8b --- /dev/null +++ b/nptl/tst-cond11-static.c @@ -0,0 +1 @@ +#include "tst-cond11.c" diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 773aaea0e9..05a4ab1e55 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -143,6 +143,9 @@ sysdep_routines += sched_getcpu oldglob tests += tst-affinity tst-affinity-pid +tests-static := tst-affinity-static +tests += $(tests-static) + CFLAGS-fork.c = $(libio-mtsafe) CFLAGS-getpid.o = -fomit-frame-pointer CFLAGS-getpid.os = -fomit-frame-pointer diff --git a/sysdeps/unix/sysv/linux/sysdep-vdso.h b/sysdeps/unix/sysv/linux/sysdep-vdso.h index 1912c1c156..9e8b1ce2e4 100644 --- a/sysdeps/unix/sysv/linux/sysdep-vdso.h +++ b/sysdeps/unix/sysv/linux/sysdep-vdso.h @@ -26,13 +26,11 @@ funcptr (args) #endif -#ifdef SHARED +#ifdef HAVE_VSYSCALL -# ifdef HAVE_VSYSCALL +# include <libc-vdso.h> -# include <libc-vdso.h> - -# define INLINE_VSYSCALL(name, nr, args...) \ +# define INLINE_VSYSCALL(name, nr, args...) \ ({ \ __label__ out; \ __label__ iserr; \ @@ -61,7 +59,7 @@ sc_ret; \ }) -# define INTERNAL_VSYSCALL(name, err, nr, args...) \ +# define INTERNAL_VSYSCALL(name, err, nr, args...) \ ({ \ __label__ out; \ long v_ret; \ @@ -79,20 +77,11 @@ out: \ v_ret; \ }) -# else -# define INLINE_VSYSCALL(name, nr, args...) \ - INLINE_SYSCALL (name, nr, ##args) -# define INTERNAL_VSYSCALL(name, err, nr, args...) \ - INTERNAL_SYSCALL (name, err, nr, ##args) -# endif /* HAVE_VSYSCALL */ - -# else /* SHARED */ - -# define INLINE_VSYSCALL(name, nr, args...) \ +#else +# define INLINE_VSYSCALL(name, nr, args...) \ INLINE_SYSCALL (name, nr, ##args) -# define INTERNAL_VSYSCALL(name, err, nr, args...) \ +# define INTERNAL_VSYSCALL(name, err, nr, args...) \ INTERNAL_SYSCALL (name, err, nr, ##args) - -#endif /* SHARED */ +#endif /* HAVE_VSYSCALL */ #endif /* SYSDEP_VDSO_LINUX_H */ diff --git a/sysdeps/unix/sysv/linux/tst-affinity-static.c b/sysdeps/unix/sysv/linux/tst-affinity-static.c new file mode 100644 index 0000000000..4022ea317a --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-affinity-static.c @@ -0,0 +1 @@ +#include "tst-affinity.c" diff --git a/sysdeps/unix/sysv/linux/x86/libc-vdso.h b/sysdeps/unix/sysv/linux/x86/libc-vdso.h index 6f86073dae..7f4e730a32 100644 --- a/sysdeps/unix/sysv/linux/x86/libc-vdso.h +++ b/sysdeps/unix/sysv/linux/x86/libc-vdso.h @@ -22,9 +22,7 @@ #include <time.h> #include <sys/time.h> -#ifdef SHARED - -# include <sysdep-vdso.h> +#include <sysdep-vdso.h> extern long int (*VDSO_SYMBOL(clock_gettime)) (clockid_t, struct timespec *) attribute_hidden; @@ -32,6 +30,4 @@ extern long int (*VDSO_SYMBOL(clock_gettime)) (clockid_t, struct timespec *) extern long int (*VDSO_SYMBOL(getcpu)) (unsigned *, unsigned *, void *) attribute_hidden; -#endif - #endif /* _LIBC_VDSO_H */ diff --git a/sysdeps/unix/sysv/linux/x86_64/init-first.c b/sysdeps/unix/sysv/linux/x86_64/init-first.c index 2320505804..ad19f4b055 100644 --- a/sysdeps/unix/sysv/linux/x86_64/init-first.c +++ b/sysdeps/unix/sysv/linux/x86_64/init-first.c @@ -16,11 +16,10 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#ifdef SHARED -# include <time.h> -# include <sysdep.h> -# include <dl-vdso.h> -# include <libc-vdso.h> +#include <time.h> +#include <sysdep.h> +#include <dl-vdso.h> +#include <libc-vdso.h> long int (*VDSO_SYMBOL(clock_gettime)) (clockid_t, struct timespec *) attribute_hidden; @@ -46,7 +45,6 @@ __vdso_platform_setup (void) VDSO_SYMBOL(getcpu) = p; } -# define VDSO_SETUP __vdso_platform_setup -#endif +#define VDSO_SETUP __vdso_platform_setup #include <csu/init-first.c> -- 2.18.0