diff mbox

[6/7] Delay initialization of CPU features struct in static binaries

Message ID 1494514306-4167-7-git-send-email-siddhesh@sourceware.org
State New
Headers show

Commit Message

Siddhesh Poyarekar May 11, 2017, 2:51 p.m. UTC
Allow the CPU features structure set up to be overridden by tunables
by delaying it to until after tunables are initialized.  The
initialization is already delayed in dynamically linked glibc, it is
only in static binaries that the initialization is set early to allow
it to influence IFUNC relocations that happen in libc-start.  It is a
bit too early however and there is a good place between tunables
initialization and IFUNC relocations where this can be done.

Verified that this does not regress the testsuite.

	* csu/libc-start.c [!ARCH_INIT_CPU_FEATURES]: Define
	ARCH_INIT_CPU_FEATURES.
	(LIBC_START_MAIN): Call it.
	* sysdeps/unix/sysv/linux/aarch64/libc-start.c
	(__libc_start_main): Remove.
	(ARCH_INIT_CPU_FEATURES): New macro.
	* sysdeps/x86/libc-start.c (__libc_start_main): Remove.
	(ARCH_INIT_CPU_FEATURES): New macro.
---
 csu/libc-start.c                             |  6 ++++++
 sysdeps/unix/sysv/linux/aarch64/libc-start.c | 23 +++++------------------
 sysdeps/x86/libc-start.c                     | 23 +++++------------------
 3 files changed, 16 insertions(+), 36 deletions(-)

Comments

Adhemerval Zanella Netto May 17, 2017, 7:20 p.m. UTC | #1
On 11/05/2017 11:51, Siddhesh Poyarekar wrote:
> Allow the CPU features structure set up to be overridden by tunables
> by delaying it to until after tunables are initialized.  The
> initialization is already delayed in dynamically linked glibc, it is
> only in static binaries that the initialization is set early to allow
> it to influence IFUNC relocations that happen in libc-start.  It is a
> bit too early however and there is a good place between tunables
> initialization and IFUNC relocations where this can be done.
> 
> Verified that this does not regress the testsuite.
> 
> 	* csu/libc-start.c [!ARCH_INIT_CPU_FEATURES]: Define
> 	ARCH_INIT_CPU_FEATURES.
> 	(LIBC_START_MAIN): Call it.
> 	* sysdeps/unix/sysv/linux/aarch64/libc-start.c
> 	(__libc_start_main): Remove.
> 	(ARCH_INIT_CPU_FEATURES): New macro.
> 	* sysdeps/x86/libc-start.c (__libc_start_main): Remove.
> 	(ARCH_INIT_CPU_FEATURES): New macro.

LGTM with some nits about indentation.

> ---
>  csu/libc-start.c                             |  6 ++++++
>  sysdeps/unix/sysv/linux/aarch64/libc-start.c | 23 +++++------------------
>  sysdeps/x86/libc-start.c                     | 23 +++++------------------
>  3 files changed, 16 insertions(+), 36 deletions(-)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 9a56dcb..c2dd159 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -104,6 +104,10 @@ apply_irel (void)
>  # define MAIN_AUXVEC_PARAM
>  #endif
>  
> +#ifndef ARCH_INIT_CPU_FEATURES
> +# define ARCH_INIT_CPU_FEATURES()
> +#endif
> +
>  STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
>  					 MAIN_AUXVEC_DECL),
>  			    int argc,
> @@ -182,6 +186,8 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>  
>    __tunables_init (__environ);
>  
> +  ARCH_INIT_CPU_FEATURES ();
> +
>    /* Perform IREL{,A} relocations.  */
>    apply_irel ();
>  
> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> index a5babd4..46041fe 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> @@ -16,26 +16,13 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#ifdef SHARED
> -# include <csu/libc-start.c>
> -# else
> -/* The main work is done in the generic function.  */
> -# define LIBC_START_DISABLE_INLINE
> -# define LIBC_START_MAIN generic_start_main
> -# include <csu/libc-start.c>
> +#ifndef SHARED
> +#include <ldsodefs.h>
>  # include <cpu-features.c>
>  
>  extern struct cpu_features _dl_aarch64_cpu_features;
>  
> -int
> -__libc_start_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> -		   int argc, char **argv,
> -		   __typeof (main) init,
> -		   void (*fini) (void),
> -		   void (*rtld_fini) (void), void *stack_end)
> -{
> -  init_cpu_features (&_dl_aarch64_cpu_features);
> -  return generic_start_main (main, argc, argv, init, fini, rtld_fini,
> -			     stack_end);
> -}
> +#define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_aarch64_cpu_features)
> +
>  #endif
> +# include <csu/libc-start.c>

Resulting indentation seems odd, I think it should be:

#ifndef SHARED
# include <ldsodefs.h>
# include <cpu-features.c>

extern struct cpu_features _dl_aarch64_cpu_features;

# define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_aarch64_cpu_features)

#endif
#include <csu/libc-start.c>

Same for x86.

> diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
> index 9a56adc..e11b490 100644
> --- a/sysdeps/x86/libc-start.c
> +++ b/sysdeps/x86/libc-start.c
> @@ -15,27 +15,14 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#ifdef SHARED
> -# include <csu/libc-start.c>
> -# else
> -/* The main work is done in the generic function.  */
> -# define LIBC_START_DISABLE_INLINE
> -# define LIBC_START_MAIN generic_start_main
> -# include <csu/libc-start.c>
> +#ifndef SHARED
> +#include <ldsodefs.h>
>  # include <cpu-features.h>
>  # include <cpu-features.c>
>  
>  extern struct cpu_features _dl_x86_cpu_features;
>  
> -int
> -__libc_start_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> -		   int argc, char **argv,
> -		   __typeof (main) init,
> -		   void (*fini) (void),
> -		   void (*rtld_fini) (void), void *stack_end)
> -{
> -  init_cpu_features (&_dl_x86_cpu_features);
> -  return generic_start_main (main, argc, argv, init, fini, rtld_fini,
> -			     stack_end);
> -}
> +#define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_x86_cpu_features)
> +
>  #endif
> +# include <csu/libc-start.c>
>
diff mbox

Patch

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 9a56dcb..c2dd159 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -104,6 +104,10 @@  apply_irel (void)
 # define MAIN_AUXVEC_PARAM
 #endif
 
+#ifndef ARCH_INIT_CPU_FEATURES
+# define ARCH_INIT_CPU_FEATURES()
+#endif
+
 STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
 					 MAIN_AUXVEC_DECL),
 			    int argc,
@@ -182,6 +186,8 @@  LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
 
   __tunables_init (__environ);
 
+  ARCH_INIT_CPU_FEATURES ();
+
   /* Perform IREL{,A} relocations.  */
   apply_irel ();
 
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
index a5babd4..46041fe 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
+++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
@@ -16,26 +16,13 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#ifdef SHARED
-# include <csu/libc-start.c>
-# else
-/* The main work is done in the generic function.  */
-# define LIBC_START_DISABLE_INLINE
-# define LIBC_START_MAIN generic_start_main
-# include <csu/libc-start.c>
+#ifndef SHARED
+#include <ldsodefs.h>
 # include <cpu-features.c>
 
 extern struct cpu_features _dl_aarch64_cpu_features;
 
-int
-__libc_start_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
-		   int argc, char **argv,
-		   __typeof (main) init,
-		   void (*fini) (void),
-		   void (*rtld_fini) (void), void *stack_end)
-{
-  init_cpu_features (&_dl_aarch64_cpu_features);
-  return generic_start_main (main, argc, argv, init, fini, rtld_fini,
-			     stack_end);
-}
+#define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_aarch64_cpu_features)
+
 #endif
+# include <csu/libc-start.c>
diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
index 9a56adc..e11b490 100644
--- a/sysdeps/x86/libc-start.c
+++ b/sysdeps/x86/libc-start.c
@@ -15,27 +15,14 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#ifdef SHARED
-# include <csu/libc-start.c>
-# else
-/* The main work is done in the generic function.  */
-# define LIBC_START_DISABLE_INLINE
-# define LIBC_START_MAIN generic_start_main
-# include <csu/libc-start.c>
+#ifndef SHARED
+#include <ldsodefs.h>
 # include <cpu-features.h>
 # include <cpu-features.c>
 
 extern struct cpu_features _dl_x86_cpu_features;
 
-int
-__libc_start_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
-		   int argc, char **argv,
-		   __typeof (main) init,
-		   void (*fini) (void),
-		   void (*rtld_fini) (void), void *stack_end)
-{
-  init_cpu_features (&_dl_x86_cpu_features);
-  return generic_start_main (main, argc, argv, init, fini, rtld_fini,
-			     stack_end);
-}
+#define ARCH_INIT_CPU_FEATURES() init_cpu_features (&_dl_x86_cpu_features)
+
 #endif
+# include <csu/libc-start.c>