This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] i386: Add _startup_sbrk and _startup_fatal [BZ #21913]
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 7 Aug 2017 09:48:11 -0700
- Subject: Re: [PATCH] i386: Add _startup_sbrk and _startup_fatal [BZ #21913]
- Authentication-results: sourceware.org; auth=none
- References: <20170806222606.GA13700@gmail.com> <a2c65072-fe5e-ff1a-6a3b-ed75e1e1071f@linaro.org>
On Mon, Aug 7, 2017 at 6:34 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 06/08/2017 19:26, H.J. Lu wrote:
> [..]
>> diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h
>> new file mode 100644
>> index 0000000000..aa63b31181
>> --- /dev/null
>> +++ b/sysdeps/generic/startup.h
>> @@ -0,0 +1,30 @@
>> +/* 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/>. */
>> +
>> +static inline void *
>> +_startup_sbrk (intptr_t __delta)
>> +{
>> + return __sbrk (__delta);
>> +}
>> +
>> +__attribute__ ((__noreturn__))
>> +static inline void
>> +_startup_fatal (const char *__message)
>> +{
>> + __libc_fatal (__message);
>> +}
>
> I think there is no need of underlying prefixes for inline functions.
But _startup_sbrk may not be inlined for i386.
>> diff --git a/sysdeps/unix/sysv/linux/i386/Makefile b/sysdeps/unix/sysv/linux/i386/Makefile
>> index 4080b8c966..cefa1511f6 100644
>> --- a/sysdeps/unix/sysv/linux/i386/Makefile
>> +++ b/sysdeps/unix/sysv/linux/i386/Makefile
>> @@ -31,6 +31,10 @@ sysdep_routines += divdi3
>> shared-only-routines += divdi3
>> CPPFLAGS-divdi3.c = -Din_divdi3_c
>> endif
>> +ifneq (,$(pic-default))
>> +sysdep_routines += startup_sbrk
>> +static-only-routines += startup_sbrk
>> +endif
>> endif
>>
>> ifeq ($(subdir),nptl)
>> diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h
>> new file mode 100644
>> index 0000000000..ccfba45153
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/i386/startup.h
>> @@ -0,0 +1,38 @@
>> +/* 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 defined PIC && !defined SHARED
>
> We already check and set build-pie-default on configure/make.in, wouldn't
> be useful if we also define a BUILD_PIE as well?
We can have
#if define PIC && !defined SHARED
# define BUILD_PIE
#endif
in include/libc-symbols.h or config.h.in
>> +# include <abort-instr.h>
>> +
>> +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */
>> +# define I386_USE_SYSENTER 0
>> +
>> +extern void * _startup_sbrk (intptr_t) attribute_hidden;
>> +
>> +__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 ();
>> +}
>
> Maybe also provide a __writev using 'int $0x80' so it can use _dl_debug_printf?
_dl_debug_printf calls _dl_debug_vdprintf which makes quite
a few syscalls. _startup_fatal is called very early during start
up when something goes wrong. When it is called, something
very very bad must have happened. I don't think ABORT_INSTRUCTION
is a terrible choice.
>
>> +#else
>> +# include_next <startup.h>
>> +#endif
>> diff --git a/sysdeps/unix/sysv/linux/i386/startup_sbrk.c b/sysdeps/unix/sysv/linux/i386/startup_sbrk.c
>> new file mode 100644
>> index 0000000000..8239938ddf
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/i386/startup_sbrk.c
>
> I am not very found of replicating generic code, but for this case it seems
> that i386 version seems to have too many specific cases that I am not sure
> if it worth trying to consolidate it.
>
>> @@ -0,0 +1,67 @@
>> +/* Linux/i386 definitions of _startup_sbrk.
>> + 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/>. */
>> +
>> +#include <unistd.h>
>> +#include <startup.h>
>> +#include <errno.h>
>> +#include <sysdep.h>
>> +
>> +/* Defined in brk.c. */
>> +extern void *__curbrk attribute_hidden;
>> +
>> +static int
>> +startup_brk (void *addr)
>> +{
>> + INTERNAL_SYSCALL_DECL (err);
>> + void *newbrk = (void *) INTERNAL_SYSCALL_CALL (brk, err, addr);
>> + __curbrk = newbrk;
>> + if (newbrk < addr)
>> + _startup_fatal (NULL);
>> + return 0;
>> +}
>> +
>> +/* Extend the process's data space by INCREMENT. If INCREMENT is negative,
>> + shrink data space by - INCREMENT. Return start of new space allocated,
>> + or call _startup_fatal for errors. */
>> +
>> +void *
>> +_startup_sbrk (intptr_t increment)
>> +{
>> + void *oldbrk;
>> +
>> + /* Update __curbrk from the kernel's brk value. That way two separate
>> + instances of __brk and __sbrk can share the heap, returning
>> + interleaved pieces of it. */
>> + if (__curbrk == NULL)
>> + if (startup_brk (0) < 0) /* Initialize the break. */
>> + _startup_fatal (NULL);
>> +
>> + if (increment == 0)
>> + return __curbrk;
>> +
>> + oldbrk = __curbrk;
>> + if (increment > 0
>> + ? ((uintptr_t) oldbrk + (uintptr_t) increment < (uintptr_t) oldbrk)
>> + : ((uintptr_t) oldbrk < (uintptr_t) -increment))
>> + _startup_fatal (NULL);
>> +
>> + if (startup_brk (oldbrk + increment) < 0)
>> + _startup_fatal (NULL);
>> +
>> + return oldbrk;
>> +}
>>
--
H.J.