Re: [PATCH] Avoid GOT relocations in ld.so on powerpc

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Wed, May 16, 2012 at 6:01 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> This removes GOT relocations in ld.so referring to dl_argv, _rtld_global
> and _rtld_global_ro, by using their internal aliases instead.
>
> Andreas.
>
> ? ? ? ?* sysdeps/powerpc/powerpc32/dl-start.S (_dl_start_user): Use
> ? ? ? ?INTUSE on _dl_argv, and _rtld_local instead of _rtld_global.
> ? ? ? ?* sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S [IS_IN_rtld]:
> ? ? ? ?Refer to _rtld_local_ro instead of _rtld_global_ro.
> ? ? ? ?* sysdeps/powerpc/powerpc32/fpu/setjmp-common.S [IS_IN_rtld]:
> ? ? ? ?Likewise.
> ? ? ? ?* sysdeps/powerpc/powerpc64/__longjmp-common.S [IS_IN_rtld]:
> ? ? ? ?Likewise.
> ? ? ? ?* sysdeps/powerpc/powerpc64/setjmp-common.S [IS_IN_rtld]:
> ? ? ? ?Likewise.
> ? ? ? ?* sysdeps/powerpc/powerpc64/dl-trampoline.S [SHARED]: Likewise.
> ? ? ? ?* sysdeps/powerpc/powerpc64/dl-machine.h: Use _rtld_local instead
> ? ? ? ?of _rtld_global, and rtld_progname instead of _dl_argv[0].
> ---
> ?sysdeps/powerpc/powerpc32/dl-start.S ? ? ? ? ? ? | ? ?7 +++----
> ?sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S | ? ?7 +++++--
> ?sysdeps/powerpc/powerpc32/fpu/setjmp-common.S ? ?| ? ?6 +++++-
> ?sysdeps/powerpc/powerpc64/__longjmp-common.S ? ? | ? ?6 +++++-
> ?sysdeps/powerpc/powerpc64/dl-machine.h ? ? ? ? ? | ? ?6 +++---
> ?sysdeps/powerpc/powerpc64/dl-trampoline.S ? ? ? ?| ? ?4 ++--
> ?sysdeps/powerpc/powerpc64/setjmp-common.S ? ? ? ?| ? ?6 +++++-
> ?7 files changed, 28 insertions(+), 14 deletions(-)

Looks good to me, a couple of minor nits.

We should be consistent about the use of _rtld_local in comments and
derivative symbol names e.g. __rtld_local.

> diff --git a/sysdeps/powerpc/powerpc32/dl-start.S b/sysdeps/powerpc/powerpc32/dl-start.S
> index 2546fa5..b2d0194 100644
> --- a/sysdeps/powerpc/powerpc32/dl-start.S
> +++ b/sysdeps/powerpc/powerpc32/dl-start.S
> @@ -1,6 +1,5 @@
> ?/* Machine-dependent ELF startup code. ?PowerPC version.
> - ? Copyright (C) 1995-2000, 2002, 2004, 2005, 2006, 2011
> - ? ? ? Free Software Foundation, Inc.
> + ? Copyright (C) 1995-2012 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
> @@ -53,9 +52,9 @@ _dl_start_user:
> ?/* ?the address of _start in r30, */
> ? ? ? ?mr ? ? ?r30,r3
> ?/* ?&_dl_argc in 29, &_dl_argv in 27, and _dl_loaded in 28. ?*/
> - ? ? ? lwz ? ? r28,_rtld_global@got(r31)
> + ? ? ? lwz ? ? r28,_rtld_local@got(r31)
> ? ? ? ?lwz ? ? r29,_dl_argc@got(r31)
> - ? ? ? lwz ? ? r27,_dl_argv@got(r31)
> + ? ? ? lwz ? ? r27,INTUSE(_dl_argv)@got(r31)
>
> ?/* Call _dl_init (_dl_loaded, _dl_argc, _dl_argv, _dl_argv+_dl_argc+1). */
> ? ? ? ?lwz ? ? r3,0(r28)
> diff --git a/sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S b/sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S
> index 4cfde6b..8246e45 100644
> --- a/sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S
> +++ b/sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S
> @@ -1,6 +1,5 @@
> ?/* longjmp for PowerPC.
> - ? Copyright (C) 1995-99, 2000, 2003-2006, 2009, 2011
> - ? ? ? Free Software Foundation, Inc.
> + ? Copyright (C) 1995-2012 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
> @@ -38,7 +37,11 @@ ENTRY (BP_SYM (__longjmp))
> ? ? ? ?addis ? r5,r5,_GLOBAL_OFFSET_TABLE_-got_label@ha
> ? ? ? ?addi ? ?r5,r5,_GLOBAL_OFFSET_TABLE_-got_label@l
> ?# ?ifdef SHARED
> +# ? ifdef IS_IN_rtld
> + ? ? ? lwz ? ? r5,_rtld_local_ro@got(r5)
> +# ? else
> ? ? ? ?lwz ? ? r5,_rtld_global_ro@got(r5)
> +# ? endif
> ? ? ? ?mtlr ? ?r6
> ? ? ? ?cfi_same_value (lr)
> ? ? ? ?lwz ? ? r5,RTLD_GLOBAL_RO_DL_HWCAP_OFFSET+4(r5)
> diff --git a/sysdeps/powerpc/powerpc32/fpu/setjmp-common.S b/sysdeps/powerpc/powerpc32/fpu/setjmp-common.S
> index b3c9f56..8b7bc60 100644
> --- a/sysdeps/powerpc/powerpc32/fpu/setjmp-common.S
> +++ b/sysdeps/powerpc/powerpc32/fpu/setjmp-common.S
> @@ -1,5 +1,5 @@
> ?/* setjmp for PowerPC.
> - ? Copyright (C) 1995-2000, 2003-2005, 2006, 2011 Free Software Foundation, Inc.
> + ? Copyright (C) 1995-2012 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
> @@ -90,7 +90,11 @@ ENTRY (BP_SYM (__sigsetjmp))
> ? ? ? ?mtlr ? ?r6
> ? ? ? ?cfi_same_value (lr)
> ?# ?ifdef SHARED
> +# ? ifdef IS_IN_rtld
> + ? ? ? lwz ? ? r5,_rtld_local_ro@got(r5)
> +# ? else
> ? ? ? ?lwz ? ? r5,_rtld_global_ro@got(r5)
> +# ? endif
> ? ? ? ?lwz ? ? r5,RTLD_GLOBAL_RO_DL_HWCAP_OFFSET+4(r5)
> ?# ?else
> ? ? ? ?lwz ? ? r5,_dl_hwcap@got(r5)
> diff --git a/sysdeps/powerpc/powerpc64/__longjmp-common.S b/sysdeps/powerpc/powerpc64/__longjmp-common.S
> index 2ff9078..3804b39 100644
> --- a/sysdeps/powerpc/powerpc64/__longjmp-common.S
> +++ b/sysdeps/powerpc/powerpc64/__longjmp-common.S
> @@ -1,5 +1,5 @@
> ?/* longjmp for PowerPC64.
> - ? Copyright (C) 1995, 1996,1997,1999-2006,2009 Free Software Foundation, Inc.
> + ? Copyright (C) 1995-2012 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
> @@ -31,7 +31,11 @@
> ? ? ? ?.section ? ? ? ?".toc","aw"
> ?.LC__dl_hwcap:
> ?# ifdef SHARED
> +# ?ifdef IS_IN_rtld
> + ? ? ? .tc _rtld_local_ro[TC],_rtld_local_ro
> +# ?else
> ? ? ? ?.tc _rtld_global_ro[TC],_rtld_global_ro
> +# ?endif

Same comment as below about RTLD_GLOBAL_RO_DL_HWCAP_OFFSET.

> ?# else
> ? ? ? ?.tc _dl_hwcap[TC],_dl_hwcap
> ?# endif
> diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
> index 7c04757..f0cad57 100644
> --- a/sysdeps/powerpc/powerpc64/dl-machine.h
> +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
> @@ -1,6 +1,6 @@
> ?/* Machine-dependent ELF dynamic relocation inline functions.
> ? ?PowerPC64 version.
> - ? Copyright 1995-2005, 2006, 2008, 2010, 2011 Free Software Foundation, Inc.
> + ? Copyright 1995-2012 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
> @@ -162,7 +162,7 @@ BODY_PREFIX "_start:\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ?" ? ? ?.pushsection ? ?\".toc\",\"aw\"\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ?DL_STARTING_UP_DEF ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ?".LC__rtld_global:\n"

Call it __rtld_local and fixup the reference?
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> -" ? ? ?.tc _rtld_global[TC],_rtld_global\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> +" ? ? ?.tc _rtld_local[TC],_rtld_local\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ?".LC__dl_argc:\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ?" ? ? ?.tc _dl_argc[TC],_dl_argc\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ?".LC__dl_argv:\n" ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> @@ -734,7 +734,7 @@ elf_machine_rela (struct link_map *map,
> ? ? ? ? ?_dl_error_printf ("%s: Symbol `%s' has different size" \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?" in shared object," \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?" consider re-linking\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? _dl_argv[0] ?: "<program name unknown>",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? rtld_progname ?: "<program name unknown>",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?strtab + refsym->st_name);
> ? ? ? ?}
> ? ? ? memcpy (reloc_addr_arg, (char *) value,
> diff --git a/sysdeps/powerpc/powerpc64/dl-trampoline.S b/sysdeps/powerpc/powerpc64/dl-trampoline.S
> index 266efe3..6435f46 100644
> --- a/sysdeps/powerpc/powerpc64/dl-trampoline.S
> +++ b/sysdeps/powerpc/powerpc64/dl-trampoline.S
> @@ -1,5 +1,5 @@
> ?/* PLT trampolines. ?PPC64 version.
> - ? Copyright (C) 2005, 2006 Free Software Foundation, Inc.
> + ? Copyright (C) 2005-2012 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
> @@ -169,7 +169,7 @@ END(_dl_runtime_resolve)
> ? ? ? ?.section ? ? ? ?".toc","aw"
> ?.LC__dl_hwcap:
> ?# ifdef SHARED
> - ? ? ? .tc _rtld_global_ro[TC],_rtld_global_ro
> + ? ? ? .tc _rtld_local_ro[TC],_rtld_local_ro
> ?# else
> ? ? ? ?.tc _dl_hwcap[TC],_dl_hwcap
> ?# endif

Same comment as below about RTLD_GLOBAL_RO_DL_HWCAP_OFFSET.

> diff --git a/sysdeps/powerpc/powerpc64/setjmp-common.S b/sysdeps/powerpc/powerpc64/setjmp-common.S
> index 1a1326e..ab017b7 100644
> --- a/sysdeps/powerpc/powerpc64/setjmp-common.S
> +++ b/sysdeps/powerpc/powerpc64/setjmp-common.S
> @@ -1,5 +1,5 @@
> ?/* setjmp for PowerPC64.
> - ? Copyright (C) 1995-2003, 2004, 2005, 2006 Free Software Foundation, Inc.
> + ? Copyright (C) 1995-2012 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
> @@ -30,7 +30,11 @@
> ? ? ? ?.section ? ? ? ?".toc","aw"
> ?.LC__dl_hwcap:
> ?# ifdef SHARED
> +# ?ifdef IS_IN_rtld
> + ? ? ? .tc _rtld_local_ro[TC],_rtld_local_ro
> +# ?else
> ? ? ? ?.tc _rtld_global_ro[TC],_rtld_global_ro
> +# ?endif
> ?# else
> ? ? ? ?.tc _dl_hwcap[TC],_dl_hwcap
> ?# endif

Could you adjust the comments that use the macro RTLD_GLOBAL_RO_DL_HWCAP_OFFSET?

e.g.

#ifdef SHARED
        /* Load _rtld_local._dl_hwcap. We use the offset
           for _rtld_global because they are the same and
           we already have them computed for the *context
           routines. */
        ld      r12,RTLD_GLOBAL_RO_DL_HWCAP_OFFSET(r12)
#else
        ld      r12,0(r12) /* Load extern _dl_hwcap.  */
#endif

Perhaps that comment is too verbose, but it should at least say "Load
_rtld_local._dl_hwcap."

Cheers,
Carlos.