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

[H.J. Lu]

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.



-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-i386-Add-startup.h-BZ-21913.patch
Type: text/x-patch
Size: 7407 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20170808/cd31eddd/attachment.bin>