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

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

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.