Message ID | CAMe9rOpRtZjbozd0-QDExPvWrpNvaEca+S5-fDjCWJJeDH2eFQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Mon, Aug 7, 2017 at 5:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > Here is the updated patch. OK for master? Thanks for the quick turnaround! Please find a more logical place in libc-symbols.h for the definition of BUILD_PIE_DEFAULT; I didn't mean to suggest you should put it _immediately_ after the #include <config.h>. Otherwise, looks good to me, but please wait at least 48 hours for more feedback; this is not code I understand deeply, so I don't feel like I can be the sole reviewer. zw
On Mon, Aug 7, 2017 at 2:24 PM, Zack Weinberg <zackw@panix.com> wrote: > On Mon, Aug 7, 2017 at 5:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >> Here is the updated patch. OK for master? > > Thanks for the quick turnaround! > > Please find a more logical place in libc-symbols.h for the definition > of BUILD_PIE_DEFAULT; I didn't mean to suggest you should put it > _immediately_ after the #include <config.h>. Otherwise, looks good to I think that place isn't bad. I have a follwup patch to reduce the size of libc.a when PIE is the default. It is better to define BUILD_PIE_DEFAULT early. > me, but please wait at least 48 hours for more feedback; this is not > code I understand deeply, so I don't feel like I can be the sole > reviewer. Will do. Thanks.
On 08/07/2017 05:17 PM, H.J. Lu wrote: > From 58d4201ae107eab72478a366a24135246236f060 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Wed, 19 Jul 2017 14:32:42 -0700 > Subject: [PATCH] i386: Add <startup.h> [BZ #21913] > > On Linux/i386, there are 3 ways to make a system call: > > 1. call *%gs:SYSINFO_OFFSET. This requires TLS initialization. > 2. call *_dl_sysinfo. This requires relocation of _dl_sysinfo. > 3. int $0x80. This is slower than #2 and #3, but works everywhere. > > When an object file is compiled with PIC, #1 is prefered since it is > faster than #3 and doesn't require relocation of _dl_sysinfo. For > dynamic executables, ld.so initializes TLS. However, for static > executables, before TLS is initialized by __libc_setup_tls, #3 should > be used for system calls. > > This patch adds <startup.h> which defines _startup_fatal and defaults > it to __libc_fatal. It replaces __libc_fatal with _startup_fatal in > static executables where it is called before __libc_setup_tls is called. > This header file is included in all files containing functions which are > called before __libc_setup_tls is called. On Linux/i386, when PIE is > enabled by default, _startup_fatal is turned into ABORT_INSTRUCTION and > I386_USE_SYSENTER is defined to 0 so that "int $0x80" is used for system > calls before __libc_setup_tls is called. > > Tested on i686 and x86-64. Without this patch, all statically-linked > tests will fail on i686 when the compiler defaults to -fPIE. Overall I don't have any objections except that you have a typo-prone macro API that needs fixing. > [BZ #21913] > * csu/libc-tls.c: Include <startup.h> first. > (__libc_setup_tls): Call _startup_fatal instead of __libc_fatal. > * elf/dl-tunables.c: Include <startup.h> first. > * include/libc-symbols.h (BUILD_PIE_DEFAULT): New. > * sysdeps/generic/startup.h: New file. > * sysdeps/unix/sysv/linux/i386/startup.h: Likewise. > * sysdeps/unix/sysv/linux/i386/brk.c [BUILD_PIE_DEFAULT] > (I386_USE_SYSENTER): New. Defined to 0. > --- > csu/libc-tls.c | 3 ++- > elf/dl-tunables.c | 1 + > include/libc-symbols.h | 6 ++++++ > sysdeps/generic/startup.h | 23 ++++++++++++++++++++++ > sysdeps/unix/sysv/linux/i386/brk.c | 5 +++++ > sysdeps/unix/sysv/linux/i386/startup.h | 36 ++++++++++++++++++++++++++++++++++ > 6 files changed, 73 insertions(+), 1 deletion(-) > create mode 100644 sysdeps/generic/startup.h > create mode 100644 sysdeps/unix/sysv/linux/i386/startup.h > > diff --git a/csu/libc-tls.c b/csu/libc-tls.c > index 3c897bf28b..00138eb43a 100644 > --- a/csu/libc-tls.c > +++ b/csu/libc-tls.c > @@ -16,6 +16,7 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > +#include <startup.h> > #include <errno.h> > #include <ldsodefs.h> > #include <tls.h> > @@ -193,7 +194,7 @@ __libc_setup_tls (void) > # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" > #endif > if (__builtin_expect (lossage != NULL, 0)) > - __libc_fatal (lossage); > + _startup_fatal (lossage); > > /* Update the executable's link map with enough information to make > the TLS routines happy. */ > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 231fb8ca93..b964a09413 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -18,6 +18,7 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > +#include <startup.h> > #include <stdint.h> > #include <stdbool.h> > #include <unistd.h> > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > index 3310e3a678..9a94676c43 100644 > --- a/include/libc-symbols.h > +++ b/include/libc-symbols.h > @@ -84,6 +84,12 @@ > > #include <config.h> > > +/* When PIC is defined and SHARED isn't defined, we are building PIE > + by default. */ > +#if defined PIC && !defined SHARED > +# define BUILD_PIE_DEFAULT > +#endif This is typo-prone. We should define BUILD_PIE_DEFAULT to 1 or 0, and always defined. > + > /* Define this for the benefit of portable GNU code that wants to check it. > Code that checks with #if will not #include <config.h> again, since we've > already done it (and this file is implicitly included in every compile, > diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h > new file mode 100644 > index 0000000000..a961e277bf > --- /dev/null > +++ b/sysdeps/generic/startup.h > @@ -0,0 +1,23 @@ > +/* Generic definitions of functions used by static libc main startup. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + 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, see > + <http://www.gnu.org/licenses/>. */ > + > +/* Targets should override this file if the default definitions below > + will not work correctly very early before TLS is initialized. */ > + > +/* Use macro instead of inline function to avoid including <stdio.h>. */ > +#define _startup_fatal(message) __libc_fatal ((message)) > diff --git a/sysdeps/unix/sysv/linux/i386/brk.c b/sysdeps/unix/sysv/linux/i386/brk.c > index 25ab1015d3..b55f0236d1 100644 > --- a/sysdeps/unix/sysv/linux/i386/brk.c > +++ b/sysdeps/unix/sysv/linux/i386/brk.c > @@ -16,6 +16,11 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > +#ifdef BUILD_PIE_DEFAULT This is a typo-prone macro-api and should use #if to allow -Wunused checking. > +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ > +# define I386_USE_SYSENTER 0 > +#endif > + > #include <errno.h> > #include <unistd.h> > #include <sysdep.h> > diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h > new file mode 100644 > index 0000000000..a8fd3134b7 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/i386/startup.h > @@ -0,0 +1,36 @@ > +/* Linux/i386 definitions of functions used by static libc main startup. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + 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, see > + <http://www.gnu.org/licenses/>. */ > + > +#ifdef BUILD_PIE_DEFAULT Likewise. > +# include <abort-instr.h> > + > +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ > +# define I386_USE_SYSENTER 0 > + > +__attribute__ ((__noreturn__)) > +static inline void > +_startup_fatal (const char *message __attribute__ ((unused))) > +{ > + /* This is only called very early during startup in static PIE. > + FIXME: How can it be improved? */ > + ABORT_INSTRUCTION; > + __builtin_unreachable (); > +} > +#else > +# include_next <startup.h> > +#endif > -- 2.13.3
From 58d4201ae107eab72478a366a24135246236f060 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Wed, 19 Jul 2017 14:32:42 -0700 Subject: [PATCH] i386: Add <startup.h> [BZ #21913] On Linux/i386, there are 3 ways to make a system call: 1. call *%gs:SYSINFO_OFFSET. This requires TLS initialization. 2. call *_dl_sysinfo. This requires relocation of _dl_sysinfo. 3. int $0x80. This is slower than #2 and #3, but works everywhere. When an object file is compiled with PIC, #1 is prefered since it is faster than #3 and doesn't require relocation of _dl_sysinfo. For dynamic executables, ld.so initializes TLS. However, for static executables, before TLS is initialized by __libc_setup_tls, #3 should be used for system calls. This patch adds <startup.h> which defines _startup_fatal and defaults it to __libc_fatal. It replaces __libc_fatal with _startup_fatal in static executables where it is called before __libc_setup_tls is called. This header file is included in all files containing functions which are called before __libc_setup_tls is called. On Linux/i386, when PIE is enabled by default, _startup_fatal is turned into ABORT_INSTRUCTION and I386_USE_SYSENTER is defined to 0 so that "int $0x80" is used for system calls before __libc_setup_tls is called. Tested on i686 and x86-64. Without this patch, all statically-linked tests will fail on i686 when the compiler defaults to -fPIE. [BZ #21913] * csu/libc-tls.c: Include <startup.h> first. (__libc_setup_tls): Call _startup_fatal instead of __libc_fatal. * elf/dl-tunables.c: Include <startup.h> first. * include/libc-symbols.h (BUILD_PIE_DEFAULT): New. * sysdeps/generic/startup.h: New file. * sysdeps/unix/sysv/linux/i386/startup.h: Likewise. * sysdeps/unix/sysv/linux/i386/brk.c [BUILD_PIE_DEFAULT] (I386_USE_SYSENTER): New. Defined to 0. --- csu/libc-tls.c | 3 ++- elf/dl-tunables.c | 1 + include/libc-symbols.h | 6 ++++++ sysdeps/generic/startup.h | 23 ++++++++++++++++++++++ sysdeps/unix/sysv/linux/i386/brk.c | 5 +++++ sysdeps/unix/sysv/linux/i386/startup.h | 36 ++++++++++++++++++++++++++++++++++ 6 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 sysdeps/generic/startup.h create mode 100644 sysdeps/unix/sysv/linux/i386/startup.h diff --git a/csu/libc-tls.c b/csu/libc-tls.c index 3c897bf28b..00138eb43a 100644 --- a/csu/libc-tls.c +++ b/csu/libc-tls.c @@ -16,6 +16,7 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#include <startup.h> #include <errno.h> #include <ldsodefs.h> #include <tls.h> @@ -193,7 +194,7 @@ __libc_setup_tls (void) # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined" #endif if (__builtin_expect (lossage != NULL, 0)) - __libc_fatal (lossage); + _startup_fatal (lossage); /* Update the executable's link map with enough information to make the TLS routines happy. */ diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 231fb8ca93..b964a09413 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -18,6 +18,7 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#include <startup.h> #include <stdint.h> #include <stdbool.h> #include <unistd.h> diff --git a/include/libc-symbols.h b/include/libc-symbols.h index 3310e3a678..9a94676c43 100644 --- a/include/libc-symbols.h +++ b/include/libc-symbols.h @@ -84,6 +84,12 @@ #include <config.h> +/* When PIC is defined and SHARED isn't defined, we are building PIE + by default. */ +#if defined PIC && !defined SHARED +# define BUILD_PIE_DEFAULT +#endif + /* Define this for the benefit of portable GNU code that wants to check it. Code that checks with #if will not #include <config.h> again, since we've already done it (and this file is implicitly included in every compile, diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h new file mode 100644 index 0000000000..a961e277bf --- /dev/null +++ b/sysdeps/generic/startup.h @@ -0,0 +1,23 @@ +/* Generic definitions of functions used by static libc main startup. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + 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, see + <http://www.gnu.org/licenses/>. */ + +/* Targets should override this file if the default definitions below + will not work correctly very early before TLS is initialized. */ + +/* Use macro instead of inline function to avoid including <stdio.h>. */ +#define _startup_fatal(message) __libc_fatal ((message)) diff --git a/sysdeps/unix/sysv/linux/i386/brk.c b/sysdeps/unix/sysv/linux/i386/brk.c index 25ab1015d3..b55f0236d1 100644 --- a/sysdeps/unix/sysv/linux/i386/brk.c +++ b/sysdeps/unix/sysv/linux/i386/brk.c @@ -16,6 +16,11 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#ifdef BUILD_PIE_DEFAULT +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ +# define I386_USE_SYSENTER 0 +#endif + #include <errno.h> #include <unistd.h> #include <sysdep.h> diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h new file mode 100644 index 0000000000..a8fd3134b7 --- /dev/null +++ b/sysdeps/unix/sysv/linux/i386/startup.h @@ -0,0 +1,36 @@ +/* Linux/i386 definitions of functions used by static libc main startup. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + 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, see + <http://www.gnu.org/licenses/>. */ + +#ifdef BUILD_PIE_DEFAULT +# include <abort-instr.h> + +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ +# define I386_USE_SYSENTER 0 + +__attribute__ ((__noreturn__)) +static inline void +_startup_fatal (const char *message __attribute__ ((unused))) +{ + /* This is only called very early during startup in static PIE. + FIXME: How can it be improved? */ + ABORT_INSTRUCTION; + __builtin_unreachable (); +} +#else +# include_next <startup.h> +#endif -- 2.13.3