i386: Add _startup_sbrk and _startup_fatal [BZ #21913]
Commit Message
On Mon, Aug 7, 2017 at 10:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Aug 7, 2017 at 10:37 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> On Aug 07 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>
>>> Given this, we need a working sbrk before TLS is initialized.
>>
>> How about making sbrk always use int $0x80?
>>
>> Andreas.
>
> We can do that in sbrk.o and will make change less complex.
> I will give it a try.
>
Here is a patch to add <startup.h>. Tested on i686. OK for master?
Comments
On Mon, Aug 7, 2017 at 4:21 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Aug 7, 2017 at 10:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Aug 7, 2017 at 10:37 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>>>
>>> How about making sbrk always use int $0x80?
>>
>> We can do that in sbrk.o and will make change less complex.
>> I will give it a try.
>
> Here is a patch to add <startup.h>. Tested on i686. OK for master?
This is looking a lot better. I withdraw my earlier objections. (I
would still like to see further investigation of not needing to call
either sbrk or __libc_fatal before TLS is initialized, at least in
static binaries, but it shouldn't block you fixing the immediate
problem.) However:
> 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.
This would be a good point in the commit message to explain how this
is tested. Something like "Without this patch, all statically-linked
tests will fail on i386 when the compiler defaults to -fPIE."
> +/* 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
Please don't add new conditionals to config.h.in. Put them in
libc-symbols.h instead (after the inclusion of config.h). If this
won't work for some reason, please explain.
> --- /dev/null
> +++ b/sysdeps/generic/startup.h
> @@ -0,0 +1,20 @@
> +/* 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/>. */
> +
Document at this point in this file when it is necessary to override
this file. Something like "Ports should override this file when the
default definitions below will not work correctly very early in
startup (e.g. before thread-local storage is initialized)."
> --- a/sysdeps/unix/sysv/linux/i386/brk.c
> +++ b/sysdeps/unix/sysv/linux/i386/brk.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 <unistd.h>
> #include <sysdep.h>
Having linux/i386/startup.h override I386_USE_SYSENTER, and therefore
change how sysdep.h defines INTERNAL_SYSCALL, for just this file, is
too much magic. Someone five years from now is going to come along and
remove that #include on the grounds that it doesn't appear to be necessary.
Since this is already an i386-specific implementation of __brk, it would
be better to have
#ifdef BUILD_PIE_DEFAULT
/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */
# define I386_USE_SYSENTER 0
#endif
directly in this file.
zw
From 99dd28489e425c6f654126871f89d514331aa69f 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 syscalls.
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.
[BZ #21913]
* config.h.in (BUILD_PIE_DEFAULT): New.
* csu/libc-tls.c: Include <startup.h>.
* elf/dl-tunables.c: Likewise.
* sysdeps/unix/sysv/linux/i386/brk.c: Likewise.
* csu/libc-tls.c: Include <startup.h>.
(__libc_setup_tls): Call _startup_fatal instead of __libc_fatal.
* sysdeps/generic/startup.h: New file.
* sysdeps/unix/sysv/linux/i386/startup.h: Likewise.
---
config.h.in | 6 ++++++
csu/libc-tls.c | 3 ++-
elf/dl-tunables.c | 1 +
sysdeps/generic/startup.h | 20 +++++++++++++++++++
sysdeps/unix/sysv/linux/i386/brk.c | 1 +
sysdeps/unix/sysv/linux/i386/startup.h | 36 ++++++++++++++++++++++++++++++++++
6 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 sysdeps/generic/startup.h
create mode 100644 sysdeps/unix/sysv/linux/i386/startup.h
@@ -47,6 +47,12 @@
#undef STACK_PROTECTOR_LEVEL
#endif
+/* 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 if the regparm attribute shall be used for local functions
(gcc on ix86 only). */
#undef USE_REGPARMS
@@ -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. */
@@ -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>
new file mode 100644
@@ -0,0 +1,20 @@
+/* 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/>. */
+
+/* Use macro instead of inline function to avoid including <stdio.h>. */
+#define _startup_fatal(message) __libc_fatal ((message))
@@ -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 <unistd.h>
#include <sysdep.h>
new file mode 100644
@@ -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