diff mbox

Introduce ELF_INITFINI for all architectures

Message ID 5ffaedea-0efb-1800-744b-583fb2faf1c2@redhat.com
State New, archived
Headers show

Commit Message

Florian Weimer Aug. 14, 2018, 10:06 a.m. UTC
On 08/08/2018 10:42 PM, Adhemerval Zanella wrote:

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

Both INIT and INIT_ARRAY are extremely common, perhaps due to the TM 
registration code in GCC.  I've dropped the early return in the attached 
patch.

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

Sorry, ABI in what sense?

>> +#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?

I'm not sure if that's happening with new ports.

Thanks,
Florian

Comments

Florian Weimer Aug. 17, 2018, 12:15 p.m. UTC | #1
On 08/14/2018 12:06 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.

Do we have consensus that this is the way to proceed?  I would like to 
commit this.

Thanks,
Florian
diff mbox

Patch

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-08-14  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): Drop early return because most
	binaries have DT_INIT and DT_INITARRAY.  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.

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..690ba6dd41 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,11 +41,6 @@  call_init (struct link_map *l, int argc, char **argv, char **env)
       && l->l_type == lt_executable)
     return;
 
-  /* Are there any constructors?  */
-  if (l->l_info[DT_INIT] == NULL
-      && __builtin_expect (l->l_info[DT_INIT_ARRAY] == NULL, 1))
-    return;
-
   /* Print a debug message if wanted.  */
   if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS))
     _dl_debug_printf ("\ncalling init: %s\n\n",
@@ -54,7 +50,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 (ELF_INITFINI && l->l_info[DT_INIT] != NULL)
     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.  */
+
+#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
+
+#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 */