Patchwork i386: Add _startup_sbrk and _startup_fatal [BZ #21913]

login
register
mail settings
Submitter H.J. Lu
Date Aug. 8, 2017, 3:29 p.m.
Message ID <CAMe9rOr_vCsm3YLL8vCRmObJPxwoSKbm96zfcoJ4KjG2zTTySQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/21980/
State New
Headers show

Comments

H.J. Lu - Aug. 8, 2017, 3:29 p.m.
On Tue, Aug 8, 2017 at 5:43 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> 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.
>
>
>>
>> +/* 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.

Done.

>
>> +/* 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.

Done.


>> +
>> +#ifdef BUILD_PIE_DEFAULT
>
> Likewise.

Done.

>> +# include <abort-instr.h>
>> +
>

Here is the patch I am checking in.

Patch

From a5eb29646bbc64530b4133c08061079ae3450633 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 != 0]
	(I386_USE_SYSENTER): New.  Defined to 0.
---
 csu/libc-tls.c                         |  3 ++-
 elf/dl-tunables.c                      |  1 +
 include/libc-symbols.h                 |  8 ++++++++
 sysdeps/generic/startup.h              | 23 ++++++++++++++++++++++
 sysdeps/unix/sysv/linux/i386/brk.c     |  5 +++++
 sysdeps/unix/sysv/linux/i386/startup.h | 36 ++++++++++++++++++++++++++++++++++
 6 files changed, 75 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..fe3ab81c51 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -84,6 +84,14 @@ 
 
 #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 1
+#else
+# define BUILD_PIE_DEFAULT 0
+#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..d67b279d55 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/>.  */
 
+#if 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..b73565a2a5
--- /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/>.  */
+
+#if 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.4