[PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

v4:
- added patches from H.J.Lu:
	- x86: fix libmvec tests
	- x86: fix syscalls in libc_enable_secure
	- x86: avoid relative reloc for _dl_sysinfo
	- x86: add ifunc test
- i386 cannot mark all symbols hidden, so use fine grain
  marking in files that participate in early code before
  static pie self relocation.
- the patch that makes all libc symbols hidden is still
  included: it is now only an optimization for non-i386
  targets.

v3:
- refactor tunables: move internals out of dl-tunables.h
- use generated max string length in the tunables list
  instead of magic values.

v2:
- check PI_STATIC_AND_HIDDEN for --enable-static-pie
- change string buffer sizes in the tunables
- fix env_alias == NULL logic in __tunables_init
- move __ehdr_start processing after self relocation


force pushed into nsz/bug27072 branch.

Issues that are not addressed:
- tunables try to allocate memory even with non-suid exe.
  allocation is only needed for rewriting the GLIBC_TUNABLES
  env var. (i think a case can be made that if anything there
  is TUNABLE_SECLEVEL_SXID_ERASE then this env var would be
  simply dropped, that would simplify this significantly).
- __sbrk only needs the hidden visibility magic because of
  tunables, ideally we would not do allocations before self
  relocation.
- tunable list data structure is not optimized for compactness.
- all symbols are forced hidden in libc.a, but i think lib*.a
  should do the same. (other than lib*_nonshared.a)
- i386 introduced a fair bit of complications: may be avoiding
  relative relocs is too much to ask for and relocations should
  be done in two steps after all: relative first, then irelative
  when tunable etc are set up.

H.J. Lu (4):
  libmvec: Add extra-test-objs to test-extras
  elf: Avoid RELATIVE relocation for _dl_sysinfo
  Use <startup.h> in __libc_init_secure
  x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]

Szabolcs Nagy (6):
  configure: Require PI_STATIC_AND_HIDDEN for static pie
  elf: Make the tunable struct definition internal only
  elf: Avoid RELATIVE relocs in __tunables_init
  Use hidden visibility for early static PIE code
  csu: Move static pie self relocation later [BZ #27072]
  Make libc symbols hidden in static PIE

 configure                                    |  14 +++
 configure.ac                                 |   5 +
 csu/libc-start.c                             |  48 +++++---
 elf/dl-reloc-static-pie.c                    |   2 +
 elf/dl-support.c                             |  18 ++-
 elf/dl-tunable-types.h                       |  42 +++++--
 elf/dl-tunables.c                            |   6 +-
 elf/dl-tunables.h                            |  35 ++----
 elf/enbl-secure.c                            |  10 +-
 include/libc-symbols.h                       |   9 +-
 misc/sbrk.c                                  |   4 +
 scripts/gen-tunables.awk                     |  16 ++-
 sysdeps/generic/startup.h                    |  26 ++++
 sysdeps/unix/sysv/linux/aarch64/libc-start.c |   5 +
 sysdeps/unix/sysv/linux/i386/startup.h       |  29 ++++-
 sysdeps/x86/Makefile                         |  14 +++
 sysdeps/x86/libc-start.c                     |   5 +
 sysdeps/x86/tst-ifunc-isa-1-static.c         |   1 +
 sysdeps/x86/tst-ifunc-isa-1.c                | 115 ++++++++++++++++++
 sysdeps/x86/tst-ifunc-isa-2-static.c         |   1 +
 sysdeps/x86/tst-ifunc-isa-2.c                | 119 +++++++++++++++++++
 sysdeps/x86_64/fpu/Makefile                  |   8 ++
 22 files changed, 465 insertions(+), 67 deletions(-)
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c

-- 
2.17.1

[PATCH v4 01/10] configure: Require PI_STATIC_AND_HIDDEN for static pie

[Szabolcs Nagy via Libc-alpha]

The glibc static pie self relocation code relies on that local
symbols can be accessed without dynamic relocations in position
independent code.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
---
 configure    | 14 ++++++++++++++
 configure.ac |  5 +++++
 2 files changed, 19 insertions(+)

diff --git a/configure b/configure
index 49f7b32b52..dfadfdf84d 100755
--- a/configure
+++ b/configure
@@ -6818,6 +6818,20 @@ if test "$static_pie" = yes; then
   if test "$libc_cv_no_dynamic_linker" != yes; then
     as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5
   fi
+
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#ifndef PI_STATIC_AND_HIDDEN
+# error static pie depends on PI_STATIC_AND_HIDDEN
+#endif
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+else
+  as_fn_error $? "the architecture does not support static pie" "$LINENO" 5
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
   # Default to PIE.
   libc_cv_pie_default=yes
   $as_echo "#define ENABLE_STATIC_PIE 1" >>confdefs.h
diff --git a/configure.ac b/configure.ac
index 341d4eeac2..702c98706b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1835,6 +1835,11 @@ if test "$static_pie" = yes; then
   if test "$libc_cv_no_dynamic_linker" != yes; then
     AC_MSG_ERROR([linker support for --no-dynamic-linker needed])
   fi
+
+  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef PI_STATIC_AND_HIDDEN
+# error static pie depends on PI_STATIC_AND_HIDDEN
+#endif]])], , AC_MSG_ERROR([the architecture does not support static pie]))
+
   # Default to PIE.
   libc_cv_pie_default=yes
   AC_DEFINE(ENABLE_STATIC_PIE)
-- 
2.17.1

[PATCH v4 02/10] libmvec: Add extra-test-objs to test-extras

[Szabolcs Nagy via Libc-alpha]

From: "H.J. Lu" <hjl.tools@gmail.com>

Add extra-test-objs to test-extras so that they are compiled with
-DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.
---
 sysdeps/x86_64/fpu/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
index a4ff2723a8..9a4bdd075c 100644
--- a/sysdeps/x86_64/fpu/Makefile
+++ b/sysdeps/x86_64/fpu/Makefile
@@ -31,6 +31,12 @@ libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \
 tests += test-double-libmvec-sincos test-double-libmvec-sincos-avx \
 	 test-double-libmvec-sincos-avx2 test-float-libmvec-sincosf \
 	 test-float-libmvec-sincosf-avx test-float-libmvec-sincosf-avx2
+test-extras += test-double-libmvec-sincos-avx-main \
+		   test-double-libmvec-sincos-avx2-main \
+		   test-double-libmvec-sincos-main \
+		   test-float-libmvec-sincosf-avx-main \
+		   test-float-libmvec-sincosf-avx2-main \
+		   test-float-libmvec-sincosf-main
 extra-test-objs += test-double-libmvec-sincos-avx-main.o \
 		   test-double-libmvec-sincos-avx2-main.o \
 		   test-double-libmvec-sincos-main.o \
@@ -66,6 +72,8 @@ ifeq (yes,$(config-cflags-avx512))
 libmvec-tests += double-vlen8 float-vlen16
 tests += test-double-libmvec-sincos-avx512 \
 	 test-float-libmvec-sincosf-avx512
+test-extras += test-double-libmvec-sincos-avx512-main \
+	       test-float-libmvec-sincosf-avx512-main
 extra-test-objs += test-double-libmvec-sincos-avx512-main.o \
 		   test-float-libmvec-sincosf-avx512-main.o
 
-- 
2.17.1

Re: [PATCH v4 02/10] libmvec: Add extra-test-objs to test-extras

[Adhemerval Zanella via Libc-alpha]

On 18/01/2021 13:23, Szabolcs Nagy via Libc-alpha wrote:
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> Add extra-test-objs to test-extras so that they are compiled with
> -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/x86_64/fpu/Makefile | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
> index a4ff2723a8..9a4bdd075c 100644
> --- a/sysdeps/x86_64/fpu/Makefile
> +++ b/sysdeps/x86_64/fpu/Makefile
> @@ -31,6 +31,12 @@ libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \
>  tests += test-double-libmvec-sincos test-double-libmvec-sincos-avx \
>  	 test-double-libmvec-sincos-avx2 test-float-libmvec-sincosf \
>  	 test-float-libmvec-sincosf-avx test-float-libmvec-sincosf-avx2
> +test-extras += test-double-libmvec-sincos-avx-main \
> +		   test-double-libmvec-sincos-avx2-main \
> +		   test-double-libmvec-sincos-main \
> +		   test-float-libmvec-sincosf-avx-main \
> +		   test-float-libmvec-sincosf-avx2-main \
> +		   test-float-libmvec-sincosf-main
>  extra-test-objs += test-double-libmvec-sincos-avx-main.o \
>  		   test-double-libmvec-sincos-avx2-main.o \
>  		   test-double-libmvec-sincos-main.o \
> @@ -66,6 +72,8 @@ ifeq (yes,$(config-cflags-avx512))
>  libmvec-tests += double-vlen8 float-vlen16
>  tests += test-double-libmvec-sincos-avx512 \
>  	 test-float-libmvec-sincosf-avx512
> +test-extras += test-double-libmvec-sincos-avx512-main \
> +	       test-float-libmvec-sincosf-avx512-main
>  extra-test-objs += test-double-libmvec-sincos-avx512-main.o \
>  		   test-float-libmvec-sincosf-avx512-main.o
>  
> 

[PATCH v4 03/10] elf: Make the tunable struct definition internal only

[Szabolcs Nagy via Libc-alpha]

The representation of the tunables including type information and
the tunable list structure are only used in the implementation not
in the tunables api that is exposed to usage within glibc.

This patch moves the representation related definitions into the
existing dl-tunable-types.h and uses that only for implementation.

The tunable callback and related types are moved to dl-tunables.h
because they are part of the tunables api.

This reduces the details exposed in the tunables api so the internals
are easier to change.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
---
 elf/dl-tunable-types.h   | 42 ++++++++++++++++++++++++++++++----------
 elf/dl-tunables.h        | 35 ++++++++-------------------------
 scripts/gen-tunables.awk |  4 +++-
 3 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index 8f6a383dcc..05d4958e1c 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -1,4 +1,4 @@
-/* Tunable type information.
+/* Internal representation of tunables.
 
    Copyright (C) 2016-2021 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
@@ -18,8 +18,14 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef _TUNABLE_TYPES_H_
-# define _TUNABLE_TYPES_H_
+#define _TUNABLE_TYPES_H_
+
+/* Note: This header is included in the generated dl-tunables-list.h and
+   only used internally in the tunables implementation in dl-tunables.c.  */
+
+#include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 
 typedef enum
 {
@@ -36,14 +42,6 @@ typedef struct
   int64_t max;
 } tunable_type_t;
 
-typedef union
-{
-  int64_t numval;
-  const char *strval;
-} tunable_val_t;
-
-typedef void (*tunable_callback_t) (tunable_val_t *);
-
 /* Security level for tunables.  This decides what to do with individual
    tunables for AT_SECURE binaries.  */
 typedef enum
@@ -58,5 +56,29 @@ typedef enum
   TUNABLE_SECLEVEL_NONE = 2,
 } tunable_seclevel_t;
 
+/* A tunable.  */
+struct _tunable
+{
+  const char *name;			/* Internal name of the tunable.  */
+  tunable_type_t type;			/* Data type of the tunable.  */
+  tunable_val_t val;			/* The value.  */
+  bool initialized;			/* Flag to indicate that the tunable is
+					   initialized.  */
+  tunable_seclevel_t security_level;	/* Specify the security level for the
+					   tunable with respect to AT_SECURE
+					   programs.  See description of
+					   tunable_seclevel_t to see a
+					   description of the values.
+
+					   Note that even if the tunable is
+					   read, it may not get used by the
+					   target module if the value is
+					   considered unsafe.  */
+  /* Compatibility elements.  */
+  const char *env_alias;		/* The compatibility environment
+					   variable name.  */
+};
+
+typedef struct _tunable tunable_t;
 
 #endif
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index dfa16c1977..971376ba8d 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -21,8 +21,6 @@
 #ifndef _TUNABLES_H_
 #define _TUNABLES_H_
 
-#include <stdbool.h>
-
 #if !HAVE_TUNABLES
 static inline void
 __always_inline
@@ -31,34 +29,17 @@ __tunables_init (char **unused __attribute__ ((unused)))
   /* This is optimized out if tunables are not enabled.  */
 }
 #else
-
+# include <stdbool.h>
 # include <stddef.h>
-# include "dl-tunable-types.h"
+# include <stdint.h>
 
-/* A tunable.  */
-struct _tunable
+typedef union
 {
-  const char *name;			/* Internal name of the tunable.  */
-  tunable_type_t type;			/* Data type of the tunable.  */
-  tunable_val_t val;			/* The value.  */
-  bool initialized;			/* Flag to indicate that the tunable is
-					   initialized.  */
-  tunable_seclevel_t security_level;	/* Specify the security level for the
-					   tunable with respect to AT_SECURE
-					   programs.  See description of
-					   tunable_seclevel_t to see a
-					   description of the values.
-
-					   Note that even if the tunable is
-					   read, it may not get used by the
-					   target module if the value is
-					   considered unsafe.  */
-  /* Compatibility elements.  */
-  const char *env_alias;		/* The compatibility environment
-					   variable name.  */
-};
-
-typedef struct _tunable tunable_t;
+  int64_t numval;
+  const char *strval;
+} tunable_val_t;
+
+typedef void (*tunable_callback_t) (tunable_val_t *);
 
 /* Full name for a tunable is top_ns.tunable_ns.id.  */
 # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index 622199061a..cda12ef62e 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -156,8 +156,10 @@ END {
   }
   print "} tunable_id_t;\n"
 
-  # Finally, the tunable list.
   print "\n#ifdef TUNABLES_INTERNAL"
+  # Internal definitions.
+  print "# include \"dl-tunable-types.h\""
+  # Finally, the tunable list.
   print "static tunable_t tunable_list[] attribute_relro = {"
   for (tnm in types) {
     split (tnm, indices, SUBSEP);
-- 
2.17.1

[PATCH v4 04/10] elf: Avoid RELATIVE relocs in __tunables_init

[Szabolcs Nagy via Libc-alpha]

With static pie linking pointers in the tunables list need
RELATIVE relocs since the absolute address is not known at link
time. We want to avoid relocations so the static pie self
relocation can be done after tunables are initialized.

This is a simple fix that embeds the tunable strings into the
tunable list instead of using pointers.  It is possible to have
a more compact representation of tunables with some additional
complexity in the generator and tunable parser logic.  Such
optimization will be useful if the list of tunables grows.

There is still an issue that tunables_strdup allocates and the
failure handling code path is sufficiently complex that it can
easily have RELATIVE relocations.  It is possible to avoid the
early allocation and only change environment variables in a
setuid exe after relocations are processed.  But that is a
bigger change and early failure is fatal anyway so it is not
as critical to fix right away. This is bug 27181.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
---
 elf/dl-tunable-types.h   |  4 ++--
 elf/dl-tunables.c        |  2 +-
 scripts/gen-tunables.awk | 12 +++++++++++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index 05d4958e1c..3fcc0806f5 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -59,7 +59,7 @@ typedef enum
 /* A tunable.  */
 struct _tunable
 {
-  const char *name;			/* Internal name of the tunable.  */
+  const char name[TUNABLE_NAME_MAX];	/* Internal name of the tunable.  */
   tunable_type_t type;			/* Data type of the tunable.  */
   tunable_val_t val;			/* The value.  */
   bool initialized;			/* Flag to indicate that the tunable is
@@ -75,7 +75,7 @@ struct _tunable
 					   target module if the value is
 					   considered unsafe.  */
   /* Compatibility elements.  */
-  const char *env_alias;		/* The compatibility environment
+  const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment
 					   variable name.  */
 };
 
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 33be00e447..e44476f204 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -351,7 +351,7 @@ __tunables_init (char **envp)
 
 	  /* Skip over tunables that have either been set already or should be
 	     skipped.  */
-	  if (cur->initialized || cur->env_alias == NULL)
+	  if (cur->initialized || cur->env_alias[0] == '\0')
 	    continue;
 
 	  const char *name = cur->env_alias;
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index cda12ef62e..fa63e86d1a 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -12,6 +12,8 @@ BEGIN {
   tunable=""
   ns=""
   top_ns=""
+  max_name_len=0
+  max_alias_len=0
 }
 
 # Skip over blank lines and comments.
@@ -57,11 +59,14 @@ $1 == "}" {
       maxvals[top_ns,ns,tunable] = max_of[types[top_ns,ns,tunable]]
     }
     if (!env_alias[top_ns,ns,tunable]) {
-      env_alias[top_ns,ns,tunable] = "NULL"
+      env_alias[top_ns,ns,tunable] = "{0}"
     }
     if (!security_level[top_ns,ns,tunable]) {
       security_level[top_ns,ns,tunable] = "SXID_ERASE"
     }
+    len = length(top_ns"."ns"."tunable)
+    if (len > max_name_len)
+      max_name_len = len
 
     tunable = ""
   }
@@ -109,6 +114,9 @@ $1 == "}" {
   }
   else if (attr == "env_alias") {
     env_alias[top_ns,ns,tunable] = sprintf("\"%s\"", val)
+    len = length(val)
+    if (len > max_alias_len)
+      max_alias_len = len
   }
   else if (attr == "security_level") {
     if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") {
@@ -158,6 +166,8 @@ END {
 
   print "\n#ifdef TUNABLES_INTERNAL"
   # Internal definitions.
+  print "# define TUNABLE_NAME_MAX " (max_name_len + 1)
+  print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1)
   print "# include \"dl-tunable-types.h\""
   # Finally, the tunable list.
   print "static tunable_t tunable_list[] attribute_relro = {"
-- 
2.17.1

[PATCH v4 05/10] Use hidden visibility for early static PIE code

[Szabolcs Nagy via Libc-alpha]

This is necessary to avoid RELATIVE relocations in code that has to run
before static PIE self relocation.  We cannot make all symbols hidden:

On i386, all calls to IFUNC functions must go through PLT and calls to
hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
may not be set up for local calls to hidden IFUNC functions.

Even if we can't make all libc symbols hidden for static PIE on i386, we
must make all symbols used before and by _dl_relocate_static_pie hidden.

This is needed for fixing bug 27072.

Co-authored-by: H.J. Lu <hjl.tools@gmail.com>
---
 csu/libc-start.c                             | 4 ++++
 elf/dl-reloc-static-pie.c                    | 2 ++
 elf/dl-support.c                             | 6 ++++++
 elf/dl-tunables.c                            | 4 ++++
 elf/enbl-secure.c                            | 4 ++++
 misc/sbrk.c                                  | 4 ++++
 sysdeps/unix/sysv/linux/aarch64/libc-start.c | 5 +++++
 sysdeps/x86/libc-start.c                     | 5 +++++
 8 files changed, 34 insertions(+)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index db859c3bed..1e90dcb0a7 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -15,6 +15,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
index a8d964061e..d5bd2f31e9 100644
--- a/elf/dl-reloc-static-pie.c
+++ b/elf/dl-reloc-static-pie.c
@@ -17,6 +17,8 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if ENABLE_STATIC_PIE
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+# pragma GCC visibility push(hidden)
 #include <unistd.h>
 #include <ldsodefs.h>
 #include "dynamic-link.h"
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 9d468d5a4b..384080dd80 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -19,6 +19,12 @@
 /* This file defines some things that for the dynamic linker are defined in
    rtld.c and dl-sysdep.c in ways appropriate to bootstrap dynamic linking.  */
 
+#include <string.h>
+/* Mark symbols hidden in static PIE for early self relocation to work.
+   Note: string.h may have ifuncs which cannot be hidden on i686.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <errno.h>
 #include <libintl.h>
 #include <stdlib.h>
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index e44476f204..b1a50b8469 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -18,6 +18,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <startup.h>
 #include <stdint.h>
 #include <stdbool.h>
diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
index bc8c5e96d2..ffd7938605 100644
--- a/elf/enbl-secure.c
+++ b/elf/enbl-secure.c
@@ -19,6 +19,10 @@
 /* This file is used in the static libc.  For the shared library,
    dl-sysdep.c defines and initializes __libc_enable_secure.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <unistd.h>
 #include <libc-internal.h>
 
diff --git a/misc/sbrk.c b/misc/sbrk.c
index 99b3fb517e..95800b32aa 100644
--- a/misc/sbrk.c
+++ b/misc/sbrk.c
@@ -15,6 +15,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <errno.h>
 #include <libc-internal.h>
 #include <stdbool.h>
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
index f816f04ee1..e1604a6ed0 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
+++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
@@ -17,6 +17,11 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef SHARED
+
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+# if BUILD_PIE_DEFAULT
+#  pragma GCC visibility push(hidden)
+# endif
 # include <ldsodefs.h>
 # include <cpu-features.c>
 
diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
index 4bbd7d555b..d30aec2aa1 100644
--- a/sysdeps/x86/libc-start.c
+++ b/sysdeps/x86/libc-start.c
@@ -16,6 +16,11 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef SHARED
+
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+# if BUILD_PIE_DEFAULT
+#  pragma GCC visibility push(hidden)
+# endif
 /* Define I386_USE_SYSENTER to support syscall during startup in static
    PIE.  */
 # include <startup.h>
-- 
2.17.1

Re: [PATCH v4 05/10] Use hidden visibility for early static PIE code

[Adhemerval Zanella via Libc-alpha]

On 18/01/2021 13:24, Szabolcs Nagy via Libc-alpha wrote:
> This is necessary to avoid RELATIVE relocations in code that has to run
> before static PIE self relocation.  We cannot make all symbols hidden:
> 
> On i386, all calls to IFUNC functions must go through PLT and calls to
> hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
> may not be set up for local calls to hidden IFUNC functions.
> 
> Even if we can't make all libc symbols hidden for static PIE on i386, we
> must make all symbols used before and by _dl_relocate_static_pie hidden.
> 
> This is needed for fixing bug 27072.
> 
> Co-authored-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  csu/libc-start.c                             | 4 ++++
>  elf/dl-reloc-static-pie.c                    | 2 ++
>  elf/dl-support.c                             | 6 ++++++
>  elf/dl-tunables.c                            | 4 ++++
>  elf/enbl-secure.c                            | 4 ++++
>  misc/sbrk.c                                  | 4 ++++
>  sysdeps/unix/sysv/linux/aarch64/libc-start.c | 5 +++++
>  sysdeps/x86/libc-start.c                     | 5 +++++
>  8 files changed, 34 insertions(+)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index db859c3bed..1e90dcb0a7 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -15,6 +15,10 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <assert.h>
>  #include <stdlib.h>
>  #include <stdio.h>

Ok.

> diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> index a8d964061e..d5bd2f31e9 100644
> --- a/elf/dl-reloc-static-pie.c
> +++ b/elf/dl-reloc-static-pie.c
> @@ -17,6 +17,8 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #if ENABLE_STATIC_PIE
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +# pragma GCC visibility push(hidden)
>  #include <unistd.h>
>  #include <ldsodefs.h>
>  #include "dynamic-link.h"

Ok.

> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index 9d468d5a4b..384080dd80 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -19,6 +19,12 @@
>  /* This file defines some things that for the dynamic linker are defined in
>     rtld.c and dl-sysdep.c in ways appropriate to bootstrap dynamic linking.  */
>  
> +#include <string.h>
> +/* Mark symbols hidden in static PIE for early self relocation to work.
> +   Note: string.h may have ifuncs which cannot be hidden on i686.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <errno.h>
>  #include <libintl.h>
>  #include <stdlib.h>

Ok.

> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index e44476f204..b1a50b8469 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -18,6 +18,10 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <startup.h>
>  #include <stdint.h>
>  #include <stdbool.h>

Ok.

> diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
> index bc8c5e96d2..ffd7938605 100644
> --- a/elf/enbl-secure.c
> +++ b/elf/enbl-secure.c
> @@ -19,6 +19,10 @@
>  /* This file is used in the static libc.  For the shared library,
>     dl-sysdep.c defines and initializes __libc_enable_secure.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <unistd.h>
>  #include <libc-internal.h>
>  

Ok.

> diff --git a/misc/sbrk.c b/misc/sbrk.c
> index 99b3fb517e..95800b32aa 100644
> --- a/misc/sbrk.c
> +++ b/misc/sbrk.c
> @@ -15,6 +15,10 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <errno.h>
>  #include <libc-internal.h>
>  #include <stdbool.h>

Ok.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> index f816f04ee1..e1604a6ed0 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> @@ -17,6 +17,11 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef SHARED
> +
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +# if BUILD_PIE_DEFAULT
> +#  pragma GCC visibility push(hidden)
> +# endif
>  # include <ldsodefs.h>
>  # include <cpu-features.c>
>  

Ok, but wouldn't be possible to focus the pragma on
csu/libc-start.c by moving the includes on generic implementation?

> diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
> index 4bbd7d555b..d30aec2aa1 100644
> --- a/sysdeps/x86/libc-start.c
> +++ b/sysdeps/x86/libc-start.c
> @@ -16,6 +16,11 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef SHARED
> +
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +# if BUILD_PIE_DEFAULT
> +#  pragma GCC visibility push(hidden)
> +# endif
>  /* Define I386_USE_SYSENTER to support syscall during startup in static
>     PIE.  */
>  # include <startup.h>
> 

Ditto.

[PATCH v4 06/10] elf: Avoid RELATIVE relocation for _dl_sysinfo

[Szabolcs Nagy via Libc-alpha]

From: "H.J. Lu" <hjl.tools@gmail.com>

In static PIE, set the default _dl_sysinfo in _dl_aux_init, instead of
using the RELATIVE relocation to intialize it.

This is needed for fixing bug 27072 on x86.
---
 elf/dl-support.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/elf/dl-support.c b/elf/dl-support.c
index 384080dd80..5acd59290f 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -200,7 +200,12 @@ struct dl_scope_free_list *_dl_scope_free_list;
 
 #ifdef NEED_DL_SYSINFO
 /* Needed for improved syscall handling on at least x86/Linux.  */
-uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
+uintptr_t _dl_sysinfo
+/* NB: Avoid RELATIVE relocation in static PIE.  */
+# ifndef BUILD_PIE_DEFAULT
+  = DL_SYSINFO_DEFAULT
+# endif
+;
 #endif
 #ifdef NEED_DL_SYSINFO_DSO
 /* Address of the ELF headers in the vsyscall page.  */
@@ -238,6 +243,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
   uid_t uid = 0;
   gid_t gid = 0;
 
+#if defined NEED_DL_SYSINFO && BUILD_PIE_DEFAULT
+  /* NB: Avoid RELATIVE relocation in static PIE.  */
+  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
+#endif
+
   _dl_auxv = av;
   for (; av->a_type != AT_NULL; ++av)
     switch (av->a_type)
-- 
2.17.1

Re: [PATCH v4 06/10] elf: Avoid RELATIVE relocation for _dl_sysinfo

[Adhemerval Zanella via Libc-alpha]

On 18/01/2021 13:24, Szabolcs Nagy via Libc-alpha wrote:
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> In static PIE, set the default _dl_sysinfo in _dl_aux_init, instead of
> using the RELATIVE relocation to intialize it.
> 
> This is needed for fixing bug 27072 on x86.
> ---
>  elf/dl-support.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index 384080dd80..5acd59290f 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -200,7 +200,12 @@ struct dl_scope_free_list *_dl_scope_free_list;
>  
>  #ifdef NEED_DL_SYSINFO
>  /* Needed for improved syscall handling on at least x86/Linux.  */
> -uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> +uintptr_t _dl_sysinfo
> +/* NB: Avoid RELATIVE relocation in static PIE.  */
> +# ifndef BUILD_PIE_DEFAULT
> +  = DL_SYSINFO_DEFAULT
> +# endif
> +;
>  #endif
>  #ifdef NEED_DL_SYSINFO_DSO
>  /* Address of the ELF headers in the vsyscall page.  */
> @@ -238,6 +243,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
>    uid_t uid = 0;
>    gid_t gid = 0;
>  
> +#if defined NEED_DL_SYSINFO && BUILD_PIE_DEFAULT
> +  /* NB: Avoid RELATIVE relocation in static PIE.  */
> +  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
> +#endif
> +

Couldn't we make it the default instead? 

>    _dl_auxv = av;
>    for (; av->a_type != AT_NULL; ++av)
>      switch (av->a_type)
> 

V2 [PATCH v4 06/10] elf: Avoid RELATIVE relocation for _dl_sysinfo

[H.J. Lu via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Tue, Jan 19, 2021 at 5:51 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 18/01/2021 13:24, Szabolcs Nagy via Libc-alpha wrote:
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> >
> > In static PIE, set the default _dl_sysinfo in _dl_aux_init, instead of
> > using the RELATIVE relocation to intialize it.
> >
> > This is needed for fixing bug 27072 on x86.
> > ---
> >  elf/dl-support.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/elf/dl-support.c b/elf/dl-support.c
> > index 384080dd80..5acd59290f 100644
> > --- a/elf/dl-support.c
> > +++ b/elf/dl-support.c
> > @@ -200,7 +200,12 @@ struct dl_scope_free_list *_dl_scope_free_list;
> >
> >  #ifdef NEED_DL_SYSINFO
> >  /* Needed for improved syscall handling on at least x86/Linux.  */
> > -uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> > +uintptr_t _dl_sysinfo
> > +/* NB: Avoid RELATIVE relocation in static PIE.  */
> > +# ifndef BUILD_PIE_DEFAULT
> > +  = DL_SYSINFO_DEFAULT
> > +# endif
> > +;
> >  #endif
> >  #ifdef NEED_DL_SYSINFO_DSO
> >  /* Address of the ELF headers in the vsyscall page.  */
> > @@ -238,6 +243,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
> >    uid_t uid = 0;
> >    gid_t gid = 0;
> >
> > +#if defined NEED_DL_SYSINFO && BUILD_PIE_DEFAULT
> > +  /* NB: Avoid RELATIVE relocation in static PIE.  */
> > +  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
> > +#endif
> > +
>
> Couldn't we make it the default instead?
>
> >    _dl_auxv = av;
> >    for (; av->a_type != AT_NULL; ++av)
> >      switch (av->a_type)
> >

Like this?

-- 
H.J.

[-- Attachment #2: 0001-elf-Avoid-RELATIVE-relocation-for-_dl_sysinfo.patch --]
[-- Type: text/x-patch, Size: 1378 bytes --]

From af3c44826b8ff07082eb5020213652e6b1fadfa4 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 18 Jan 2021 11:45:46 +0000
Subject: [PATCH] elf: Avoid RELATIVE relocation for _dl_sysinfo

Set the default _dl_sysinfo in _dl_aux_init to avoid RELATIVE relocation
in static PIE.

This is needed for fixing bug 27072 on x86.
---
 elf/dl-support.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/elf/dl-support.c b/elf/dl-support.c
index 384080dd80..7abb65d8e3 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -199,8 +199,9 @@ int _dl_thread_gscope_count;
 struct dl_scope_free_list *_dl_scope_free_list;
 
 #ifdef NEED_DL_SYSINFO
-/* Needed for improved syscall handling on at least x86/Linux.  */
-uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
+/* Needed for improved syscall handling on at least x86/Linux.  NB: Don't
+   initialize it here to avoid RELATIVE relocation in static PIE.  */
+uintptr_t _dl_sysinfo;
 #endif
 #ifdef NEED_DL_SYSINFO_DSO
 /* Address of the ELF headers in the vsyscall page.  */
@@ -238,6 +239,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
   uid_t uid = 0;
   gid_t gid = 0;
 
+#ifdef NEED_DL_SYSINFO
+  /* NB: Avoid RELATIVE relocation in static PIE.  */
+  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
+#endif
+
   _dl_auxv = av;
   for (; av->a_type != AT_NULL; ++av)
     switch (av->a_type)
-- 
2.29.2

Re: V2 [PATCH v4 06/10] elf: Avoid RELATIVE relocation for _dl_sysinfo

[Adhemerval Zanella via Libc-alpha]

On 19/01/2021 11:25, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 5:51 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 18/01/2021 13:24, Szabolcs Nagy via Libc-alpha wrote:
>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>>
>>> In static PIE, set the default _dl_sysinfo in _dl_aux_init, instead of
>>> using the RELATIVE relocation to intialize it.
>>>
>>> This is needed for fixing bug 27072 on x86.
>>> ---
>>>  elf/dl-support.c | 12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/elf/dl-support.c b/elf/dl-support.c
>>> index 384080dd80..5acd59290f 100644
>>> --- a/elf/dl-support.c
>>> +++ b/elf/dl-support.c
>>> @@ -200,7 +200,12 @@ struct dl_scope_free_list *_dl_scope_free_list;
>>>
>>>  #ifdef NEED_DL_SYSINFO
>>>  /* Needed for improved syscall handling on at least x86/Linux.  */
>>> -uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
>>> +uintptr_t _dl_sysinfo
>>> +/* NB: Avoid RELATIVE relocation in static PIE.  */
>>> +# ifndef BUILD_PIE_DEFAULT
>>> +  = DL_SYSINFO_DEFAULT
>>> +# endif
>>> +;
>>>  #endif
>>>  #ifdef NEED_DL_SYSINFO_DSO
>>>  /* Address of the ELF headers in the vsyscall page.  */
>>> @@ -238,6 +243,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
>>>    uid_t uid = 0;
>>>    gid_t gid = 0;
>>>
>>> +#if defined NEED_DL_SYSINFO && BUILD_PIE_DEFAULT
>>> +  /* NB: Avoid RELATIVE relocation in static PIE.  */
>>> +  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
>>> +#endif
>>> +
>>
>> Couldn't we make it the default instead?
>>
>>>    _dl_auxv = av;
>>>    for (; av->a_type != AT_NULL; ++av)
>>>      switch (av->a_type)
>>>
> 
> Like this?
> 

> From af3c44826b8ff07082eb5020213652e6b1fadfa4 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Mon, 18 Jan 2021 11:45:46 +0000
> Subject: [PATCH] elf: Avoid RELATIVE relocation for _dl_sysinfo
> 
> Set the default _dl_sysinfo in _dl_aux_init to avoid RELATIVE relocation
> in static PIE.
> 
> This is needed for fixing bug 27072 on x86.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-support.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index 384080dd80..7abb65d8e3 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -199,8 +199,9 @@ int _dl_thread_gscope_count;
>  struct dl_scope_free_list *_dl_scope_free_list;
>  
>  #ifdef NEED_DL_SYSINFO
> -/* Needed for improved syscall handling on at least x86/Linux.  */
> -uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> +/* Needed for improved syscall handling on at least x86/Linux.  NB: Don't
> +   initialize it here to avoid RELATIVE relocation in static PIE.  */
> +uintptr_t _dl_sysinfo;
>  #endif
>  #ifdef NEED_DL_SYSINFO_DSO
>  /* Address of the ELF headers in the vsyscall page.  */
> @@ -238,6 +239,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
>    uid_t uid = 0;
>    gid_t gid = 0;
>  
> +#ifdef NEED_DL_SYSINFO
> +  /* NB: Avoid RELATIVE relocation in static PIE.  */
> +  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
> +#endif
> +
>    _dl_auxv = av;
>    for (; av->a_type != AT_NULL; ++av)
>      switch (av->a_type)
> -- 
> 2.29.2

[PATCH v4 07/10] Use <startup.h> in __libc_init_secure

[Szabolcs Nagy via Libc-alpha]

From: "H.J. Lu" <hjl.tools@gmail.com>

Since __libc_init_secure is called before ARCH_SETUP_TLS, it must use
"int $0x80" for system calls in i386 static PIE.  Add startup_getuid,
startup_geteuid, startup_getgid and startup_getegid to <startup.h>.
Update __libc_init_secure to use them.
---
 elf/enbl-secure.c                      |  6 +++---
 sysdeps/generic/startup.h              | 26 +++++++++++++++++++++++
 sysdeps/unix/sysv/linux/i386/startup.h | 29 ++++++++++++++++++++++++--
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
index ffd7938605..9e47526bd3 100644
--- a/elf/enbl-secure.c
+++ b/elf/enbl-secure.c
@@ -23,7 +23,7 @@
 #if BUILD_PIE_DEFAULT
 # pragma GCC visibility push(hidden)
 #endif
-#include <unistd.h>
+#include <startup.h>
 #include <libc-internal.h>
 
 /* If nonzero __libc_enable_secure is already set.  */
@@ -35,6 +35,6 @@ void
 __libc_init_secure (void)
 {
   if (__libc_enable_secure_decided == 0)
-    __libc_enable_secure = (__geteuid () != __getuid ()
-			    || __getegid () != __getgid ());
+    __libc_enable_secure = (startup_geteuid () != startup_getuid ()
+			    || startup_getegid () != startup_getgid ());
 }
diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h
index 56c899a65e..04f20cde47 100644
--- a/sysdeps/generic/startup.h
+++ b/sysdeps/generic/startup.h
@@ -19,5 +19,31 @@
 /* Targets should override this file if the default definitions below
    will not work correctly very early before TLS is initialized.  */
 
+#include <unistd.h>
+
 /* Use macro instead of inline function to avoid including <stdio.h>.  */
 #define _startup_fatal(message) __libc_fatal ((message))
+
+static inline uid_t
+startup_getuid (void)
+{
+  return __getuid ();
+}
+
+static inline uid_t
+startup_geteuid (void)
+{
+  return __geteuid ();
+}
+
+static inline gid_t
+startup_getgid (void)
+{
+  return __getgid ();
+}
+
+static inline gid_t
+startup_getegid (void)
+{
+  return __getegid ();
+}
diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h
index 3eb4cc43a2..dee7a4f1d3 100644
--- a/sysdeps/unix/sysv/linux/i386/startup.h
+++ b/sysdeps/unix/sysv/linux/i386/startup.h
@@ -17,11 +17,12 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if BUILD_PIE_DEFAULT
-# include <abort-instr.h>
-
 /* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
 # define I386_USE_SYSENTER 0
 
+# include <sysdep.h>
+# include <abort-instr.h>
+
 __attribute__ ((__noreturn__))
 static inline void
 _startup_fatal (const char *message __attribute__ ((unused)))
@@ -31,6 +32,30 @@ _startup_fatal (const char *message __attribute__ ((unused)))
   ABORT_INSTRUCTION;
   __builtin_unreachable ();
 }
+
+static inline uid_t
+startup_getuid (void)
+{
+  return (uid_t) INTERNAL_SYSCALL_CALL (getuid32);
+}
+
+static inline uid_t
+startup_geteuid (void)
+{
+  return (uid_t) INTERNAL_SYSCALL_CALL (geteuid32);
+}
+
+static inline gid_t
+startup_getgid (void)
+{
+  return (gid_t) INTERNAL_SYSCALL_CALL (getgid32);
+}
+
+static inline gid_t
+startup_getegid (void)
+{
+  return (gid_t) INTERNAL_SYSCALL_CALL (getegid32);
+}
 #else
 # include_next <startup.h>
 #endif
-- 
2.17.1

Re: [PATCH v4 07/10] Use <startup.h> in __libc_init_secure

[Adhemerval Zanella via Libc-alpha]

On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> Since __libc_init_secure is called before ARCH_SETUP_TLS, it must use
> "int $0x80" for system calls in i386 static PIE.  Add startup_getuid,
> startup_geteuid, startup_getgid and startup_getegid to <startup.h>.
> Update __libc_init_secure to use them.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/enbl-secure.c                      |  6 +++---
>  sysdeps/generic/startup.h              | 26 +++++++++++++++++++++++
>  sysdeps/unix/sysv/linux/i386/startup.h | 29 ++++++++++++++++++++++++--
>  3 files changed, 56 insertions(+), 5 deletions(-)
> 
> diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
> index ffd7938605..9e47526bd3 100644
> --- a/elf/enbl-secure.c
> +++ b/elf/enbl-secure.c
> @@ -23,7 +23,7 @@
>  #if BUILD_PIE_DEFAULT
>  # pragma GCC visibility push(hidden)
>  #endif
> -#include <unistd.h>
> +#include <startup.h>
>  #include <libc-internal.h>
>  
>  /* If nonzero __libc_enable_secure is already set.  */
> @@ -35,6 +35,6 @@ void
>  __libc_init_secure (void)
>  {
>    if (__libc_enable_secure_decided == 0)
> -    __libc_enable_secure = (__geteuid () != __getuid ()
> -			    || __getegid () != __getgid ());
> +    __libc_enable_secure = (startup_geteuid () != startup_getuid ()
> +			    || startup_getegid () != startup_getgid ());
>  }

Ok.

> diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h
> index 56c899a65e..04f20cde47 100644
> --- a/sysdeps/generic/startup.h
> +++ b/sysdeps/generic/startup.h
> @@ -19,5 +19,31 @@
>  /* Targets should override this file if the default definitions below
>     will not work correctly very early before TLS is initialized.  */
>  
> +#include <unistd.h>
> +
>  /* Use macro instead of inline function to avoid including <stdio.h>.  */
>  #define _startup_fatal(message) __libc_fatal ((message))
> +
> +static inline uid_t
> +startup_getuid (void)
> +{
> +  return __getuid ();
> +}
> +
> +static inline uid_t
> +startup_geteuid (void)
> +{
> +  return __geteuid ();
> +}
> +
> +static inline gid_t
> +startup_getgid (void)
> +{
> +  return __getgid ();
> +}
> +
> +static inline gid_t
> +startup_getegid (void)
> +{
> +  return __getegid ();
> +}

Ok.

> diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h
> index 3eb4cc43a2..dee7a4f1d3 100644
> --- a/sysdeps/unix/sysv/linux/i386/startup.h
> +++ b/sysdeps/unix/sysv/linux/i386/startup.h
> @@ -17,11 +17,12 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #if BUILD_PIE_DEFAULT
> -# include <abort-instr.h>
> -
>  /* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
>  # define I386_USE_SYSENTER 0
>  
> +# include <sysdep.h>
> +# include <abort-instr.h>
> +
>  __attribute__ ((__noreturn__))
>  static inline void
>  _startup_fatal (const char *message __attribute__ ((unused)))
> @@ -31,6 +32,30 @@ _startup_fatal (const char *message __attribute__ ((unused)))
>    ABORT_INSTRUCTION;
>    __builtin_unreachable ();
>  }
> +
> +static inline uid_t
> +startup_getuid (void)
> +{
> +  return (uid_t) INTERNAL_SYSCALL_CALL (getuid32);
> +}
> +
> +static inline uid_t
> +startup_geteuid (void)
> +{
> +  return (uid_t) INTERNAL_SYSCALL_CALL (geteuid32);
> +}
> +
> +static inline gid_t
> +startup_getgid (void)
> +{
> +  return (gid_t) INTERNAL_SYSCALL_CALL (getgid32);
> +}
> +
> +static inline gid_t
> +startup_getegid (void)
> +{
> +  return (gid_t) INTERNAL_SYSCALL_CALL (getegid32);
> +}
>  #else
>  # include_next <startup.h>
>  #endif
> 

Ok.

[PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

IFUNC resolvers may depend on tunables and cpu feature setup so
move static pie self relocation after those.

It is hard to guarantee that the ealy startup code does not rely
on relocations so this is a bit fragile. It would be more robust
to handle RELATIVE relocs early and only IRELATIVE relocs later,
but the current relocation processing code cannot do that.

The early startup code before relocation processing includes

  _dl_aux_init (auxvec);
  __libc_init_secure ();
  __tunables_init (__environ);
  ARCH_INIT_CPU_FEATURES ();

These are simple enough that RELATIVE relocs can be avoided.

__ehdr_start may require RELATIVE relocation so it was moved
later, fortunately ehdr and phdr are not used in the early code.

Fixes bug 27072.
---
 csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 1e90dcb0a7..c2b59431a3 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -146,8 +146,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
   int result;
 
 #ifndef SHARED
-  _dl_relocate_static_pie ();
-
   char **ev = &argv[argc + 1];
 
   __environ = ev;
@@ -169,24 +167,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
   }
 #  endif
   _dl_aux_init (auxvec);
-  if (GL(dl_phdr) == NULL)
 # endif
-    {
-      /* Starting from binutils-2.23, the linker will define the
-         magic symbol __ehdr_start to point to our own ELF header
-         if it is visible in a segment that also includes the phdrs.
-         So we can set up _dl_phdr and _dl_phnum even without any
-         information from auxv.  */
-
-      extern const ElfW(Ehdr) __ehdr_start
-	__attribute__ ((weak, visibility ("hidden")));
-      if (&__ehdr_start != NULL)
-        {
-          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
-          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
-          GL(dl_phnum) = __ehdr_start.e_phnum;
-        }
-    }
 
   /* Initialize very early so that tunables can use it.  */
   __libc_init_secure ();
@@ -195,6 +176,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
 
   ARCH_INIT_CPU_FEATURES ();
 
+  /* Do static pie self relocation after tunables and cpu features
+     are setup for ifunc resolvers. Before this point relocations
+     must be avoided.  */
+  _dl_relocate_static_pie ();
+
   /* Perform IREL{,A} relocations.  */
   ARCH_SETUP_IREL ();
 
@@ -206,6 +192,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
      hwcap and platform fields available in the TCB.  */
   ARCH_APPLY_IREL ();
 
+# ifdef HAVE_AUX_VECTOR
+  if (GL(dl_phdr) == NULL)
+# endif
+    {
+      /* Starting from binutils-2.23, the linker will define the
+         magic symbol __ehdr_start to point to our own ELF header
+         if it is visible in a segment that also includes the phdrs.
+         So we can set up _dl_phdr and _dl_phnum even without any
+         information from auxv.  */
+
+      extern const ElfW(Ehdr) __ehdr_start
+	__attribute__ ((weak, visibility ("hidden")));
+      if (&__ehdr_start != NULL)
+        {
+          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
+          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
+          GL(dl_phnum) = __ehdr_start.e_phnum;
+        }
+    }
+
   /* Set up the stack checker's canary.  */
   uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
 # ifdef THREAD_SET_STACK_GUARD
-- 
2.17.1

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> IFUNC resolvers may depend on tunables and cpu feature setup so
> move static pie self relocation after those.
> 
> It is hard to guarantee that the ealy startup code does not rely
> on relocations so this is a bit fragile. It would be more robust
> to handle RELATIVE relocs early and only IRELATIVE relocs later,
> but the current relocation processing code cannot do that.
> 
> The early startup code before relocation processing includes
> 
>   _dl_aux_init (auxvec);
>   __libc_init_secure ();
>   __tunables_init (__environ);
>   ARCH_INIT_CPU_FEATURES ();
> 
> These are simple enough that RELATIVE relocs can be avoided.
> 
> __ehdr_start may require RELATIVE relocation so it was moved
> later, fortunately ehdr and phdr are not used in the early code.
> 
> Fixes bug 27072.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
>  1 file changed, 25 insertions(+), 19 deletions(-)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 1e90dcb0a7..c2b59431a3 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -146,8 +146,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    int result;
>  
>  #ifndef SHARED
> -  _dl_relocate_static_pie ();
> -
>    char **ev = &argv[argc + 1];
>  
>    __environ = ev;
Ok.


> @@ -169,24 +167,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    }
>  #  endif
>    _dl_aux_init (auxvec);
> -  if (GL(dl_phdr) == NULL)
>  # endif
> -    {
> -      /* Starting from binutils-2.23, the linker will define the
> -         magic symbol __ehdr_start to point to our own ELF header
> -         if it is visible in a segment that also includes the phdrs.
> -         So we can set up _dl_phdr and _dl_phnum even without any
> -         information from auxv.  */
> -
> -      extern const ElfW(Ehdr) __ehdr_start
> -	__attribute__ ((weak, visibility ("hidden")));
> -      if (&__ehdr_start != NULL)
> -        {
> -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> -          GL(dl_phnum) = __ehdr_start.e_phnum;
> -        }
> -    }
>  
>    /* Initialize very early so that tunables can use it.  */
>    __libc_init_secure ();

Ok.

> @@ -195,6 +176,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>  
>    ARCH_INIT_CPU_FEATURES ();
>  
> +  /* Do static pie self relocation after tunables and cpu features
> +     are setup for ifunc resolvers. Before this point relocations
> +     must be avoided.  */
> +  _dl_relocate_static_pie ();
> +
>    /* Perform IREL{,A} relocations.  */
>    ARCH_SETUP_IREL ();
>  

Ok.

> @@ -206,6 +192,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>       hwcap and platform fields available in the TCB.  */
>    ARCH_APPLY_IREL ();
>  
> +# ifdef HAVE_AUX_VECTOR
> +  if (GL(dl_phdr) == NULL)
> +# endif
> +    {
> +      /* Starting from binutils-2.23, the linker will define the
> +         magic symbol __ehdr_start to point to our own ELF header
> +         if it is visible in a segment that also includes the phdrs.
> +         So we can set up _dl_phdr and _dl_phnum even without any
> +         information from auxv.  */
> +
> +      extern const ElfW(Ehdr) __ehdr_start
> +	__attribute__ ((weak, visibility ("hidden")));
> +      if (&__ehdr_start != NULL)
> +        {
> +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> +          GL(dl_phnum) = __ehdr_start.e_phnum;
> +        }
> +    }
> +
>    /* Set up the stack checker's canary.  */
>    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
>  # ifdef THREAD_SET_STACK_GUARD
> 

Ok.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/19/2021 11:07, Adhemerval Zanella wrote:
> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> > IFUNC resolvers may depend on tunables and cpu feature setup so
> > move static pie self relocation after those.
> > 
> > It is hard to guarantee that the ealy startup code does not rely
> > on relocations so this is a bit fragile. It would be more robust
> > to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > but the current relocation processing code cannot do that.
> > 
> > The early startup code before relocation processing includes
> > 
> >   _dl_aux_init (auxvec);
> >   __libc_init_secure ();
> >   __tunables_init (__environ);
> >   ARCH_INIT_CPU_FEATURES ();
> > 
> > These are simple enough that RELATIVE relocs can be avoided.
> > 
> > __ehdr_start may require RELATIVE relocation so it was moved
> > later, fortunately ehdr and phdr are not used in the early code.
> > 
> > Fixes bug 27072.
> 
> LGTM, thanks.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>


sigh, this is an old version of this patch, i made a
mistake putting the series together.

the problem is that _dl_phdr is used in ARCH_SETUP_TLS
(to get the tls program headers) so the __ehdr_start
magic should be before that (this only matters if auxv
lacks AT_PHDR for some reason, which should not happen
normally on linux, so testing won't show the problem)

i'm trying to figure out where i lost the new version..

thanks for the reviews.

> 
> > ---
> >  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
> >  1 file changed, 25 insertions(+), 19 deletions(-)
> > 
> > diff --git a/csu/libc-start.c b/csu/libc-start.c
> > index 1e90dcb0a7..c2b59431a3 100644
> > --- a/csu/libc-start.c
> > +++ b/csu/libc-start.c
> > @@ -146,8 +146,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >    int result;
> >  
> >  #ifndef SHARED
> > -  _dl_relocate_static_pie ();
> > -
> >    char **ev = &argv[argc + 1];
> >  
> >    __environ = ev;
> Ok.
> 
> 
> > @@ -169,24 +167,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >    }
> >  #  endif
> >    _dl_aux_init (auxvec);
> > -  if (GL(dl_phdr) == NULL)
> >  # endif
> > -    {
> > -      /* Starting from binutils-2.23, the linker will define the
> > -         magic symbol __ehdr_start to point to our own ELF header
> > -         if it is visible in a segment that also includes the phdrs.
> > -         So we can set up _dl_phdr and _dl_phnum even without any
> > -         information from auxv.  */
> > -
> > -      extern const ElfW(Ehdr) __ehdr_start
> > -	__attribute__ ((weak, visibility ("hidden")));
> > -      if (&__ehdr_start != NULL)
> > -        {
> > -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > -          GL(dl_phnum) = __ehdr_start.e_phnum;
> > -        }
> > -    }
> >  
> >    /* Initialize very early so that tunables can use it.  */
> >    __libc_init_secure ();
> 
> Ok.
> 
> > @@ -195,6 +176,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >  
> >    ARCH_INIT_CPU_FEATURES ();
> >  
> > +  /* Do static pie self relocation after tunables and cpu features
> > +     are setup for ifunc resolvers. Before this point relocations
> > +     must be avoided.  */
> > +  _dl_relocate_static_pie ();
> > +
> >    /* Perform IREL{,A} relocations.  */
> >    ARCH_SETUP_IREL ();
> >  
> 
> Ok.
> 
> > @@ -206,6 +192,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >       hwcap and platform fields available in the TCB.  */
> >    ARCH_APPLY_IREL ();
> >  
> > +# ifdef HAVE_AUX_VECTOR
> > +  if (GL(dl_phdr) == NULL)
> > +# endif
> > +    {
> > +      /* Starting from binutils-2.23, the linker will define the
> > +         magic symbol __ehdr_start to point to our own ELF header
> > +         if it is visible in a segment that also includes the phdrs.
> > +         So we can set up _dl_phdr and _dl_phnum even without any
> > +         information from auxv.  */
> > +
> > +      extern const ElfW(Ehdr) __ehdr_start
> > +	__attribute__ ((weak, visibility ("hidden")));
> > +      if (&__ehdr_start != NULL)
> > +        {
> > +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > +          GL(dl_phnum) = __ehdr_start.e_phnum;
> > +        }
> > +    }
> > +
> >    /* Set up the stack checker's canary.  */
> >    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> >  # ifdef THREAD_SET_STACK_GUARD
> > 
> 
> Ok.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

On 19/01/2021 11:35, Szabolcs Nagy wrote:
> The 01/19/2021 11:07, Adhemerval Zanella wrote:
>> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
>>> IFUNC resolvers may depend on tunables and cpu feature setup so
>>> move static pie self relocation after those.
>>>
>>> It is hard to guarantee that the ealy startup code does not rely
>>> on relocations so this is a bit fragile. It would be more robust
>>> to handle RELATIVE relocs early and only IRELATIVE relocs later,
>>> but the current relocation processing code cannot do that.
>>>
>>> The early startup code before relocation processing includes
>>>
>>>   _dl_aux_init (auxvec);
>>>   __libc_init_secure ();
>>>   __tunables_init (__environ);
>>>   ARCH_INIT_CPU_FEATURES ();
>>>
>>> These are simple enough that RELATIVE relocs can be avoided.
>>>
>>> __ehdr_start may require RELATIVE relocation so it was moved
>>> later, fortunately ehdr and phdr are not used in the early code.
>>>
>>> Fixes bug 27072.
>>
>> LGTM, thanks.
>>
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> 
> 
> sigh, this is an old version of this patch, i made a
> mistake putting the series together.
> 
> the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> (to get the tls program headers) so the __ehdr_start
> magic should be before that (this only matters if auxv
> lacks AT_PHDR for some reason, which should not happen
> normally on linux, so testing won't show the problem)

By normally do you mean it might happen on a specific kernel version
or is it architecture specific?

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > The 01/19/2021 11:07, Adhemerval Zanella wrote:
> >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> >>> IFUNC resolvers may depend on tunables and cpu feature setup so
> >>> move static pie self relocation after those.
> >>>
> >>> It is hard to guarantee that the ealy startup code does not rely
> >>> on relocations so this is a bit fragile. It would be more robust
> >>> to handle RELATIVE relocs early and only IRELATIVE relocs later,
> >>> but the current relocation processing code cannot do that.
> >>>
> >>> The early startup code before relocation processing includes
> >>>
> >>>   _dl_aux_init (auxvec);
> >>>   __libc_init_secure ();
> >>>   __tunables_init (__environ);
> >>>   ARCH_INIT_CPU_FEATURES ();
> >>>
> >>> These are simple enough that RELATIVE relocs can be avoided.
> >>>
> >>> __ehdr_start may require RELATIVE relocation so it was moved
> >>> later, fortunately ehdr and phdr are not used in the early code.
> >>>
> >>> Fixes bug 27072.
> >>
> >> LGTM, thanks.
> >>
> >> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> >
> >
> > sigh, this is an old version of this patch, i made a
> > mistake putting the series together.
> >
> > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > (to get the tls program headers) so the __ehdr_start
> > magic should be before that (this only matters if auxv
> > lacks AT_PHDR for some reason, which should not happen
> > normally on linux, so testing won't show the problem)
>
> By normally do you mean it might happen on a specific kernel version
> or is it architecture specific?

I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE
relocation.  I verified it by adding -Wl,-z,report-relative-reloc when building
elf/sln on x86.

-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/19/2021 06:48, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> >
> >
> > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > The 01/19/2021 11:07, Adhemerval Zanella wrote:
> > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> > >>> IFUNC resolvers may depend on tunables and cpu feature setup so
> > >>> move static pie self relocation after those.
> > >>>
> > >>> It is hard to guarantee that the ealy startup code does not rely
> > >>> on relocations so this is a bit fragile. It would be more robust
> > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > >>> but the current relocation processing code cannot do that.
> > >>>
> > >>> The early startup code before relocation processing includes
> > >>>
> > >>>   _dl_aux_init (auxvec);
> > >>>   __libc_init_secure ();
> > >>>   __tunables_init (__environ);
> > >>>   ARCH_INIT_CPU_FEATURES ();
> > >>>
> > >>> These are simple enough that RELATIVE relocs can be avoided.
> > >>>
> > >>> __ehdr_start may require RELATIVE relocation so it was moved
> > >>> later, fortunately ehdr and phdr are not used in the early code.
> > >>>
> > >>> Fixes bug 27072.
> > >>
> > >> LGTM, thanks.
> > >>
> > >> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> > >
> > >
> > > sigh, this is an old version of this patch, i made a
> > > mistake putting the series together.
> > >
> > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > (to get the tls program headers) so the __ehdr_start
> > > magic should be before that (this only matters if auxv
> > > lacks AT_PHDR for some reason, which should not happen
> > > normally on linux, so testing won't show the problem)
> >
> > By normally do you mean it might happen on a specific kernel version
> > or is it architecture specific?

i guess __ehdr_start symbol can be useful and with it
glibc does not have to depend on auxv (which an elf
loader like valgrind/qemu-user may get wrong)

however it is only used as a fallback and on linux
AT_PHDR is always expected to be present. (i don't
know if this ever triggers)

> 
> I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE
> relocation.  I verified it by adding -Wl,-z,report-relative-reloc when building
> elf/sln on x86.

it needs relative reloc on aarch64: it can be an undefined weak
symbol and that must be 0. a pc relative address computation
cannot give 0 (unless linker does some instruction rewriting,
but on aarch64 the address computation is multiple instructions
that can be spread far apart). so yeah it needs a GOT entry and
that will be either 0 or needs a RELATIVE reloc.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/19/2021 06:48, H.J. Lu wrote:
> > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > >
> > >
> > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > The 01/19/2021 11:07, Adhemerval Zanella wrote:
> > > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> > > >>> IFUNC resolvers may depend on tunables and cpu feature setup so
> > > >>> move static pie self relocation after those.
> > > >>>
> > > >>> It is hard to guarantee that the ealy startup code does not rely
> > > >>> on relocations so this is a bit fragile. It would be more robust
> > > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > > >>> but the current relocation processing code cannot do that.
> > > >>>
> > > >>> The early startup code before relocation processing includes
> > > >>>
> > > >>>   _dl_aux_init (auxvec);
> > > >>>   __libc_init_secure ();
> > > >>>   __tunables_init (__environ);
> > > >>>   ARCH_INIT_CPU_FEATURES ();
> > > >>>
> > > >>> These are simple enough that RELATIVE relocs can be avoided.
> > > >>>
> > > >>> __ehdr_start may require RELATIVE relocation so it was moved
> > > >>> later, fortunately ehdr and phdr are not used in the early code.
> > > >>>
> > > >>> Fixes bug 27072.
> > > >>
> > > >> LGTM, thanks.
> > > >>
> > > >> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> > > >
> > > >
> > > > sigh, this is an old version of this patch, i made a
> > > > mistake putting the series together.
> > > >
> > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > (to get the tls program headers) so the __ehdr_start
> > > > magic should be before that (this only matters if auxv
> > > > lacks AT_PHDR for some reason, which should not happen
> > > > normally on linux, so testing won't show the problem)
> > >
> > > By normally do you mean it might happen on a specific kernel version
> > > or is it architecture specific?
>
> i guess __ehdr_start symbol can be useful and with it
> glibc does not have to depend on auxv (which an elf
> loader like valgrind/qemu-user may get wrong)
>
> however it is only used as a fallback and on linux
> AT_PHDR is always expected to be present. (i don't
> know if this ever triggers)

Only used on Hurd?

> >
> > I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE
> > relocation.  I verified it by adding -Wl,-z,report-relative-reloc when building
> > elf/sln on x86.
>
> it needs relative reloc on aarch64: it can be an undefined weak
> symbol and that must be 0. a pc relative address computation
> cannot give 0 (unless linker does some instruction rewriting,
> but on aarch64 the address computation is multiple instructions
> that can be spread far apart). so yeah it needs a GOT entry and
> that will be either 0 or needs a RELATIVE reloc.

On x86, I converted load from GOT to simple LEA without RELATIVE
in this case.   But this is an x86 specific optimization.

-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/19/2021 06:48, H.J. Lu wrote:
> > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > <libc-alpha@sourceware.org> wrote:
> > > >
> > > >
> > > >
> > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > The 01/19/2021 11:07, Adhemerval Zanella wrote:
> > > > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> > > > >>> IFUNC resolvers may depend on tunables and cpu feature setup so
> > > > >>> move static pie self relocation after those.
> > > > >>>
> > > > >>> It is hard to guarantee that the ealy startup code does not rely
> > > > >>> on relocations so this is a bit fragile. It would be more robust
> > > > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > > > >>> but the current relocation processing code cannot do that.
> > > > >>>
> > > > >>> The early startup code before relocation processing includes
> > > > >>>
> > > > >>>   _dl_aux_init (auxvec);
> > > > >>>   __libc_init_secure ();
> > > > >>>   __tunables_init (__environ);
> > > > >>>   ARCH_INIT_CPU_FEATURES ();
> > > > >>>
> > > > >>> These are simple enough that RELATIVE relocs can be avoided.
> > > > >>>
> > > > >>> __ehdr_start may require RELATIVE relocation so it was moved
> > > > >>> later, fortunately ehdr and phdr are not used in the early code.
> > > > >>>
> > > > >>> Fixes bug 27072.
> > > > >>
> > > > >> LGTM, thanks.
> > > > >>
> > > > >> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> > > > >
> > > > >
> > > > > sigh, this is an old version of this patch, i made a
> > > > > mistake putting the series together.
> > > > >
> > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > (to get the tls program headers) so the __ehdr_start
> > > > > magic should be before that (this only matters if auxv
> > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > normally on linux, so testing won't show the problem)
> > > >
> > > > By normally do you mean it might happen on a specific kernel version
> > > > or is it architecture specific?
> >
> > i guess __ehdr_start symbol can be useful and with it
> > glibc does not have to depend on auxv (which an elf
> > loader like valgrind/qemu-user may get wrong)
> >
> > however it is only used as a fallback and on linux
> > AT_PHDR is always expected to be present. (i don't
> > know if this ever triggers)
>
> Only used on Hurd?

Does arm64 linker always define __ehdr_start?  If yes, can you drop
"weak," to see if RELATIVE goes away?

> > >
> > > I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE
> > > relocation.  I verified it by adding -Wl,-z,report-relative-reloc when building
> > > elf/sln on x86.
> >
> > it needs relative reloc on aarch64: it can be an undefined weak
> > symbol and that must be 0. a pc relative address computation
> > cannot give 0 (unless linker does some instruction rewriting,
> > but on aarch64 the address computation is multiple instructions
> > that can be spread far apart). so yeah it needs a GOT entry and
> > that will be either 0 or needs a RELATIVE reloc.
>
> On x86, I converted load from GOT to simple LEA without RELATIVE
> in this case.   But this is an x86 specific optimization.
>
> --
> H.J.



-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/19/2021 08:47, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > <libc-alpha@sourceware.org> wrote:
> > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > magic should be before that (this only matters if auxv
> > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > normally on linux, so testing won't show the problem)
> > > > >
> > > > > By normally do you mean it might happen on a specific kernel version
> > > > > or is it architecture specific?
> > >
> > > i guess __ehdr_start symbol can be useful and with it
> > > glibc does not have to depend on auxv (which an elf
> > > loader like valgrind/qemu-user may get wrong)
> > >
> > > however it is only used as a fallback and on linux
> > > AT_PHDR is always expected to be present. (i don't
> > > know if this ever triggers)
> >
> > Only used on Hurd?
> 
> Does arm64 linker always define __ehdr_start?  If yes, can you drop
> "weak," to see if RELATIVE goes away?

__ehdr_start support was added in binutils 2.23
so i guess all supported binutils has it which
means we can make it non-weak indeed.

good idea.

(we can also ignore auxv and rely on __ehdr_start only,
but for now just making it non-weak should be fine.)

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/19/2021 08:47, H.J. Lu wrote:
> > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > magic should be before that (this only matters if auxv
> > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > normally on linux, so testing won't show the problem)
> > > > > >
> > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > or is it architecture specific?
> > > >
> > > > i guess __ehdr_start symbol can be useful and with it
> > > > glibc does not have to depend on auxv (which an elf
> > > > loader like valgrind/qemu-user may get wrong)
> > > >
> > > > however it is only used as a fallback and on linux
> > > > AT_PHDR is always expected to be present. (i don't
> > > > know if this ever triggers)
> > >
> > > Only used on Hurd?
> >
> > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > "weak," to see if RELATIVE goes away?
>
> __ehdr_start support was added in binutils 2.23

We may assume binutils >= 2.33 when building for static PIE
since all static PIE linkers should define __ehdr_start.

Does lld define __ehdr_start?

> so i guess all supported binutils has it which
> means we can make it non-weak indeed.
>
> good idea.
>
> (we can also ignore auxv and rely on __ehdr_start only,
> but for now just making it non-weak should be fine.)
>


-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Fāng-ruì Sòng via Libc-alpha]

On Tue, Jan 19, 2021 at 9:11 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/19/2021 08:47, H.J. Lu wrote:
> > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > >
> > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > or is it architecture specific?
> > > > >
> > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > glibc does not have to depend on auxv (which an elf
> > > > > loader like valgrind/qemu-user may get wrong)
> > > > >
> > > > > however it is only used as a fallback and on linux
> > > > > AT_PHDR is always expected to be present. (i don't
> > > > > know if this ever triggers)
> > > >
> > > > Only used on Hurd?
> > >
> > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > "weak," to see if RELATIVE goes away?
> >
> > __ehdr_start support was added in binutils 2.23
>
> We may assume binutils >= 2.33 when building for static PIE
> since all static PIE linkers should define __ehdr_start.
>
> Does lld define __ehdr_start?

LLD defines __ehdr_start as hidden if it is not a defined symbol in
-no-pie/-pie/-shared modes.
The behavior seems to match gold. GNU ld seems to use STB_LOCAL
STV_DEFAULT but the exterior behavior should be the same.

> > so i guess all supported binutils has it which
> > means we can make it non-weak indeed.
> >
> > good idea.
> >
> > (we can also ignore auxv and rely on __ehdr_start only,
> > but for now just making it non-weak should be fine.)
> >
>
>
> --
> H.J.



-- 
宋方睿

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 9:26 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Tue, Jan 19, 2021 at 9:11 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 01/19/2021 08:47, H.J. Lu wrote:
> > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > > >
> > > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > > or is it architecture specific?
> > > > > >
> > > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > > glibc does not have to depend on auxv (which an elf
> > > > > > loader like valgrind/qemu-user may get wrong)
> > > > > >
> > > > > > however it is only used as a fallback and on linux
> > > > > > AT_PHDR is always expected to be present. (i don't
> > > > > > know if this ever triggers)
> > > > >
> > > > > Only used on Hurd?
> > > >
> > > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > > "weak," to see if RELATIVE goes away?
> > >
> > > __ehdr_start support was added in binutils 2.23
> >
> > We may assume binutils >= 2.33 when building for static PIE
> > since all static PIE linkers should define __ehdr_start.
> >
> > Does lld define __ehdr_start?
>
> LLD defines __ehdr_start as hidden if it is not a defined symbol in
> -no-pie/-pie/-shared modes.
> The behavior seems to match gold. GNU ld seems to use STB_LOCAL
> STV_DEFAULT but the exterior behavior should be the same.

I am not sure where you got such incorrect info.  Both ld and gold were changed
to STV_HIDDEN in 2013 by the same commit:

ommit cde7cb0129a884a060b99c7c83e8f5c9af728b0a
Author: Maciej W. Rozycki <macro@linux-mips.org>
Date:   Fri May 3 15:19:27 2013 +0000

            gold/
            PR ld/15365
            * layout.cc (Layout::finalize): Make __ehdr_start STV_HIDDEN.

            ld/
            PR ld/15365
            * emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation):
            Restrict __ehdr_start's export class to no less than STV_HIDDEN.

-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Fāng-ruì Sòng via Libc-alpha]

On Tue, Jan 19, 2021 at 9:34 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 9:26 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On Tue, Jan 19, 2021 at 9:11 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > >
> > > > The 01/19/2021 08:47, H.J. Lu wrote:
> > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > > > >
> > > > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > > > or is it architecture specific?
> > > > > > >
> > > > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > > > glibc does not have to depend on auxv (which an elf
> > > > > > > loader like valgrind/qemu-user may get wrong)
> > > > > > >
> > > > > > > however it is only used as a fallback and on linux
> > > > > > > AT_PHDR is always expected to be present. (i don't
> > > > > > > know if this ever triggers)
> > > > > >
> > > > > > Only used on Hurd?
> > > > >
> > > > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > > > "weak," to see if RELATIVE goes away?
> > > >
> > > > __ehdr_start support was added in binutils 2.23
> > >
> > > We may assume binutils >= 2.33 when building for static PIE
> > > since all static PIE linkers should define __ehdr_start.
> > >
> > > Does lld define __ehdr_start?
> >
> > LLD defines __ehdr_start as hidden if it is not a defined symbol in
> > -no-pie/-pie/-shared modes.
> > The behavior seems to match gold. GNU ld seems to use STB_LOCAL
> > STV_DEFAULT but the exterior behavior should be the same.
>
> I am not sure where you got such incorrect info.  Both ld and gold were changed
> to STV_HIDDEN in 2013 by the same commit:
>
> ommit cde7cb0129a884a060b99c7c83e8f5c9af728b0a
> Author: Maciej W. Rozycki <macro@linux-mips.org>
> Date:   Fri May 3 15:19:27 2013 +0000
>
>             gold/
>             PR ld/15365
>             * layout.cc (Layout::finalize): Make __ehdr_start STV_HIDDEN.
>
>             ld/
>             PR ld/15365
>             * emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation):
>             Restrict __ehdr_start's export class to no less than STV_HIDDEN.
>
> --
> H.J.

% cat a.s
.global _start, __ehdr_start
_start:
  leaq __ehdr_start(%rip), %rax
% gcc -fuse-ld=bfd -nostdlib a.s -o a.so
% readelf -Ws a.so
...
     9: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    1 __ehdr_start

When the binding is STB_LOCAL, the visibility does not really matter.

ELF spec: "A symbol with STB_LOCAL binding may not have STV_PROTECTED
visibility. A hidden symbol contained in a relocatable object must be
either removed or converted to STB_LOCAL binding by the link-editor
when the relocatable object is included in an executable file or
shared object."

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/19/2021 09:10, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/19/2021 08:47, H.J. Lu wrote:
> > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > >
> > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > or is it architecture specific?
> > > > >
> > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > glibc does not have to depend on auxv (which an elf
> > > > > loader like valgrind/qemu-user may get wrong)
> > > > >
> > > > > however it is only used as a fallback and on linux
> > > > > AT_PHDR is always expected to be present. (i don't
> > > > > know if this ever triggers)
> > > >
> > > > Only used on Hurd?
> > >
> > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > "weak," to see if RELATIVE goes away?
> >
> > __ehdr_start support was added in binutils 2.23
> 
> We may assume binutils >= 2.33 when building for static PIE
> since all static PIE linkers should define __ehdr_start.

this piece of code is used for both static PIE and non-PIE,
but we already require binutils >= 2.25 for building glibc,
dropping weak should be fine.

i will resend the series with this.

> 
> Does lld define __ehdr_start?
> 
> > so i guess all supported binutils has it which
> > means we can make it non-weak indeed.
> >
> > good idea.
> >
> > (we can also ignore auxv and rely on __ehdr_start only,
> > but for now just making it non-weak should be fine.)
> >
> 
> 
> -- 
> H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/19/2021 09:10, H.J. Lu wrote:
> > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 01/19/2021 08:47, H.J. Lu wrote:
> > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > > >
> > > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > > or is it architecture specific?
> > > > > >
> > > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > > glibc does not have to depend on auxv (which an elf
> > > > > > loader like valgrind/qemu-user may get wrong)
> > > > > >
> > > > > > however it is only used as a fallback and on linux
> > > > > > AT_PHDR is always expected to be present. (i don't
> > > > > > know if this ever triggers)
> > > > >
> > > > > Only used on Hurd?
> > > >
> > > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > > "weak," to see if RELATIVE goes away?
> > >
> > > __ehdr_start support was added in binutils 2.23
> >
> > We may assume binutils >= 2.33 when building for static PIE
> > since all static PIE linkers should define __ehdr_start.
>
> this piece of code is used for both static PIE and non-PIE,
> but we already require binutils >= 2.25 for building glibc,
> dropping weak should be fine.
>

It is safer to check BUILD_PIE_DEFAULT when dropping
weak.

-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/19/2021 09:42, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/19/2021 09:10, H.J. Lu wrote:
> > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > >
> > > > The 01/19/2021 08:47, H.J. Lu wrote:
> > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > > > >
> > > > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > > > or is it architecture specific?
> > > > > > >
> > > > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > > > glibc does not have to depend on auxv (which an elf
> > > > > > > loader like valgrind/qemu-user may get wrong)
> > > > > > >
> > > > > > > however it is only used as a fallback and on linux
> > > > > > > AT_PHDR is always expected to be present. (i don't
> > > > > > > know if this ever triggers)
> > > > > >
> > > > > > Only used on Hurd?
> > > > >
> > > > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > > > "weak," to see if RELATIVE goes away?
> > > >
> > > > __ehdr_start support was added in binutils 2.23
> > >
> > > We may assume binutils >= 2.33 when building for static PIE
> > > since all static PIE linkers should define __ehdr_start.
> >
> > this piece of code is used for both static PIE and non-PIE,
> > but we already require binutils >= 2.25 for building glibc,
> > dropping weak should be fine.
> >
> 
> It is safer to check BUILD_PIE_DEFAULT when dropping
> weak.

ok.

does static linking have weaker linker version requirement
than building glibc?

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 9:47 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/19/2021 09:42, H.J. Lu wrote:
> > On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 01/19/2021 09:10, H.J. Lu wrote:
> > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > >
> > > > > The 01/19/2021 08:47, H.J. Lu wrote:
> > > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > > > > >
> > > > > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > > > > or is it architecture specific?
> > > > > > > >
> > > > > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > > > > glibc does not have to depend on auxv (which an elf
> > > > > > > > loader like valgrind/qemu-user may get wrong)
> > > > > > > >
> > > > > > > > however it is only used as a fallback and on linux
> > > > > > > > AT_PHDR is always expected to be present. (i don't
> > > > > > > > know if this ever triggers)
> > > > > > >
> > > > > > > Only used on Hurd?
> > > > > >
> > > > > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > > > > "weak," to see if RELATIVE goes away?
> > > > >
> > > > > __ehdr_start support was added in binutils 2.23
> > > >
> > > > We may assume binutils >= 2.33 when building for static PIE
> > > > since all static PIE linkers should define __ehdr_start.
> > >
> > > this piece of code is used for both static PIE and non-PIE,
> > > but we already require binutils >= 2.25 for building glibc,
> > > dropping weak should be fine.
> > >
> >
> > It is safer to check BUILD_PIE_DEFAULT when dropping
> > weak.
>
> ok.
>
> does static linking have weaker linker version requirement
> than building glibc?

Very unlikely.  But one may be forced to use the older linker
for some reason.

-- 
H.J.

Re: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 9:53 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 9:47 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/19/2021 09:42, H.J. Lu wrote:
> > > On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > >
> > > > The 01/19/2021 09:10, H.J. Lu wrote:
> > > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > >
> > > > > > The 01/19/2021 08:47, H.J. Lu wrote:
> > > > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > > > > The 01/19/2021 06:48, H.J. Lu wrote:
> > > > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha
> > > > > > > > > > <libc-alpha@sourceware.org> wrote:
> > > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote:
> > > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS
> > > > > > > > > > > > (to get the tls program headers) so the __ehdr_start
> > > > > > > > > > > > magic should be before that (this only matters if auxv
> > > > > > > > > > > > lacks AT_PHDR for some reason, which should not happen
> > > > > > > > > > > > normally on linux, so testing won't show the problem)
> > > > > > > > > > >
> > > > > > > > > > > By normally do you mean it might happen on a specific kernel version
> > > > > > > > > > > or is it architecture specific?
> > > > > > > > >
> > > > > > > > > i guess __ehdr_start symbol can be useful and with it
> > > > > > > > > glibc does not have to depend on auxv (which an elf
> > > > > > > > > loader like valgrind/qemu-user may get wrong)
> > > > > > > > >
> > > > > > > > > however it is only used as a fallback and on linux
> > > > > > > > > AT_PHDR is always expected to be present. (i don't
> > > > > > > > > know if this ever triggers)
> > > > > > > >
> > > > > > > > Only used on Hurd?
> > > > > > >
> > > > > > > Does arm64 linker always define __ehdr_start?  If yes, can you drop
> > > > > > > "weak," to see if RELATIVE goes away?
> > > > > >
> > > > > > __ehdr_start support was added in binutils 2.23
> > > > >
> > > > > We may assume binutils >= 2.33 when building for static PIE
> > > > > since all static PIE linkers should define __ehdr_start.
> > > >
> > > > this piece of code is used for both static PIE and non-PIE,
> > > > but we already require binutils >= 2.25 for building glibc,
> > > > dropping weak should be fine.
> > > >
> > >
> > > It is safer to check BUILD_PIE_DEFAULT when dropping
> > > weak.
> >
> > ok.
> >
> > does static linking have weaker linker version requirement
> > than building glibc?
>
> Very unlikely.  But one may be forced to use the older linker
> for some reason.
>

BTW, I pushed

commit 22b79ed7f413cd980a7af0cf258da5bf82b6d5e5
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Jan 15 06:46:12 2021 -0800

    Use <startup.h> in __libc_init_secure

to master.  You need to rebase.

-- 
H.J.

[PATCH v4 09/10] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

From: "H.J. Lu" <hjl.tools@gmail.com>

Check ifunc resolver with CPU_FEATURE_USABLE and tunables in dynamic and
static executables to verify that CPUID features are initialized early in
static PIE.
---
 sysdeps/x86/Makefile                 |  14 ++++
 sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
 sysdeps/x86/tst-ifunc-isa-1.c        | 115 ++++++++++++++++++++++++++
 sysdeps/x86/tst-ifunc-isa-2-static.c |   1 +
 sysdeps/x86/tst-ifunc-isa-2.c        | 119 +++++++++++++++++++++++++++
 5 files changed, 250 insertions(+)
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index adaa2a92cd..f7969309bc 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -9,6 +9,16 @@ sysdep_headers += sys/platform/x86.h
 tests += tst-get-cpu-features tst-get-cpu-features-static \
 	 tst-cpu-features-cpuinfo tst-cpu-features-supports
 tests-static += tst-get-cpu-features-static
+ifeq (yes,$(have-ifunc))
+tests += \
+  tst-ifunc-isa-1 \
+  tst-ifunc-isa-1-static \
+  tst-ifunc-isa-2 \
+  tst-ifunc-isa-2-static
+tests-static += \
+  tst-ifunc-isa-1-static \
+  tst-ifunc-isa-2-static
+endif
 ifeq (yes,$(enable-x86-isa-level))
 tests += tst-isa-level-1
 modules-names += tst-isa-level-mod-1-baseline \
@@ -39,6 +49,10 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
 			      $(objpfx)tst-isa-level-mod-1-v3.so \
 			      $(objpfx)tst-isa-level-mod-1-v4.so
 endif
+ifneq ($(have-tunables),no)
+tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
+tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
+endif
 endif
 
 ifeq ($(subdir),math)
diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
new file mode 100644
index 0000000000..0e94f6119b
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
@@ -0,0 +1 @@
+#include "tst-ifunc-isa-1.c"
diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
new file mode 100644
index 0000000000..b3bc2a55a2
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1.c
@@ -0,0 +1,115 @@
+/* Check ifunc with CPU_FEATURE_USABLE.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+
+enum isa
+{
+  none,
+  sse2,
+  sse4_2,
+  avx,
+  avx2,
+  avx512f
+};
+
+enum isa
+get_isa (void)
+{
+  if (CPU_FEATURE_USABLE (AVX512F))
+    return avx512f;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return avx2;
+  if (CPU_FEATURE_USABLE (AVX))
+    return avx;
+  if (CPU_FEATURE_USABLE (SSE4_2))
+    return sse4_2;
+  if (CPU_FEATURE_USABLE (SSE2))
+    return sse2;
+  return none;
+}
+
+static int
+isa_sse2 (void)
+{
+  return sse2;
+}
+
+static int
+isa_sse4_2 (void)
+{
+  return sse4_2;
+}
+
+static int
+isa_avx (void)
+{
+  return avx;
+}
+
+static int
+isa_avx2 (void)
+{
+  return avx2;
+}
+
+static int
+isa_avx512f (void)
+{
+  return avx512f;
+}
+
+static int
+isa_none (void)
+{
+  return none;
+}
+
+int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
+
+void *
+foo_ifunc (void)
+{
+  switch (get_isa ())
+    {
+    case avx512f:
+      return isa_avx512f;
+    case avx2:
+      return isa_avx2;
+    case avx:
+      return isa_avx;
+    case sse4_2:
+      return isa_sse4_2;
+    case sse2:
+      return isa_sse2;
+    default:
+      break;
+    }
+  return isa_none;
+}
+
+static int
+do_test (void)
+{
+  enum isa value = foo ();
+  enum isa expected = get_isa ();
+  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-ifunc-isa-2-static.c b/sysdeps/x86/tst-ifunc-isa-2-static.c
new file mode 100644
index 0000000000..4a5af9a270
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-2-static.c
@@ -0,0 +1 @@
+#include "tst-ifunc-isa-2.c"
diff --git a/sysdeps/x86/tst-ifunc-isa-2.c b/sysdeps/x86/tst-ifunc-isa-2.c
new file mode 100644
index 0000000000..bb0f76c3e4
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-2.c
@@ -0,0 +1,119 @@
+/* Check ifunc with CPU_FEATURE_USABLE and tunables.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+#include <support/test-driver.h>
+
+enum isa
+{
+  none,
+  sse2,
+  sse4_2,
+  avx,
+  avx2,
+  avx512f
+};
+
+enum isa
+get_isa (void)
+{
+  if (CPU_FEATURE_USABLE (AVX512F))
+    return avx512f;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return avx2;
+  if (CPU_FEATURE_USABLE (AVX))
+    return avx;
+  if (CPU_FEATURE_USABLE (SSE4_2))
+    return sse4_2;
+  if (CPU_FEATURE_USABLE (SSE2))
+    return sse2;
+  return none;
+}
+
+static int
+isa_sse2 (void)
+{
+  return sse2;
+}
+
+static int
+isa_sse4_2 (void)
+{
+  return sse4_2;
+}
+
+static int
+isa_avx (void)
+{
+  return avx;
+}
+
+static int
+isa_avx2 (void)
+{
+  return avx2;
+}
+
+static int
+isa_avx512f (void)
+{
+  return avx512f;
+}
+
+static int
+isa_none (void)
+{
+  return none;
+}
+
+int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
+
+void *
+foo_ifunc (void)
+{
+  switch (get_isa ())
+    {
+    case avx512f:
+      return isa_avx512f;
+    case avx2:
+      return isa_avx2;
+    case avx:
+      return isa_avx;
+    case sse4_2:
+      return isa_sse4_2;
+    case sse2:
+      return isa_sse2;
+    default:
+      break;
+    }
+  return isa_none;
+}
+
+static int
+do_test (void)
+{
+  /* CPU must support SSE2.  */
+  if (!__builtin_cpu_supports ("sse2"))
+    return EXIT_UNSUPPORTED;
+  enum isa value = foo ();
+  /* All ISAs, but SSE2, are disabled by tunables.  */
+  return value == sse2 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
-- 
2.17.1

Re: [PATCH v4 09/10] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> Check ifunc resolver with CPU_FEATURE_USABLE and tunables in dynamic and
> static executables to verify that CPUID features are initialized early in
> static PIE.

LGTM, thanks.  Maybe refactor the tests to use a common file with the
ifunc definitions, since both a copying and pasting similar code.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/x86/Makefile                 |  14 ++++
>  sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
>  sysdeps/x86/tst-ifunc-isa-1.c        | 115 ++++++++++++++++++++++++++
>  sysdeps/x86/tst-ifunc-isa-2-static.c |   1 +
>  sysdeps/x86/tst-ifunc-isa-2.c        | 119 +++++++++++++++++++++++++++
>  5 files changed, 250 insertions(+)
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c
> 
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index adaa2a92cd..f7969309bc 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -9,6 +9,16 @@ sysdep_headers += sys/platform/x86.h
>  tests += tst-get-cpu-features tst-get-cpu-features-static \
>  	 tst-cpu-features-cpuinfo tst-cpu-features-supports
>  tests-static += tst-get-cpu-features-static
> +ifeq (yes,$(have-ifunc))
> +tests += \
> +  tst-ifunc-isa-1 \
> +  tst-ifunc-isa-1-static \
> +  tst-ifunc-isa-2 \
> +  tst-ifunc-isa-2-static
> +tests-static += \
> +  tst-ifunc-isa-1-static \
> +  tst-ifunc-isa-2-static
> +endif
>  ifeq (yes,$(enable-x86-isa-level))
>  tests += tst-isa-level-1
>  modules-names += tst-isa-level-mod-1-baseline \
> @@ -39,6 +49,10 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
>  			      $(objpfx)tst-isa-level-mod-1-v3.so \
>  			      $(objpfx)tst-isa-level-mod-1-v4.so
>  endif
> +ifneq ($(have-tunables),no)
> +tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
> +tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
> +endif
>  endif
>  
>  ifeq ($(subdir),math)

Ok.

> diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
> new file mode 100644
> index 0000000000..0e94f6119b
> --- /dev/null
> +++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
> @@ -0,0 +1 @@
> +#include "tst-ifunc-isa-1.c"

Ok.

> diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
> new file mode 100644
> index 0000000000..b3bc2a55a2
> --- /dev/null
> +++ b/sysdeps/x86/tst-ifunc-isa-1.c
> @@ -0,0 +1,115 @@
> +/* Check ifunc with CPU_FEATURE_USABLE.
> +   Copyright (C) 2021 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include <sys/platform/x86.h>
> +
> +enum isa
> +{
> +  none,
> +  sse2,
> +  sse4_2,
> +  avx,
> +  avx2,
> +  avx512f
> +};
> +
> +enum isa
> +get_isa (void)
> +{
> +  if (CPU_FEATURE_USABLE (AVX512F))
> +    return avx512f;
> +  if (CPU_FEATURE_USABLE (AVX2))
> +    return avx2;
> +  if (CPU_FEATURE_USABLE (AVX))
> +    return avx;
> +  if (CPU_FEATURE_USABLE (SSE4_2))
> +    return sse4_2;
> +  if (CPU_FEATURE_USABLE (SSE2))
> +    return sse2;
> +  return none;
> +}
> +
> +static int
> +isa_sse2 (void)
> +{
> +  return sse2;
> +}
> +
> +static int
> +isa_sse4_2 (void)
> +{
> +  return sse4_2;
> +}
> +
> +static int
> +isa_avx (void)
> +{
> +  return avx;
> +}
> +
> +static int
> +isa_avx2 (void)
> +{
> +  return avx2;
> +}
> +
> +static int
> +isa_avx512f (void)
> +{
> +  return avx512f;
> +}
> +
> +static int
> +isa_none (void)
> +{
> +  return none;
> +}
> +
> +int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
> +
> +void *
> +foo_ifunc (void)
> +{
> +  switch (get_isa ())
> +    {
> +    case avx512f:
> +      return isa_avx512f;
> +    case avx2:
> +      return isa_avx2;
> +    case avx:
> +      return isa_avx;
> +    case sse4_2:
> +      return isa_sse4_2;
> +    case sse2:
> +      return isa_sse2;
> +    default:
> +      break;
> +    }
> +  return isa_none;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  enum isa value = foo ();
> +  enum isa expected = get_isa ();
> +  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +#include <support/test-driver.c>

Ok.

> diff --git a/sysdeps/x86/tst-ifunc-isa-2-static.c b/sysdeps/x86/tst-ifunc-isa-2-static.c
> new file mode 100644
> index 0000000000..4a5af9a270
> --- /dev/null
> +++ b/sysdeps/x86/tst-ifunc-isa-2-static.c
> @@ -0,0 +1 @@
> +#include "tst-ifunc-isa-2.c"

Ok.

> diff --git a/sysdeps/x86/tst-ifunc-isa-2.c b/sysdeps/x86/tst-ifunc-isa-2.c
> new file mode 100644
> index 0000000000..bb0f76c3e4
> --- /dev/null
> +++ b/sysdeps/x86/tst-ifunc-isa-2.c
> @@ -0,0 +1,119 @@
> +/* Check ifunc with CPU_FEATURE_USABLE and tunables.
> +   Copyright (C) 2021 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include <sys/platform/x86.h>
> +#include <support/test-driver.h>
> +
> +enum isa
> +{
> +  none,
> +  sse2,
> +  sse4_2,
> +  avx,
> +  avx2,
> +  avx512f
> +};
> +
> +enum isa
> +get_isa (void)
> +{
> +  if (CPU_FEATURE_USABLE (AVX512F))
> +    return avx512f;
> +  if (CPU_FEATURE_USABLE (AVX2))
> +    return avx2;
> +  if (CPU_FEATURE_USABLE (AVX))
> +    return avx;
> +  if (CPU_FEATURE_USABLE (SSE4_2))
> +    return sse4_2;
> +  if (CPU_FEATURE_USABLE (SSE2))
> +    return sse2;
> +  return none;
> +}
> +
> +static int
> +isa_sse2 (void)
> +{
> +  return sse2;
> +}
> +
> +static int
> +isa_sse4_2 (void)
> +{
> +  return sse4_2;
> +}
> +
> +static int
> +isa_avx (void)
> +{
> +  return avx;
> +}
> +
> +static int
> +isa_avx2 (void)
> +{
> +  return avx2;
> +}
> +
> +static int
> +isa_avx512f (void)
> +{
> +  return avx512f;
> +}
> +
> +static int
> +isa_none (void)
> +{
> +  return none;
> +}
> +
> +int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
> +
> +void *
> +foo_ifunc (void)
> +{
> +  switch (get_isa ())
> +    {
> +    case avx512f:
> +      return isa_avx512f;
> +    case avx2:
> +      return isa_avx2;
> +    case avx:
> +      return isa_avx;
> +    case sse4_2:
> +      return isa_sse4_2;
> +    case sse2:
> +      return isa_sse2;
> +    default:
> +      break;
> +    }
> +  return isa_none;
> +}
> +

Maybe move this part to a common file?

> +static int
> +do_test (void)
> +{
> +  /* CPU must support SSE2.  */
> +  if (!__builtin_cpu_supports ("sse2"))
> +    return EXIT_UNSUPPORTED;
> +  enum isa value = foo ();
> +  /* All ISAs, but SSE2, are disabled by tunables.  */
> +  return value == sse2 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +#include <support/test-driver.c>
> 

Ok.

V2 [PATCH v4 09/10] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]

[H.J. Lu via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 9055 bytes --]

On Tue, Jan 19, 2021 at 6:12 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> >
> > Check ifunc resolver with CPU_FEATURE_USABLE and tunables in dynamic and
> > static executables to verify that CPUID features are initialized early in
> > static PIE.
>
> LGTM, thanks.  Maybe refactor the tests to use a common file with the
> ifunc definitions, since both a copying and pasting similar code.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> >  sysdeps/x86/Makefile                 |  14 ++++
> >  sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
> >  sysdeps/x86/tst-ifunc-isa-1.c        | 115 ++++++++++++++++++++++++++
> >  sysdeps/x86/tst-ifunc-isa-2-static.c |   1 +
> >  sysdeps/x86/tst-ifunc-isa-2.c        | 119 +++++++++++++++++++++++++++
> >  5 files changed, 250 insertions(+)
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c
> >
> > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> > index adaa2a92cd..f7969309bc 100644
> > --- a/sysdeps/x86/Makefile
> > +++ b/sysdeps/x86/Makefile
> > @@ -9,6 +9,16 @@ sysdep_headers += sys/platform/x86.h
> >  tests += tst-get-cpu-features tst-get-cpu-features-static \
> >        tst-cpu-features-cpuinfo tst-cpu-features-supports
> >  tests-static += tst-get-cpu-features-static
> > +ifeq (yes,$(have-ifunc))
> > +tests += \
> > +  tst-ifunc-isa-1 \
> > +  tst-ifunc-isa-1-static \
> > +  tst-ifunc-isa-2 \
> > +  tst-ifunc-isa-2-static
> > +tests-static += \
> > +  tst-ifunc-isa-1-static \
> > +  tst-ifunc-isa-2-static
> > +endif
> >  ifeq (yes,$(enable-x86-isa-level))
> >  tests += tst-isa-level-1
> >  modules-names += tst-isa-level-mod-1-baseline \
> > @@ -39,6 +49,10 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
> >                             $(objpfx)tst-isa-level-mod-1-v3.so \
> >                             $(objpfx)tst-isa-level-mod-1-v4.so
> >  endif
> > +ifneq ($(have-tunables),no)
> > +tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
> > +tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
> > +endif
> >  endif
> >
> >  ifeq ($(subdir),math)
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
> > new file mode 100644
> > index 0000000000..0e94f6119b
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
> > @@ -0,0 +1 @@
> > +#include "tst-ifunc-isa-1.c"
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
> > new file mode 100644
> > index 0000000000..b3bc2a55a2
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-ifunc-isa-1.c
> > @@ -0,0 +1,115 @@
> > +/* Check ifunc with CPU_FEATURE_USABLE.
> > +   Copyright (C) 2021 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
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <stdlib.h>
> > +#include <sys/platform/x86.h>
> > +
> > +enum isa
> > +{
> > +  none,
> > +  sse2,
> > +  sse4_2,
> > +  avx,
> > +  avx2,
> > +  avx512f
> > +};
> > +
> > +enum isa
> > +get_isa (void)
> > +{
> > +  if (CPU_FEATURE_USABLE (AVX512F))
> > +    return avx512f;
> > +  if (CPU_FEATURE_USABLE (AVX2))
> > +    return avx2;
> > +  if (CPU_FEATURE_USABLE (AVX))
> > +    return avx;
> > +  if (CPU_FEATURE_USABLE (SSE4_2))
> > +    return sse4_2;
> > +  if (CPU_FEATURE_USABLE (SSE2))
> > +    return sse2;
> > +  return none;
> > +}
> > +
> > +static int
> > +isa_sse2 (void)
> > +{
> > +  return sse2;
> > +}
> > +
> > +static int
> > +isa_sse4_2 (void)
> > +{
> > +  return sse4_2;
> > +}
> > +
> > +static int
> > +isa_avx (void)
> > +{
> > +  return avx;
> > +}
> > +
> > +static int
> > +isa_avx2 (void)
> > +{
> > +  return avx2;
> > +}
> > +
> > +static int
> > +isa_avx512f (void)
> > +{
> > +  return avx512f;
> > +}
> > +
> > +static int
> > +isa_none (void)
> > +{
> > +  return none;
> > +}
> > +
> > +int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
> > +
> > +void *
> > +foo_ifunc (void)
> > +{
> > +  switch (get_isa ())
> > +    {
> > +    case avx512f:
> > +      return isa_avx512f;
> > +    case avx2:
> > +      return isa_avx2;
> > +    case avx:
> > +      return isa_avx;
> > +    case sse4_2:
> > +      return isa_sse4_2;
> > +    case sse2:
> > +      return isa_sse2;
> > +    default:
> > +      break;
> > +    }
> > +  return isa_none;
> > +}
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  enum isa value = foo ();
> > +  enum isa expected = get_isa ();
> > +  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
> > +}
> > +
> > +#include <support/test-driver.c>
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-ifunc-isa-2-static.c b/sysdeps/x86/tst-ifunc-isa-2-static.c
> > new file mode 100644
> > index 0000000000..4a5af9a270
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-ifunc-isa-2-static.c
> > @@ -0,0 +1 @@
> > +#include "tst-ifunc-isa-2.c"
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-ifunc-isa-2.c b/sysdeps/x86/tst-ifunc-isa-2.c
> > new file mode 100644
> > index 0000000000..bb0f76c3e4
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-ifunc-isa-2.c
> > @@ -0,0 +1,119 @@
> > +/* Check ifunc with CPU_FEATURE_USABLE and tunables.
> > +   Copyright (C) 2021 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
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <stdlib.h>
> > +#include <sys/platform/x86.h>
> > +#include <support/test-driver.h>
> > +
> > +enum isa
> > +{
> > +  none,
> > +  sse2,
> > +  sse4_2,
> > +  avx,
> > +  avx2,
> > +  avx512f
> > +};
> > +
> > +enum isa
> > +get_isa (void)
> > +{
> > +  if (CPU_FEATURE_USABLE (AVX512F))
> > +    return avx512f;
> > +  if (CPU_FEATURE_USABLE (AVX2))
> > +    return avx2;
> > +  if (CPU_FEATURE_USABLE (AVX))
> > +    return avx;
> > +  if (CPU_FEATURE_USABLE (SSE4_2))
> > +    return sse4_2;
> > +  if (CPU_FEATURE_USABLE (SSE2))
> > +    return sse2;
> > +  return none;
> > +}
> > +
> > +static int
> > +isa_sse2 (void)
> > +{
> > +  return sse2;
> > +}
> > +
> > +static int
> > +isa_sse4_2 (void)
> > +{
> > +  return sse4_2;
> > +}
> > +
> > +static int
> > +isa_avx (void)
> > +{
> > +  return avx;
> > +}
> > +
> > +static int
> > +isa_avx2 (void)
> > +{
> > +  return avx2;
> > +}
> > +
> > +static int
> > +isa_avx512f (void)
> > +{
> > +  return avx512f;
> > +}
> > +
> > +static int
> > +isa_none (void)
> > +{
> > +  return none;
> > +}
> > +
> > +int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
> > +
> > +void *
> > +foo_ifunc (void)
> > +{
> > +  switch (get_isa ())
> > +    {
> > +    case avx512f:
> > +      return isa_avx512f;
> > +    case avx2:
> > +      return isa_avx2;
> > +    case avx:
> > +      return isa_avx;
> > +    case sse4_2:
> > +      return isa_sse4_2;
> > +    case sse2:
> > +      return isa_sse2;
> > +    default:
> > +      break;
> > +    }
> > +  return isa_none;
> > +}
> > +
>
> Maybe move this part to a common file?

Fixed.

> > +static int
> > +do_test (void)
> > +{
> > +  /* CPU must support SSE2.  */
> > +  if (!__builtin_cpu_supports ("sse2"))
> > +    return EXIT_UNSUPPORTED;
> > +  enum isa value = foo ();
> > +  /* All ISAs, but SSE2, are disabled by tunables.  */
> > +  return value == sse2 ? EXIT_SUCCESS : EXIT_FAILURE;
> > +}
> > +
> > +#include <support/test-driver.c>
> >
>
> Ok.

Here is the updated patch.  OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-x86-Check-ifunc-resolver-with-CPU_FEATURE_USABLE-BZ-.patch --]
[-- Type: text/x-patch, Size: 7492 bytes --]

From f311b138e90a77f5a1153093a0496884bcb8c7e4 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 12 Jan 2021 14:41:10 -0800
Subject: [PATCH] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]

Check ifunc resolver with CPU_FEATURE_USABLE and tunables in dynamic and
static executables to verify that CPUID features are initialized early in
static PIE.
---
 sysdeps/x86/Makefile                 |  14 ++++
 sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
 sysdeps/x86/tst-ifunc-isa-1.c        |  30 ++++++++
 sysdeps/x86/tst-ifunc-isa-2-static.c |   1 +
 sysdeps/x86/tst-ifunc-isa-2.c        |  34 +++++++++
 sysdeps/x86/tst-ifunc-isa.h          | 104 +++++++++++++++++++++++++++
 6 files changed, 184 insertions(+)
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa.h

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index adaa2a92cd..f7969309bc 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -9,6 +9,16 @@ sysdep_headers += sys/platform/x86.h
 tests += tst-get-cpu-features tst-get-cpu-features-static \
 	 tst-cpu-features-cpuinfo tst-cpu-features-supports
 tests-static += tst-get-cpu-features-static
+ifeq (yes,$(have-ifunc))
+tests += \
+  tst-ifunc-isa-1 \
+  tst-ifunc-isa-1-static \
+  tst-ifunc-isa-2 \
+  tst-ifunc-isa-2-static
+tests-static += \
+  tst-ifunc-isa-1-static \
+  tst-ifunc-isa-2-static
+endif
 ifeq (yes,$(enable-x86-isa-level))
 tests += tst-isa-level-1
 modules-names += tst-isa-level-mod-1-baseline \
@@ -39,6 +49,10 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
 			      $(objpfx)tst-isa-level-mod-1-v3.so \
 			      $(objpfx)tst-isa-level-mod-1-v4.so
 endif
+ifneq ($(have-tunables),no)
+tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
+tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
+endif
 endif
 
 ifeq ($(subdir),math)
diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
new file mode 100644
index 0000000000..0e94f6119b
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
@@ -0,0 +1 @@
+#include "tst-ifunc-isa-1.c"
diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
new file mode 100644
index 0000000000..37d599210c
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1.c
@@ -0,0 +1,30 @@
+/* Check IFUNC resolver with CPU_FEATURE_USABLE.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include "tst-ifunc-isa.h"
+
+static int
+do_test (void)
+{
+  enum isa value = foo ();
+  enum isa expected = get_isa ();
+  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-ifunc-isa-2-static.c b/sysdeps/x86/tst-ifunc-isa-2-static.c
new file mode 100644
index 0000000000..4a5af9a270
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-2-static.c
@@ -0,0 +1 @@
+#include "tst-ifunc-isa-2.c"
diff --git a/sysdeps/x86/tst-ifunc-isa-2.c b/sysdeps/x86/tst-ifunc-isa-2.c
new file mode 100644
index 0000000000..0bdf7176fa
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-2.c
@@ -0,0 +1,34 @@
+/* Check IFUNC resolver with CPU_FEATURE_USABLE and tunables.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include "tst-ifunc-isa.h"
+#include <support/test-driver.h>
+
+static int
+do_test (void)
+{
+  /* CPU must support SSE2.  */
+  if (!__builtin_cpu_supports ("sse2"))
+    return EXIT_UNSUPPORTED;
+  enum isa value = foo ();
+  /* All ISAs, but SSE2, are disabled by tunables.  */
+  return value == sse2 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-ifunc-isa.h b/sysdeps/x86/tst-ifunc-isa.h
new file mode 100644
index 0000000000..60aa1cea6a
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa.h
@@ -0,0 +1,104 @@
+/* IFUNC resolver with CPU_FEATURE_USABLE.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sys/platform/x86.h>
+
+enum isa
+{
+  none,
+  sse2,
+  sse4_2,
+  avx,
+  avx2,
+  avx512f
+};
+
+enum isa
+get_isa (void)
+{
+  if (CPU_FEATURE_USABLE (AVX512F))
+    return avx512f;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return avx2;
+  if (CPU_FEATURE_USABLE (AVX))
+    return avx;
+  if (CPU_FEATURE_USABLE (SSE4_2))
+    return sse4_2;
+  if (CPU_FEATURE_USABLE (SSE2))
+    return sse2;
+  return none;
+}
+
+static int
+isa_sse2 (void)
+{
+  return sse2;
+}
+
+static int
+isa_sse4_2 (void)
+{
+  return sse4_2;
+}
+
+static int
+isa_avx (void)
+{
+  return avx;
+}
+
+static int
+isa_avx2 (void)
+{
+  return avx2;
+}
+
+static int
+isa_avx512f (void)
+{
+  return avx512f;
+}
+
+static int
+isa_none (void)
+{
+  return none;
+}
+
+int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
+
+void *
+foo_ifunc (void)
+{
+  switch (get_isa ())
+    {
+    case avx512f:
+      return isa_avx512f;
+    case avx2:
+      return isa_avx2;
+    case avx:
+      return isa_avx;
+    case sse4_2:
+      return isa_sse4_2;
+    case sse2:
+      return isa_sse2;
+    default:
+      break;
+    }
+  return isa_none;
+}
-- 
2.29.2

Re: V2 [PATCH v4 09/10] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 6:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 6:12 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> >
> >
> > On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote:
> > > From: "H.J. Lu" <hjl.tools@gmail.com>
> > >
> > > Check ifunc resolver with CPU_FEATURE_USABLE and tunables in dynamic and
> > > static executables to verify that CPUID features are initialized early in
> > > static PIE.
> >
> > LGTM, thanks.  Maybe refactor the tests to use a common file with the
> > ifunc definitions, since both a copying and pasting similar code.
> >
> > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> >
> > > ---
> > >  sysdeps/x86/Makefile                 |  14 ++++
> > >  sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
> > >  sysdeps/x86/tst-ifunc-isa-1.c        | 115 ++++++++++++++++++++++++++
> > >  sysdeps/x86/tst-ifunc-isa-2-static.c |   1 +
> > >  sysdeps/x86/tst-ifunc-isa-2.c        | 119 +++++++++++++++++++++++++++
> > >  5 files changed, 250 insertions(+)
> > >  create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
> > >  create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
> > >  create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
> > >  create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c
> > >
> > > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> > > index adaa2a92cd..f7969309bc 100644
> > > --- a/sysdeps/x86/Makefile
> > > +++ b/sysdeps/x86/Makefile
> > > @@ -9,6 +9,16 @@ sysdep_headers += sys/platform/x86.h
> > >  tests += tst-get-cpu-features tst-get-cpu-features-static \
> > >        tst-cpu-features-cpuinfo tst-cpu-features-supports
> > >  tests-static += tst-get-cpu-features-static
> > > +ifeq (yes,$(have-ifunc))
> > > +tests += \
> > > +  tst-ifunc-isa-1 \
> > > +  tst-ifunc-isa-1-static \
> > > +  tst-ifunc-isa-2 \
> > > +  tst-ifunc-isa-2-static
> > > +tests-static += \
> > > +  tst-ifunc-isa-1-static \
> > > +  tst-ifunc-isa-2-static
> > > +endif
> > >  ifeq (yes,$(enable-x86-isa-level))
> > >  tests += tst-isa-level-1
> > >  modules-names += tst-isa-level-mod-1-baseline \
> > > @@ -39,6 +49,10 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
> > >                             $(objpfx)tst-isa-level-mod-1-v3.so \
> > >                             $(objpfx)tst-isa-level-mod-1-v4.so
> > >  endif
> > > +ifneq ($(have-tunables),no)
> > > +tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
> > > +tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
> > > +endif
> > >  endif
> > >
> > >  ifeq ($(subdir),math)
> >
> > Ok.
> >
> > > diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
> > > new file mode 100644
> > > index 0000000000..0e94f6119b
> > > --- /dev/null
> > > +++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
> > > @@ -0,0 +1 @@
> > > +#include "tst-ifunc-isa-1.c"
> >
> > Ok.
> >
> > > diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
> > > new file mode 100644
> > > index 0000000000..b3bc2a55a2
> > > --- /dev/null
> > > +++ b/sysdeps/x86/tst-ifunc-isa-1.c
> > > @@ -0,0 +1,115 @@
> > > +/* Check ifunc with CPU_FEATURE_USABLE.
> > > +   Copyright (C) 2021 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
> > > +   <https://www.gnu.org/licenses/>.  */
> > > +
> > > +#include <stdlib.h>
> > > +#include <sys/platform/x86.h>
> > > +
> > > +enum isa
> > > +{
> > > +  none,
> > > +  sse2,
> > > +  sse4_2,
> > > +  avx,
> > > +  avx2,
> > > +  avx512f
> > > +};
> > > +
> > > +enum isa
> > > +get_isa (void)
> > > +{
> > > +  if (CPU_FEATURE_USABLE (AVX512F))
> > > +    return avx512f;
> > > +  if (CPU_FEATURE_USABLE (AVX2))
> > > +    return avx2;
> > > +  if (CPU_FEATURE_USABLE (AVX))
> > > +    return avx;
> > > +  if (CPU_FEATURE_USABLE (SSE4_2))
> > > +    return sse4_2;
> > > +  if (CPU_FEATURE_USABLE (SSE2))
> > > +    return sse2;
> > > +  return none;
> > > +}
> > > +
> > > +static int
> > > +isa_sse2 (void)
> > > +{
> > > +  return sse2;
> > > +}
> > > +
> > > +static int
> > > +isa_sse4_2 (void)
> > > +{
> > > +  return sse4_2;
> > > +}
> > > +
> > > +static int
> > > +isa_avx (void)
> > > +{
> > > +  return avx;
> > > +}
> > > +
> > > +static int
> > > +isa_avx2 (void)
> > > +{
> > > +  return avx2;
> > > +}
> > > +
> > > +static int
> > > +isa_avx512f (void)
> > > +{
> > > +  return avx512f;
> > > +}
> > > +
> > > +static int
> > > +isa_none (void)
> > > +{
> > > +  return none;
> > > +}
> > > +
> > > +int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
> > > +
> > > +void *
> > > +foo_ifunc (void)
> > > +{
> > > +  switch (get_isa ())
> > > +    {
> > > +    case avx512f:
> > > +      return isa_avx512f;
> > > +    case avx2:
> > > +      return isa_avx2;
> > > +    case avx:
> > > +      return isa_avx;
> > > +    case sse4_2:
> > > +      return isa_sse4_2;
> > > +    case sse2:
> > > +      return isa_sse2;
> > > +    default:
> > > +      break;
> > > +    }
> > > +  return isa_none;
> > > +}
> > > +
> > > +static int
> > > +do_test (void)
> > > +{
> > > +  enum isa value = foo ();
> > > +  enum isa expected = get_isa ();
> > > +  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
> > > +}
> > > +
> > > +#include <support/test-driver.c>
> >
> > Ok.
> >
> > > diff --git a/sysdeps/x86/tst-ifunc-isa-2-static.c b/sysdeps/x86/tst-ifunc-isa-2-static.c
> > > new file mode 100644
> > > index 0000000000..4a5af9a270
> > > --- /dev/null
> > > +++ b/sysdeps/x86/tst-ifunc-isa-2-static.c
> > > @@ -0,0 +1 @@
> > > +#include "tst-ifunc-isa-2.c"
> >
> > Ok.
> >
> > > diff --git a/sysdeps/x86/tst-ifunc-isa-2.c b/sysdeps/x86/tst-ifunc-isa-2.c
> > > new file mode 100644
> > > index 0000000000..bb0f76c3e4
> > > --- /dev/null
> > > +++ b/sysdeps/x86/tst-ifunc-isa-2.c
> > > @@ -0,0 +1,119 @@
> > > +/* Check ifunc with CPU_FEATURE_USABLE and tunables.
> > > +   Copyright (C) 2021 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
> > > +   <https://www.gnu.org/licenses/>.  */
> > > +
> > > +#include <stdlib.h>
> > > +#include <sys/platform/x86.h>
> > > +#include <support/test-driver.h>
> > > +
> > > +enum isa
> > > +{
> > > +  none,
> > > +  sse2,
> > > +  sse4_2,
> > > +  avx,
> > > +  avx2,
> > > +  avx512f
> > > +};
> > > +
> > > +enum isa
> > > +get_isa (void)
> > > +{
> > > +  if (CPU_FEATURE_USABLE (AVX512F))
> > > +    return avx512f;
> > > +  if (CPU_FEATURE_USABLE (AVX2))
> > > +    return avx2;
> > > +  if (CPU_FEATURE_USABLE (AVX))
> > > +    return avx;
> > > +  if (CPU_FEATURE_USABLE (SSE4_2))
> > > +    return sse4_2;
> > > +  if (CPU_FEATURE_USABLE (SSE2))
> > > +    return sse2;
> > > +  return none;
> > > +}
> > > +
> > > +static int
> > > +isa_sse2 (void)
> > > +{
> > > +  return sse2;
> > > +}
> > > +
> > > +static int
> > > +isa_sse4_2 (void)
> > > +{
> > > +  return sse4_2;
> > > +}
> > > +
> > > +static int
> > > +isa_avx (void)
> > > +{
> > > +  return avx;
> > > +}
> > > +
> > > +static int
> > > +isa_avx2 (void)
> > > +{
> > > +  return avx2;
> > > +}
> > > +
> > > +static int
> > > +isa_avx512f (void)
> > > +{
> > > +  return avx512f;
> > > +}
> > > +
> > > +static int
> > > +isa_none (void)
> > > +{
> > > +  return none;
> > > +}
> > > +
> > > +int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
> > > +
> > > +void *
> > > +foo_ifunc (void)
> > > +{
> > > +  switch (get_isa ())
> > > +    {
> > > +    case avx512f:
> > > +      return isa_avx512f;
> > > +    case avx2:
> > > +      return isa_avx2;
> > > +    case avx:
> > > +      return isa_avx;
> > > +    case sse4_2:
> > > +      return isa_sse4_2;
> > > +    case sse2:
> > > +      return isa_sse2;
> > > +    default:
> > > +      break;
> > > +    }
> > > +  return isa_none;
> > > +}
> > > +
> >
> > Maybe move this part to a common file?
>
> Fixed.
>
> > > +static int
> > > +do_test (void)
> > > +{
> > > +  /* CPU must support SSE2.  */
> > > +  if (!__builtin_cpu_supports ("sse2"))
> > > +    return EXIT_UNSUPPORTED;
> > > +  enum isa value = foo ();
> > > +  /* All ISAs, but SSE2, are disabled by tunables.  */
> > > +  return value == sse2 ? EXIT_SUCCESS : EXIT_FAILURE;
> > > +}
> > > +
> > > +#include <support/test-driver.c>
> > >
> >
> > Ok.
>
> Here is the updated patch.  OK for master?
>
> Thanks.
>

Static PIE changes have been checked in.  I am checking this in.

-- 
H.J.

[PATCH v4 10/10] Make libc symbols hidden in static PIE

[Szabolcs Nagy via Libc-alpha]

Hidden matters with static PIE: extern symbol access in position
independent code usually involves GOT indirections which needs
RELATIVE relocs in a static linked PIE. Hidden visibility avoids
indirections and RELATIVE relocs on targets that can access symbols
pc-relative.

The check should use IS_IN_LIB instead of IS_IN(libc) since all
static libraries can use hidden visibility to avoid indirections,
however the test system links objects from libcrypt.a into dynamic
linked test binaries so hidden does not work there.  I think mixing
static and shared libc components in the same binary should not be
supported usage, but to be safe only use hidden in libc.a.

This is an optimization. But on some targets (i386) it cannot be
applied because hidden visibility PIE ifunc functions don't work,
so it is gated by NO_HIDDEN_EXTERN_FUNC_IN_PIE.

From -static-pie linked 'int main(){}' this shaves off 71 relative
relocs on aarch64 and reduces code size by about 2k.
---
 include/libc-symbols.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index ea126ae70c..f4dd735555 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,13 +434,18 @@ for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED \
-  || (BUILD_PIE_DEFAULT && IS_IN (libc))
+#if defined SHARED || defined LIBC_NONSHARED
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
 #endif
 
+/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
+#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+    && IS_IN (libc) && !defined LIBC_NONSHARED
+# pragma GCC visibility push(hidden)
+#endif
+
 #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
 
 #define attribute_relro __attribute__ ((section (".data.rel.ro")))
-- 
2.17.1

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

As a side note I tried your branch with a build for all support Linux
ABIs and it as least improves by issuing an error on architectures
where previously indicated support static-pie but it is broken in practice
(powerpc for instance [1])

So currently static-pie are support for all architectures, however it
fails to build for:

alpha-linux-gnu
arc-linux-gnuhf
hppa-linux-gnu
ia64-linux-gnu
m68k-linux-gnu
microblaze-linux-gnu
riscv32-linux-gnu-rv32imafdc-ilp32d
riscv64-linux-gnu-rv64imafdc-lp64d
s390-linux-gnu
sparc64-linux-gnu
sparcv9-linux-gnu

By requiring PI_STATIC_AND_HIDDEN hppa, m68, microblaze, mips, nios2,
and powerpc fail at configure.  Some architecture still fails at build:

alpha-linux-gnu
arc-linux-gnuhf
riscv32-linux-gnu-rv32imafdc-ilp32d
riscv64-linux-gnu-rv64imafdc-lp64d
s390-linux-gnu
sparc64-linux-gnu
sparcv9-linux-gnu

I haven't checked if mips is currently broken for static-pie (since
as for powerpc, it does not define PI_STATIC_AND_HIDDEN); but I would
expect so.

It would be good if we could avoid building the broken configuration
and warn it on configure, but I don't think this should be a blocker.
The NEWS for 2.27 states this features is only supported for x86
and aarch64, so I wonder if would be better to just enable it for
the supported architectures instead of relying on PI_STATIC_AND_HIDDEN.

I will finish review the patchset tomorrow.

[1] https://bugs.gentoo.org/719444

On 18/01/2021 13:22, Szabolcs Nagy via Libc-alpha wrote:
> v4:
> - added patches from H.J.Lu:
> 	- x86: fix libmvec tests
> 	- x86: fix syscalls in libc_enable_secure
> 	- x86: avoid relative reloc for _dl_sysinfo
> 	- x86: add ifunc test
> - i386 cannot mark all symbols hidden, so use fine grain
>   marking in files that participate in early code before
>   static pie self relocation.
> - the patch that makes all libc symbols hidden is still
>   included: it is now only an optimization for non-i386
>   targets.
> 
> v3:
> - refactor tunables: move internals out of dl-tunables.h
> - use generated max string length in the tunables list
>   instead of magic values.
> 
> v2:
> - check PI_STATIC_AND_HIDDEN for --enable-static-pie
> - change string buffer sizes in the tunables
> - fix env_alias == NULL logic in __tunables_init
> - move __ehdr_start processing after self relocation
> 
> 
> force pushed into nsz/bug27072 branch.
> 
> Issues that are not addressed:
> - tunables try to allocate memory even with non-suid exe.
>   allocation is only needed for rewriting the GLIBC_TUNABLES
>   env var. (i think a case can be made that if anything there
>   is TUNABLE_SECLEVEL_SXID_ERASE then this env var would be
>   simply dropped, that would simplify this significantly).
> - __sbrk only needs the hidden visibility magic because of
>   tunables, ideally we would not do allocations before self
>   relocation.
> - tunable list data structure is not optimized for compactness.
> - all symbols are forced hidden in libc.a, but i think lib*.a
>   should do the same. (other than lib*_nonshared.a)
> - i386 introduced a fair bit of complications: may be avoiding
>   relative relocs is too much to ask for and relocations should
>   be done in two steps after all: relative first, then irelative
>   when tunable etc are set up.
> 
> H.J. Lu (4):
>   libmvec: Add extra-test-objs to test-extras
>   elf: Avoid RELATIVE relocation for _dl_sysinfo
>   Use <startup.h> in __libc_init_secure
>   x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]
> 
> Szabolcs Nagy (6):
>   configure: Require PI_STATIC_AND_HIDDEN for static pie
>   elf: Make the tunable struct definition internal only
>   elf: Avoid RELATIVE relocs in __tunables_init
>   Use hidden visibility for early static PIE code
>   csu: Move static pie self relocation later [BZ #27072]
>   Make libc symbols hidden in static PIE
> 
>  configure                                    |  14 +++
>  configure.ac                                 |   5 +
>  csu/libc-start.c                             |  48 +++++---
>  elf/dl-reloc-static-pie.c                    |   2 +
>  elf/dl-support.c                             |  18 ++-
>  elf/dl-tunable-types.h                       |  42 +++++--
>  elf/dl-tunables.c                            |   6 +-
>  elf/dl-tunables.h                            |  35 ++----
>  elf/enbl-secure.c                            |  10 +-
>  include/libc-symbols.h                       |   9 +-
>  misc/sbrk.c                                  |   4 +
>  scripts/gen-tunables.awk                     |  16 ++-
>  sysdeps/generic/startup.h                    |  26 ++++
>  sysdeps/unix/sysv/linux/aarch64/libc-start.c |   5 +
>  sysdeps/unix/sysv/linux/i386/startup.h       |  29 ++++-
>  sysdeps/x86/Makefile                         |  14 +++
>  sysdeps/x86/libc-start.c                     |   5 +
>  sysdeps/x86/tst-ifunc-isa-1-static.c         |   1 +
>  sysdeps/x86/tst-ifunc-isa-1.c                | 115 ++++++++++++++++++
>  sysdeps/x86/tst-ifunc-isa-2-static.c         |   1 +
>  sysdeps/x86/tst-ifunc-isa-2.c                | 119 +++++++++++++++++++
>  sysdeps/x86_64/fpu/Makefile                  |   8 ++
>  22 files changed, 465 insertions(+), 67 deletions(-)
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
>  create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c
> 

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/18/2021 18:37, Adhemerval Zanella wrote:
> As a side note I tried your branch with a build for all support Linux
> ABIs and it as least improves by issuing an error on architectures
> where previously indicated support static-pie but it is broken in practice
> (powerpc for instance [1])
> 
> So currently static-pie are support for all architectures, however it
> fails to build for:
> 
> alpha-linux-gnu
> arc-linux-gnuhf
> hppa-linux-gnu
> ia64-linux-gnu
> m68k-linux-gnu
> microblaze-linux-gnu
> riscv32-linux-gnu-rv32imafdc-ilp32d
> riscv64-linux-gnu-rv64imafdc-lp64d
> s390-linux-gnu
> sparc64-linux-gnu
> sparcv9-linux-gnu
> 
> By requiring PI_STATIC_AND_HIDDEN hppa, m68, microblaze, mips, nios2,
> and powerpc fail at configure.  Some architecture still fails at build:
> 
> alpha-linux-gnu
> arc-linux-gnuhf
> riscv32-linux-gnu-rv32imafdc-ilp32d
> riscv64-linux-gnu-rv64imafdc-lp64d
> s390-linux-gnu
> sparc64-linux-gnu
> sparcv9-linux-gnu
> 
> I haven't checked if mips is currently broken for static-pie (since
> as for powerpc, it does not define PI_STATIC_AND_HIDDEN); but I would
> expect so.
> 
> It would be good if we could avoid building the broken configuration
> and warn it on configure, but I don't think this should be a blocker.
> The NEWS for 2.27 states this features is only supported for x86
> and aarch64, so I wonder if would be better to just enable it for
> the supported architectures instead of relying on PI_STATIC_AND_HIDDEN.

i randomly checked alpha, which fails while linking sln:

elf/dl-reloc-static-pie.c:40: undefined reference to `_DYNAMIC'

i think this should be possible to configure test in case others
targets fail in a similar way.


> 
> I will finish review the patchset tomorrow.
> 
> [1] https://bugs.gentoo.org/719444
> 
> On 18/01/2021 13:22, Szabolcs Nagy via Libc-alpha wrote:
> > v4:
> > - added patches from H.J.Lu:
> > 	- x86: fix libmvec tests
> > 	- x86: fix syscalls in libc_enable_secure
> > 	- x86: avoid relative reloc for _dl_sysinfo
> > 	- x86: add ifunc test
> > - i386 cannot mark all symbols hidden, so use fine grain
> >   marking in files that participate in early code before
> >   static pie self relocation.
> > - the patch that makes all libc symbols hidden is still
> >   included: it is now only an optimization for non-i386
> >   targets.
> > 
> > v3:
> > - refactor tunables: move internals out of dl-tunables.h
> > - use generated max string length in the tunables list
> >   instead of magic values.
> > 
> > v2:
> > - check PI_STATIC_AND_HIDDEN for --enable-static-pie
> > - change string buffer sizes in the tunables
> > - fix env_alias == NULL logic in __tunables_init
> > - move __ehdr_start processing after self relocation
> > 
> > 
> > force pushed into nsz/bug27072 branch.
> > 
> > Issues that are not addressed:
> > - tunables try to allocate memory even with non-suid exe.
> >   allocation is only needed for rewriting the GLIBC_TUNABLES
> >   env var. (i think a case can be made that if anything there
> >   is TUNABLE_SECLEVEL_SXID_ERASE then this env var would be
> >   simply dropped, that would simplify this significantly).
> > - __sbrk only needs the hidden visibility magic because of
> >   tunables, ideally we would not do allocations before self
> >   relocation.
> > - tunable list data structure is not optimized for compactness.
> > - all symbols are forced hidden in libc.a, but i think lib*.a
> >   should do the same. (other than lib*_nonshared.a)
> > - i386 introduced a fair bit of complications: may be avoiding
> >   relative relocs is too much to ask for and relocations should
> >   be done in two steps after all: relative first, then irelative
> >   when tunable etc are set up.
> > 
> > H.J. Lu (4):
> >   libmvec: Add extra-test-objs to test-extras
> >   elf: Avoid RELATIVE relocation for _dl_sysinfo
> >   Use <startup.h> in __libc_init_secure
> >   x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072]
> > 
> > Szabolcs Nagy (6):
> >   configure: Require PI_STATIC_AND_HIDDEN for static pie
> >   elf: Make the tunable struct definition internal only
> >   elf: Avoid RELATIVE relocs in __tunables_init
> >   Use hidden visibility for early static PIE code
> >   csu: Move static pie self relocation later [BZ #27072]
> >   Make libc symbols hidden in static PIE
> > 
> >  configure                                    |  14 +++
> >  configure.ac                                 |   5 +
> >  csu/libc-start.c                             |  48 +++++---
> >  elf/dl-reloc-static-pie.c                    |   2 +
> >  elf/dl-support.c                             |  18 ++-
> >  elf/dl-tunable-types.h                       |  42 +++++--
> >  elf/dl-tunables.c                            |   6 +-
> >  elf/dl-tunables.h                            |  35 ++----
> >  elf/enbl-secure.c                            |  10 +-
> >  include/libc-symbols.h                       |   9 +-
> >  misc/sbrk.c                                  |   4 +
> >  scripts/gen-tunables.awk                     |  16 ++-
> >  sysdeps/generic/startup.h                    |  26 ++++
> >  sysdeps/unix/sysv/linux/aarch64/libc-start.c |   5 +
> >  sysdeps/unix/sysv/linux/i386/startup.h       |  29 ++++-
> >  sysdeps/x86/Makefile                         |  14 +++
> >  sysdeps/x86/libc-start.c                     |   5 +
> >  sysdeps/x86/tst-ifunc-isa-1-static.c         |   1 +
> >  sysdeps/x86/tst-ifunc-isa-1.c                | 115 ++++++++++++++++++
> >  sysdeps/x86/tst-ifunc-isa-2-static.c         |   1 +
> >  sysdeps/x86/tst-ifunc-isa-2.c                | 119 +++++++++++++++++++
> >  sysdeps/x86_64/fpu/Makefile                  |   8 ++
> >  22 files changed, 465 insertions(+), 67 deletions(-)
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-2-static.c
> >  create mode 100644 sysdeps/x86/tst-ifunc-isa-2.c
> > 

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 10:25 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> The 01/18/2021 18:37, Adhemerval Zanella wrote:
> > As a side note I tried your branch with a build for all support Linux
> > ABIs and it as least improves by issuing an error on architectures
> > where previously indicated support static-pie but it is broken in practice
> > (powerpc for instance [1])
> >
> > So currently static-pie are support for all architectures, however it
> > fails to build for:
> >
> > alpha-linux-gnu
> > arc-linux-gnuhf
> > hppa-linux-gnu
> > ia64-linux-gnu
> > m68k-linux-gnu
> > microblaze-linux-gnu
> > riscv32-linux-gnu-rv32imafdc-ilp32d
> > riscv64-linux-gnu-rv64imafdc-lp64d
> > s390-linux-gnu
> > sparc64-linux-gnu
> > sparcv9-linux-gnu
> >
> > By requiring PI_STATIC_AND_HIDDEN hppa, m68, microblaze, mips, nios2,
> > and powerpc fail at configure.  Some architecture still fails at build:
> >
> > alpha-linux-gnu
> > arc-linux-gnuhf
> > riscv32-linux-gnu-rv32imafdc-ilp32d
> > riscv64-linux-gnu-rv64imafdc-lp64d
> > s390-linux-gnu
> > sparc64-linux-gnu
> > sparcv9-linux-gnu
> >
> > I haven't checked if mips is currently broken for static-pie (since
> > as for powerpc, it does not define PI_STATIC_AND_HIDDEN); but I would
> > expect so.
> >
> > It would be good if we could avoid building the broken configuration
> > and warn it on configure, but I don't think this should be a blocker.
> > The NEWS for 2.27 states this features is only supported for x86
> > and aarch64, so I wonder if would be better to just enable it for
> > the supported architectures instead of relying on PI_STATIC_AND_HIDDEN.
>
> i randomly checked alpha, which fails while linking sln:
>
> elf/dl-reloc-static-pie.c:40: undefined reference to `_DYNAMIC'
>
> i think this should be possible to configure test in case others
> targets fail in a similar way.
>

Linker must be fixed to support static PIE:

https://sourceware.org/bugzilla/show_bug.cgi?id=22269
https://sourceware.org/bugzilla/show_bug.cgi?id=22263
https://sourceware.org/bugzilla/show_bug.cgi?id=21252

-- 
H.J.

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

On 19/01/2021 16:41, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 10:25 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> The 01/18/2021 18:37, Adhemerval Zanella wrote:
>>> As a side note I tried your branch with a build for all support Linux
>>> ABIs and it as least improves by issuing an error on architectures
>>> where previously indicated support static-pie but it is broken in practice
>>> (powerpc for instance [1])
>>>
>>> So currently static-pie are support for all architectures, however it
>>> fails to build for:
>>>
>>> alpha-linux-gnu
>>> arc-linux-gnuhf
>>> hppa-linux-gnu
>>> ia64-linux-gnu
>>> m68k-linux-gnu
>>> microblaze-linux-gnu
>>> riscv32-linux-gnu-rv32imafdc-ilp32d
>>> riscv64-linux-gnu-rv64imafdc-lp64d
>>> s390-linux-gnu
>>> sparc64-linux-gnu
>>> sparcv9-linux-gnu
>>>
>>> By requiring PI_STATIC_AND_HIDDEN hppa, m68, microblaze, mips, nios2,
>>> and powerpc fail at configure.  Some architecture still fails at build:
>>>
>>> alpha-linux-gnu
>>> arc-linux-gnuhf
>>> riscv32-linux-gnu-rv32imafdc-ilp32d
>>> riscv64-linux-gnu-rv64imafdc-lp64d
>>> s390-linux-gnu
>>> sparc64-linux-gnu
>>> sparcv9-linux-gnu
>>>
>>> I haven't checked if mips is currently broken for static-pie (since
>>> as for powerpc, it does not define PI_STATIC_AND_HIDDEN); but I would
>>> expect so.
>>>
>>> It would be good if we could avoid building the broken configuration
>>> and warn it on configure, but I don't think this should be a blocker.
>>> The NEWS for 2.27 states this features is only supported for x86
>>> and aarch64, so I wonder if would be better to just enable it for
>>> the supported architectures instead of relying on PI_STATIC_AND_HIDDEN.
>>
>> i randomly checked alpha, which fails while linking sln:
>>
>> elf/dl-reloc-static-pie.c:40: undefined reference to `_DYNAMIC'
>>
>> i think this should be possible to configure test in case others
>> targets fail in a similar way.
>>
> 
> Linker must be fixed to support static PIE:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=22269
> https://sourceware.org/bugzilla/show_bug.cgi?id=22263
> https://sourceware.org/bugzilla/show_bug.cgi?id=21252

My question is which is the correct way to check at configure time
for this support? Currently this patchset added the PI_STATIC_AND_HIDDEN,
which is set by each configure snipper within glibc.

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[H.J. Lu via Libc-alpha]

On Tue, Jan 19, 2021 at 12:16 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 19/01/2021 16:41, H.J. Lu wrote:
> > On Tue, Jan 19, 2021 at 10:25 AM Szabolcs Nagy via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> The 01/18/2021 18:37, Adhemerval Zanella wrote:
> >>> As a side note I tried your branch with a build for all support Linux
> >>> ABIs and it as least improves by issuing an error on architectures
> >>> where previously indicated support static-pie but it is broken in practice
> >>> (powerpc for instance [1])
> >>>
> >>> So currently static-pie are support for all architectures, however it
> >>> fails to build for:
> >>>
> >>> alpha-linux-gnu
> >>> arc-linux-gnuhf
> >>> hppa-linux-gnu
> >>> ia64-linux-gnu
> >>> m68k-linux-gnu
> >>> microblaze-linux-gnu
> >>> riscv32-linux-gnu-rv32imafdc-ilp32d
> >>> riscv64-linux-gnu-rv64imafdc-lp64d
> >>> s390-linux-gnu
> >>> sparc64-linux-gnu
> >>> sparcv9-linux-gnu
> >>>
> >>> By requiring PI_STATIC_AND_HIDDEN hppa, m68, microblaze, mips, nios2,
> >>> and powerpc fail at configure.  Some architecture still fails at build:
> >>>
> >>> alpha-linux-gnu
> >>> arc-linux-gnuhf
> >>> riscv32-linux-gnu-rv32imafdc-ilp32d
> >>> riscv64-linux-gnu-rv64imafdc-lp64d
> >>> s390-linux-gnu
> >>> sparc64-linux-gnu
> >>> sparcv9-linux-gnu
> >>>
> >>> I haven't checked if mips is currently broken for static-pie (since
> >>> as for powerpc, it does not define PI_STATIC_AND_HIDDEN); but I would
> >>> expect so.
> >>>
> >>> It would be good if we could avoid building the broken configuration
> >>> and warn it on configure, but I don't think this should be a blocker.
> >>> The NEWS for 2.27 states this features is only supported for x86
> >>> and aarch64, so I wonder if would be better to just enable it for
> >>> the supported architectures instead of relying on PI_STATIC_AND_HIDDEN.
> >>
> >> i randomly checked alpha, which fails while linking sln:
> >>
> >> elf/dl-reloc-static-pie.c:40: undefined reference to `_DYNAMIC'
> >>
> >> i think this should be possible to configure test in case others
> >> targets fail in a similar way.
> >>
> >
> > Linker must be fixed to support static PIE:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=22269
> > https://sourceware.org/bugzilla/show_bug.cgi?id=22263
> > https://sourceware.org/bugzilla/show_bug.cgi?id=21252
>
> My question is which is the correct way to check at configure time
> for this support? Currently this patchset added the PI_STATIC_AND_HIDDEN,
> which is set by each configure snipper within glibc.

Add and define SUPPORT_STATIC_PIE for x86 and aarch64.   Other
targets can opt-in.

-- 
H.J.

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

On 19/01/2021 18:38, H.J. Lu wrote:
> On Tue, Jan 19, 2021 at 12:16 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 19/01/2021 16:41, H.J. Lu wrote:
>>> On Tue, Jan 19, 2021 at 10:25 AM Szabolcs Nagy via Libc-alpha
>>> <libc-alpha@sourceware.org> wrote:
>>>>
>>>> The 01/18/2021 18:37, Adhemerval Zanella wrote:
>>>>> As a side note I tried your branch with a build for all support Linux
>>>>> ABIs and it as least improves by issuing an error on architectures
>>>>> where previously indicated support static-pie but it is broken in practice
>>>>> (powerpc for instance [1])
>>>>>
>>>>> So currently static-pie are support for all architectures, however it
>>>>> fails to build for:
>>>>>
>>>>> alpha-linux-gnu
>>>>> arc-linux-gnuhf
>>>>> hppa-linux-gnu
>>>>> ia64-linux-gnu
>>>>> m68k-linux-gnu
>>>>> microblaze-linux-gnu
>>>>> riscv32-linux-gnu-rv32imafdc-ilp32d
>>>>> riscv64-linux-gnu-rv64imafdc-lp64d
>>>>> s390-linux-gnu
>>>>> sparc64-linux-gnu
>>>>> sparcv9-linux-gnu
>>>>>
>>>>> By requiring PI_STATIC_AND_HIDDEN hppa, m68, microblaze, mips, nios2,
>>>>> and powerpc fail at configure.  Some architecture still fails at build:
>>>>>
>>>>> alpha-linux-gnu
>>>>> arc-linux-gnuhf
>>>>> riscv32-linux-gnu-rv32imafdc-ilp32d
>>>>> riscv64-linux-gnu-rv64imafdc-lp64d
>>>>> s390-linux-gnu
>>>>> sparc64-linux-gnu
>>>>> sparcv9-linux-gnu
>>>>>
>>>>> I haven't checked if mips is currently broken for static-pie (since
>>>>> as for powerpc, it does not define PI_STATIC_AND_HIDDEN); but I would
>>>>> expect so.
>>>>>
>>>>> It would be good if we could avoid building the broken configuration
>>>>> and warn it on configure, but I don't think this should be a blocker.
>>>>> The NEWS for 2.27 states this features is only supported for x86
>>>>> and aarch64, so I wonder if would be better to just enable it for
>>>>> the supported architectures instead of relying on PI_STATIC_AND_HIDDEN.
>>>>
>>>> i randomly checked alpha, which fails while linking sln:
>>>>
>>>> elf/dl-reloc-static-pie.c:40: undefined reference to `_DYNAMIC'
>>>>
>>>> i think this should be possible to configure test in case others
>>>> targets fail in a similar way.
>>>>
>>>
>>> Linker must be fixed to support static PIE:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=22269
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=22263
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=21252
>>
>> My question is which is the correct way to check at configure time
>> for this support? Currently this patchset added the PI_STATIC_AND_HIDDEN,
>> which is set by each configure snipper within glibc.
> 
> Add and define SUPPORT_STATIC_PIE for x86 and aarch64.   Other
> targets can opt-in.

I was expecting a way without an extra flag, but I think for now it
should be suffice.

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/20/2021 08:29, Adhemerval Zanella wrote:
> On 19/01/2021 18:38, H.J. Lu wrote:
> > On Tue, Jan 19, 2021 at 12:16 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >> On 19/01/2021 16:41, H.J. Lu wrote:
> >>> Linker must be fixed to support static PIE:
> >>>
> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=22269
> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=22263
> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=21252
> >>
> >> My question is which is the correct way to check at configure time
> >> for this support? Currently this patchset added the PI_STATIC_AND_HIDDEN,
> >> which is set by each configure snipper within glibc.
> > 
> > Add and define SUPPORT_STATIC_PIE for x86 and aarch64.   Other
> > targets can opt-in.
> 
> I was expecting a way without an extra flag, but I think for now it
> should be suffice.

i can add the flag but when a target adds support there
will be no check if the used linker is new enough.

Re: [PATCH v4 00/10] fix ifunc with static pie [BZ #27072]

[H.J. Lu via Libc-alpha]

On Wed, Jan 20, 2021 at 4:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/20/2021 08:29, Adhemerval Zanella wrote:
> > On 19/01/2021 18:38, H.J. Lu wrote:
> > > On Tue, Jan 19, 2021 at 12:16 PM Adhemerval Zanella
> > > <adhemerval.zanella@linaro.org> wrote:
> > >> On 19/01/2021 16:41, H.J. Lu wrote:
> > >>> Linker must be fixed to support static PIE:
> > >>>
> > >>> https://sourceware.org/bugzilla/show_bug.cgi?id=22269
> > >>> https://sourceware.org/bugzilla/show_bug.cgi?id=22263
> > >>> https://sourceware.org/bugzilla/show_bug.cgi?id=21252
> > >>
> > >> My question is which is the correct way to check at configure time
> > >> for this support? Currently this patchset added the PI_STATIC_AND_HIDDEN,
> > >> which is set by each configure snipper within glibc.
> > >
> > > Add and define SUPPORT_STATIC_PIE for x86 and aarch64.   Other
> > > targets can opt-in.
> >
> > I was expecting a way without an extra flag, but I think for now it
> > should be suffice.
>
> i can add the flag but when a target adds support there
> will be no check if the used linker is new enough.

The minimum link should work for x86 and aarch64.   But if linker fixes
are needed for other targets, they should add the linker check.

-- 
H.J.