This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Introduce ELF_INITFINI for all architectures
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Wed, 8 Aug 2018 17:42:10 -0300
- Subject: Re: [PATCH] Introduce ELF_INITFINI for all architectures
- References: <20180623214521.290CE402DFEF9@oldenburg.str.redhat.com> <7e88376d-17ee-2015-4b9a-6cf788971f15@redhat.com>
On 03/07/2018 06:59, Florian Weimer wrote:
> On 06/23/2018 11:45 PM, Florian Weimer wrote:
> Subject: [PATCH] Introduce ELF_INITFINI for all architectures
> To: libc-alpha@sourceware.org
>
> This supersedes the init_array sysdeps directory. It allows us to
> check for ELF_INITFINI in both C and assembler code, and skip DT_INIT
> and DT_FINI processing completely on RISC-V.
>
> A new header file is needed because <dl-machine.h> is incompatible
> with assembler code. <sysdep.h> is compatible with assembler code,
> but it cannot be included in all assembler files because on some
> architectures, it redefines register names, and some assembler files
> conflict with that.
>
> 2018-06-23 Florian Weimer <fweimer@redhat.com>
>
> Introduce ELF_INITFINI, set everywhere except on RISC-V.
> * sysdeps/init_array/crti.S: Move to ...
> * sysdepes/generic/crti.S: here. Report an error if ELF_INITFINI.
> * sysdeps/init_array/crtn.S: Move to ...
> * sysdeps/generic/crtn.S: here. Report an error if ELF_INITFINI.
> * csu/elf-init.c: Check ELF_INITFINI instead of NO_INITFINI.
> * gmon/gmon-start.c [!ELF_INITFINI] (GMON_START_ARRAY_SECTION):
> Define.
> * elf/dl-fini.c (_dl_fini): Check for ELF_INITFINI before using
> DT_FINI.
> * elf/dl-init.c (call_init): Check for ELF_INITFINI before using
> DT_INIT.
> * nptl/pt-crti.S [ELF_INITFINI]: Use .init_array instead of
> PREINIT_FUNCTION.
> * sysdeps/generic/platform-params.h: New file.
> * sysdeps/init_array/elf-init.c: Remove file.
> * sysdeps/init_array/gmon-start.c: Likewise.
> * sysdeps/init_array/pt-crti.S: Likewise.
> * sysdeps/riscv/Implies (init_array): Remove.
> * sysdeps/riscv/platform-params.h: New file.
LGTM to with just a nit and some clarification below.
>
> diff --git a/csu/elf-init.c b/csu/elf-init.c
> index da59b2c77b..8bde25a340 100644
> --- a/csu/elf-init.c
> +++ b/csu/elf-init.c
> @@ -34,6 +34,7 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <stddef.h>
> +#include <platform-params.h>
>
>
> /* These magic symbols are provided by the linker. */
> @@ -49,7 +50,7 @@ extern void (*__fini_array_start []) (void) attribute_hidden;
> extern void (*__fini_array_end []) (void) attribute_hidden;
>
>
> -#ifndef NO_INITFINI
> +#if ELF_INITFINI
> /* These function symbols are provided for the .init/.fini section entry
> points automagically by the linker. */
> extern void _init (void);
> @@ -79,7 +80,7 @@ __libc_csu_init (int argc, char **argv, char **envp)
> }
> #endif
>
> -#ifndef NO_INITFINI
> +#if ELF_INITFINI
> _init ();
> #endif
>
> @@ -99,7 +100,7 @@ __libc_csu_fini (void)
> while (i-- > 0)
> (*__fini_array_start [i]) ();
>
> -# ifndef NO_INITFINI
> +# if ELF_INITFINI
> _fini ();
> # endif
> #endif
> diff --git a/csu/gmon-start.c b/csu/gmon-start.c
> index 6a130890a9..f9fbdbfed8 100644
> --- a/csu/gmon-start.c
> +++ b/csu/gmon-start.c
> @@ -37,6 +37,7 @@
> #include <sys/gmon.h>
> #include <stdlib.h>
> #include <unistd.h>
> +#include <platform-params.h>
> #define __ASSEMBLY__
> #include <entry.h>
>
> @@ -59,6 +60,13 @@ extern char etext[];
> # endif
> #endif
>
> +#if !ELF_INITFINI
> +/* Instead of defining __gmon_start__ globally in gcrt1.o, we make it
> + static and just put a pointer to it into the .preinit_array
> + section. */
> +# define GMON_START_ARRAY_SECTION ".preinit_array"
> +#endif
> +
> #ifdef GMON_START_ARRAY_SECTION
> static void __gmon_start__ (void);
> static void (*const gmon_start_initializer) (void)
> diff --git a/elf/dl-fini.c b/elf/dl-fini.c
> index 3cfc262400..20c5581525 100644
> --- a/elf/dl-fini.c
> +++ b/elf/dl-fini.c
> @@ -19,6 +19,7 @@
> #include <assert.h>
> #include <string.h>
> #include <ldsodefs.h>
> +#include <platform-params.h>
>
>
> /* Type of the constructor functions. */
> @@ -117,7 +118,7 @@ _dl_fini (void)
>
> /* Is there a destructor function? */
> if (l->l_info[DT_FINI_ARRAY] != NULL
> - || l->l_info[DT_FINI] != NULL)
> + || (ELF_INITFINI && l->l_info[DT_FINI] != NULL))
> {
> /* When debugging print a message first. */
> if (__builtin_expect (GLRO(dl_debug_mask)
> @@ -139,7 +140,7 @@ _dl_fini (void)
> }
>
> /* Next try the old-style destructor. */
> - if (l->l_info[DT_FINI] != NULL)
> + if (ELF_INITFINI && l->l_info[DT_FINI] != NULL)
> DL_CALL_DT_FINI
> (l, l->l_addr + l->l_info[DT_FINI]->d_un.d_ptr);
> }
> diff --git a/elf/dl-init.c b/elf/dl-init.c
> index 3e72fa3013..31eb12b974 100644
> --- a/elf/dl-init.c
> +++ b/elf/dl-init.c
> @@ -18,6 +18,7 @@
>
> #include <stddef.h>
> #include <ldsodefs.h>
> +#include <platform-params.h>
>
>
> /* Type of the initializer. */
> @@ -40,9 +41,10 @@ call_init (struct link_map *l, int argc, char **argv, char **env)
> && l->l_type == lt_executable)
> return;
>
> + bool have_init = ELF_INITFINI && l->l_info[DT_INIT] != NULL;
> +
> /* Are there any constructors? */
> - if (l->l_info[DT_INIT] == NULL
> - && __builtin_expect (l->l_info[DT_INIT_ARRAY] == NULL, 1))
> + if (!have_init && __builtin_expect (l->l_info[DT_INIT_ARRAY] == NULL, 1))
> return;
Maybe __glibc_likely.
>
> /* Print a debug message if wanted. */
> @@ -54,7 +56,7 @@ call_init (struct link_map *l, int argc, char **argv, char **env)
> - the one named by DT_INIT
> - the others in the DT_INIT_ARRAY.
> */
> - if (l->l_info[DT_INIT] != NULL)
> + if (have_init)
> DL_CALL_DT_INIT(l, l->l_addr + l->l_info[DT_INIT]->d_un.d_ptr, argc, argv, env);
>
> /* Next see whether there is an array with initialization functions. */
> diff --git a/nptl/pt-crti.S b/nptl/pt-crti.S
> index a0deb9979c..efd28c8f7e 100644
> --- a/nptl/pt-crti.S
> +++ b/nptl/pt-crti.S
> @@ -33,11 +33,18 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> +#include <platform-params.h>
> +
> /* Arrange for __pthread_initialize_minimal_internal to be called at
> libpthread startup, instead of conditionally calling
> __gmon_start__. */
>
> -#define PREINIT_FUNCTION __pthread_initialize_minimal_internal
> -#define PREINIT_FUNCTION_WEAK 0
> +#if ELF_INITFINI
> +# define PREINIT_FUNCTION __pthread_initialize_minimal_internal
> +# define PREINIT_FUNCTION_WEAK 0
>
> -#include <crti.S>
> +# include <crti.S>
> +#else
> + .section .init_array,"a",%init_array
> + .dc.a __pthread_initialize_minimal_internal
> +#endif
> diff --git a/sysdeps/init_array/crti.S b/sysdeps/generic/crti.S
> similarity index 80%
> rename from sysdeps/init_array/crti.S
> rename to sysdeps/generic/crti.S
> index 145c918f93..1a86132b88 100644
> --- a/sysdeps/init_array/crti.S
> +++ b/sysdeps/generic/crti.S
> @@ -12,11 +12,17 @@
> toolchains without .init_array support can use this to avoid the
> superfluous .init and .fini boilerplate code. */
>
> +#include <platform-params.h>
> +
> +#if ELF_INITFINI
> +# error Cannot use default crti.S because it lacks _init code
> +#endif
> +
> #ifdef PREINIT_FUNCTION
>
> -#if PREINIT_FUNCTION_WEAK
> -# error PREINIT_FUNCTION_WEAK is unsupported
> -#endif
> +# if PREINIT_FUNCTION_WEAK
> +# error PREINIT_FUNCTION_WEAK is unsupported
> +# endif
>
> /* This arranges for PREINIT_FUNCTION to be called upon loading a library that
> contains crti.o. */
> diff --git a/sysdeps/init_array/crtn.S b/sysdeps/generic/crtn.S
> similarity index 82%
> rename from sysdeps/init_array/crtn.S
> rename to sysdeps/generic/crtn.S
> index 6f70e77160..6996e0001d 100644
> --- a/sysdeps/init_array/crtn.S
> +++ b/sysdeps/generic/crtn.S
> @@ -11,3 +11,9 @@
> But new configurations without compatibility concerns for
> toolchains without .init_array support can use this to avoid the
> superfluous .init and .fini boilerplate code. */
> +
> +#include <platform-params.h>
> +
> +#if ELF_INITFINI
> +# error Cannot use genetric crtn.S because it lacks _fini code
> +#endif
> diff --git a/sysdeps/generic/platform-params.h b/sysdeps/generic/platform-params.h
> new file mode 100644
> index 0000000000..5a732fa02f
> --- /dev/null
> +++ b/sysdeps/generic/platform-params.h
> @@ -0,0 +1,32 @@
> +/* Various definitions for specifying platform behavior. Generic version.
> + Copyright (C) 2018 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/>. */
> +
> +/* This file contains definitions specifying behavior of a platform
> + (CPU + kernel) combination. It must be usable from both assembler
> + sources and C sources. Definitions should be namespace-clean. */
> +
Shouldn't we note ABI take in consideration as well?
> +#ifndef _PLATFORM_PARAMS_H
> +#define _PLATFORM_PARAMS_H
> +
> +/* Most platforms use _init/_fini symbols to call constructors and
> + destructors. If defined to 0, the dynamic loader will ignore
> + DT_INIT and DT_FINI tags, and static binaries will not call the
> + _init or _fini functions. */
> +#define ELF_INITFINI 1
I assume for new ABIs init_array is the prefered and the expected way to
initialize constructors. Should we add a comment that new ports are
expected set it as 0?
> +
> +#endif /* _PLATFORM_PARAMS_H */
> diff --git a/sysdeps/init_array/elf-init.c b/sysdeps/init_array/elf-init.c
> deleted file mode 100644
> index 9fbe82e93c..0000000000
> --- a/sysdeps/init_array/elf-init.c
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -/* Startup support for ELF initializers/finalizers in the main executable.
> - Copyright (C) 2013-2018 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.
> -
> - In addition to the permissions in the GNU Lesser General Public
> - License, the Free Software Foundation gives you unlimited
> - permission to link the compiled version of this file with other
> - programs, and to distribute those programs without any restriction
> - coming from the use of this file. (The GNU Lesser General Public
> - License restrictions do apply in other respects; for example, they
> - cover modification of the file, and distribution when not linked
> - into another program.)
> -
> - Note that people who make modified versions of this file are not
> - obligated to grant this special exception for their modified
> - versions; it is their choice whether to do so. The GNU Lesser
> - General Public License gives permission to release a modified
> - version without this exception; this exception also makes it
> - possible to release a modified version which carries forward this
> - exception.
> -
> - 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/>. */
> -
> -#define NO_INITFINI
> -#include <csu/elf-init.c>
> diff --git a/sysdeps/init_array/gmon-start.c b/sysdeps/init_array/gmon-start.c
> deleted file mode 100644
> index 3bacaf7e38..0000000000
> --- a/sysdeps/init_array/gmon-start.c
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/* gmon startup hook using .preinit_array.
> - Copyright (C) 2013-2018 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.
> -
> - In addition to the permissions in the GNU Lesser General Public
> - License, the Free Software Foundation gives you unlimited
> - permission to link the compiled version of this file with other
> - programs, and to distribute those programs without any restriction
> - coming from the use of this file. (The GNU Lesser General Public
> - License restrictions do apply in other respects; for example, they
> - cover modification of the file, and distribution when not linked
> - into another program.)
> -
> - Note that people who make modified versions of this file are not
> - obligated to grant this special exception for their modified
> - versions; it is their choice whether to do so. The GNU Lesser
> - General Public License gives permission to release a modified
> - version without this exception; this exception also makes it
> - possible to release a modified version which carries forward this
> - exception.
> -
> - 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/>. */
> -
> -/* Instead of defining __gmon_start__ globally in gcrt1.o, we make it
> - static and just put a pointer to it into the .preinit_array section. */
> -
> -#define GMON_START_ARRAY_SECTION ".preinit_array"
> -
> -#include <csu/gmon-start.c>
> diff --git a/sysdeps/riscv/Implies b/sysdeps/riscv/Implies
> index c88325b8be..1945b1f4bb 100644
> --- a/sysdeps/riscv/Implies
> +++ b/sysdeps/riscv/Implies
> @@ -1,5 +1,3 @@
> -init_array
> -
> ieee754/ldbl-128
> ieee754/dbl-64
> ieee754/flt-32
> diff --git a/sysdeps/init_array/pt-crti.S b/sysdeps/riscv/platform-params.h
> similarity index 70%
> rename from sysdeps/init_array/pt-crti.S
> rename to sysdeps/riscv/platform-params.h
> index 9bf17059dd..b71591437d 100644
> --- a/sysdeps/init_array/pt-crti.S
> +++ b/sysdeps/riscv/platform-params.h
> @@ -1,5 +1,5 @@
> -/* Special initializer support for libpthread.
> - Copyright (C) 2015-2018 Free Software Foundation, Inc.
> +/* Various definitions for specifying platform behavior. RISC_V version.
> + Copyright (C) 2018 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
> @@ -16,8 +16,10 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -/* This arranges for libpthread.so's special initializer to be called as
> - soon as the library is loaded. */
> +#ifndef _PLATFORM_PARAMS_H
> +#define _PLATFORM_PARAMS_H
>
> - .section .init_array,"a",%init_array
> - .dc.a __pthread_initialize_minimal_internal
> +/* RISC-V does not have .init/.fini. */
> +#define ELF_INITFINI 0
> +
> +#endif /* _PLATFORM_PARAMS_H */
>