Patchwork [v2] Enable VDSO on statically linked programs.

login
register
mail settings
Submitter Rafael Avila de Espindola
Date Sept. 21, 2018, 11:40 p.m.
Message ID <Rt9LNuZNJXNeI-mSgZ_YAja7MzVVLO7IYVyN2uyhHcnpBpJH9yzQATx-2GaiHZoMA6mScM6GvrheNFkdCQK4ZGmDbbXhVJ-RI1dp0AGlezA=@espindo.la>
Download mbox | patch
Permalink /patch/29501/
State New
Headers show

Comments

Rafael Avila de Espindola - Sept. 21, 2018, 11:40 p.m.
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
H.J. Lu - Sept. 22, 2018, 6:18 p.m.
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.
Florian Weimer - Sept. 22, 2018, 6:29 p.m.
* 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
Rafael Avila de Espindola - Sept. 24, 2018, 1:24 a.m.
"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
Rafael Avila de Espindola - Sept. 24, 2018, 1:46 a.m.
"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
Florian Weimer - Sept. 24, 2018, 8:43 a.m.
* 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
Szabolcs Nagy - Sept. 24, 2018, 9:28 a.m.
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.
H.J. Lu - Sept. 24, 2018, 1:25 p.m.
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
>

Patch

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