[v2] Enable VDSO on statically linked programs.
Commit Message
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.
Cheers,
Rafael
Comments
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
@@ -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
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-cond11.c"
@@ -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
@@ -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 */
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-affinity.c"
@@ -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 */
@@ -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