elf/get-dynamic-info.h: fix early GNU_HASH processing on ia64

  From: Sergei Trofimovich <siarheit@google.com>

The following code when being
 called with l    = _rtld_local._dl_rtld_map
             info = l->l_info

  elf_get_dynamic_info(struct link_map *l,ElfW(Dyn) *dyn) {
    ...
    info[DT_ADDRTAGIDX (dyn->d_tag) + DT_NUM + DT_THISPROCNUM
         + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM] = dyn;
    ...

led to generation of the follwoing assembly code:

    [MMI]       ld8 r14=[r32];;      # r14 = dyn->d_tag
                shladd r15=r14,3,r0  # r15 = dyn->d_tag * 8
                addl r14=163312,r1;; # r14 = gp + @ltoffx(_rtld_local#+0x380000000)
    [MMI]       ld8 r14=[r14];;
                adds r14=992,r14     # r14 = [abs_reloc] + 992
                nop.i 0x0;;
    [MMI]       nop.m 0x0
                sub r14=r14,r15
                nop.i 0x0;;
    [MIB]       st8 [r14]=r32        # [[abs_reloc] + 992] = dyn
                nop.i 0x0
                br.ret.sptk.many b0;;

This 'abs_reloc' is a relocation of R_IA64_REL64LSB type.

objdump -r -R ld.so:
  DYNAMIC RELOCATION RECORDS
  OFFSET           TYPE              VALUE
  ...
  0000000000052910 REL64LSB          *ABS*+0x0000000380052a60

After gcc's preprocessor and constant propagation phase
the code

  info[DT_ADDRTAGIDX (dyn->d_tag) + DT_NUM + DT_THISPROCNUM
       + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM] = dyn;

became equivalent equivalent to:

   _rtld_local._dl_rtld_map.l_info[(0x6ffffeff - dyn->d_tag) + 66]

    (0x6ffffeff + 66) * 8 + 2520 = 0x3800003e0
        # 2520 is offset of '_rtld_local._dl_rtld_map.l_info'
        # 0x3e0 = 992

To workaround generation of that huge offset and relocation
I've moved index computation into a separate variable to trick
gcc into simpler code.

Bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60465#c32
Signed-off-by: Sergei Trofimovich <siarheit@google.com>
---
 elf/get-dynamic-info.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
  

On 26 Dec 2015 19:32, slyich@gmail.com wrote:
> The following code when being
>  called with l    = _rtld_local._dl_rtld_map
>              info = l->l_info
> 
>   elf_get_dynamic_info(struct link_map *l,ElfW(Dyn) *dyn) {
>     ...
>     info[DT_ADDRTAGIDX (dyn->d_tag) + DT_NUM + DT_THISPROCNUM
>          + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM] = dyn;
>     ...
> 
> led to generation of the follwoing assembly code:
> 
>     [MMI]       ld8 r14=[r32];;      # r14 = dyn->d_tag
>                 shladd r15=r14,3,r0  # r15 = dyn->d_tag * 8
>                 addl r14=163312,r1;; # r14 = gp + @ltoffx(_rtld_local#+0x380000000)
>     [MMI]       ld8 r14=[r14];;
>                 adds r14=992,r14     # r14 = [abs_reloc] + 992
>                 nop.i 0x0;;
>     [MMI]       nop.m 0x0
>                 sub r14=r14,r15
>                 nop.i 0x0;;
>     [MIB]       st8 [r14]=r32        # [[abs_reloc] + 992] = dyn
>                 nop.i 0x0
>                 br.ret.sptk.many b0;;
> 
> This 'abs_reloc' is a relocation of R_IA64_REL64LSB type.
> 
> objdump -r -R ld.so:
>   DYNAMIC RELOCATION RECORDS
>   OFFSET           TYPE              VALUE
>   ...
>   0000000000052910 REL64LSB          *ABS*+0x0000000380052a60
> 
> After gcc's preprocessor and constant propagation phase
> the code
> 
>   info[DT_ADDRTAGIDX (dyn->d_tag) + DT_NUM + DT_THISPROCNUM
>        + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM] = dyn;
> 
> became equivalent equivalent to:
> 
>    _rtld_local._dl_rtld_map.l_info[(0x6ffffeff - dyn->d_tag) + 66]
> 
>     (0x6ffffeff + 66) * 8 + 2520 = 0x3800003e0
>         # 2520 is offset of '_rtld_local._dl_rtld_map.l_info'
>         # 0x3e0 = 992
> 
> To workaround generation of that huge offset and relocation
> I've moved index computation into a separate variable to trick
> gcc into simpler code.

i don't think this change is what we want -- you're modifying common code
in a non-obvious way to trick gcc on a specific arch to generate "better"
code.  future versions of gcc (or reasonable code shuffling here) could
easily break this behavior.

from reading your analysis, it seems to come down to:
- ia64 gcc generates a reloc (R_IA64_REL64LSB) in elf_get_dynamic_info
  (specifically, the DT_GNU_HASH type)
- that reloc type isn't processed until after in ELF_DYNAMIC_RELOCATE()
- ELF_DYNAMIC_RELOCATE() relies on the DT info being initialized in
  elf_get_dynamic_info in order to process the relocs
- elf_get_dynamic_info crashes because reloc is uninitialized

this scenario can come up with any of the cases in elf_get_dynamic_info,
not just ones in the DT_ADDRRNG range.  so even this minor reworking of
the code for these values does nothing for the others.

i don't think we can really split up the elf_get_dynamic_info logic into
an "early" and "late" phase as there might be DT sections we need even in
the "early" phase :/.

since we understand the source of the problem now -- rather than gcc being
horribly broken and generating bad code, it's just generating relocs in a
way that upset the ldso bootstrap -- i don't have a problem including this
workaround in Gentoo.  funnily enough, i wrote the same patch almost two
years ago in gcc/60558.  i just didn't want to deploy anything in case the
glibc crash was a harbinger.
-mike
  

On 12/28/2015 06:12 AM, Mike Frysinger wrote:
> i don't think this change is what we want -- you're modifying common code
> in a non-obvious way to trick gcc on a specific arch to generate "better"
> code.  future versions of gcc (or reasonable code shuffling here) could
> easily break this behavior.

Agreed, this looks rather fishy.

Would it be possible to add a GCC option which inhibits it from creating
such relocations, so that the required parts of the dynamic linker can
be compiled with it?

Florian
  

On Mon, 28 Dec 2015 12:57:00 +0100
Florian Weimer <fweimer@redhat.com> wrote:

> On 12/28/2015 06:12 AM, Mike Frysinger wrote:
> > i don't think this change is what we want -- you're modifying common code
> > in a non-obvious way to trick gcc on a specific arch to generate "better"
> > code.  future versions of gcc (or reasonable code shuffling here) could
> > easily break this behavior.
> 
> Agreed, this looks rather fishy.
> 
> Would it be possible to add a GCC option which inhibits it from creating
> such relocations, so that the required parts of the dynamic linker can
> be compiled with it?

I agree the patch is fragile.
I think gcc can be taught to use something like

    movl r32 = @gprel(_rtld_local#+15032385536)
    add  r32 = r32, gp

on ia64. That would (i hope) use statically resolvable
relocation and would not require .got entry.
  

> I think gcc can be taught to use something like
> 
>     movl r32 = @gprel(_rtld_local#+15032385536)
>     add  r32 = r32, gp
> 
> on ia64. That would (i hope) use statically resolvable
> relocation and would not require .got entry.

Something like that:
    https://gcc.gnu.org/bugzilla/attachment.cgi?id=37180&action=diff
  

On Mon, 28 Dec 2015 17:49:01 +0000
Sergei Trofimovich <slyich@gmail.com> wrote:

> > I think gcc can be taught to use something like
> > 
> >     movl r32 = @gprel(_rtld_local#+15032385536)
> >     add  r32 = r32, gp
> > 
> > on ia64. That would (i hope) use statically resolvable
> > relocation and would not require .got entry.

[for email thread completeness] Patch gone in upstream GCC as:
    https://github.com/gcc-mirror/gcc/commit/afe82e5b25dda439d84a99d95b3767d6cced79c8

Thanks!