[PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[slyich]

From: Sergei Trofimovich <siarheit@google.com>

elf_machine_lazy_rel() used to support _NONE relocation
since it's initial merge:

  commit 01f3e03bcd992cc2917f5d228e763ef3d65e67ad
  Author: Roland McGrath <roland@gnu.org>
  Date:   Tue Feb 13 09:26:53 1996 +0000

Then it was (accidentally?) removed in

  commit 96e1bff2513873062233a13c7fd1eea57bb8db24
  Author: Roland McGrath <roland@gnu.org>
  Date:   Wed Jul 17 18:00:33 1996 +0000

I've noticed this error when tested ghc-HEAD dynamic executable
in qemu:

  ghc-m68k $ LD_DEBUG=reloc bash -x inplace/bin/ghc-stage2 --info
  ...
  5599:     relocation processing: .../compiler/stage2/build/libHSghc-8.1-ghc8.1.20161028.so (lazy)
  5599:     relocation processing: /lib/libncurses.so.6 (lazy)
  5599:     relocation processing: .../libraries/terminfo/dist-install/build/libHSterminfo-0.4.0.2-ghc8.1.20161028.so (lazy)
  5599:     relocation processing: .../libraries/haskeline/dist-install/build/libHShaskeline-0.7.2.3-ghc8.1.20161028.so (lazy)
  5599:     relocation processing: .../inplace/lib/bin/ghc-stage2 (lazy)
  .../inplace/lib/bin/ghc-stage2: error while loading shared libraries: unexpected PLT reloc type 0x00

ghc-HEAD used to work in March 2016.

  $ m68k-unknown-linux-gnu-objdump -rR inplace/lib/bin/ghc-stage2

  DYNAMIC RELOCATION RECORDS
  OFFSET   TYPE              VALUE
  00000000 R_68K_NONE        *ABS*
  00000000 R_68K_NONE        *ABS*
  ...

https://lists.debian.org/debian-68k/2003/02/msg00009.html
suggests it's somewhat known problem in bintuils.

I don't know yet how to force toolchain to binaries with
such relocations.

This change makes ghc-stage2 load and run fine on the following
toolchain version:

    gcc-5.4.0
    binutils-2.26.1
    glibc-2.23

CC: Andreas Schwab <schwab@linux-m68k.org>
CC: Roland McGrath <roland@hack.frob.com>

Signed-off-by: Sergei Trofimovich <siarheit@google.com>
---
 sysdeps/m68k/dl-machine.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sysdeps/m68k/dl-machine.h b/sysdeps/m68k/dl-machine.h
index 41c179c..1b6b6a4 100644
--- a/sysdeps/m68k/dl-machine.h
+++ b/sysdeps/m68k/dl-machine.h
@@ -312,6 +312,8 @@ elf_machine_lazy_rel (struct link_map *map,
   Elf32_Addr *const reloc_addr = (void *) (l_addr + reloc->r_offset);
   if (ELF32_R_TYPE (reloc->r_info) == R_68K_JMP_SLOT)
     *reloc_addr += l_addr;
+  else if (ELF32_R_TYPE (reloc->r_info) == R_68K_NONE)
+    return;
   else
     _dl_reloc_bad_type (map, ELF32_R_TYPE (reloc->r_info), 1);
 }
-- 
2.10.1

Re: [PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[Andreas Schwab]

On Okt 29 2016, slyich@gmail.com wrote:

>   commit 96e1bff2513873062233a13c7fd1eea57bb8db24
>   Author: Roland McGrath <roland@gnu.org>
>   Date:   Wed Jul 17 18:00:33 1996 +0000

??? This commit touches csu/initfini.c only.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[Sergei Trofimovich]

On Sat, 29 Oct 2016 17:37:08 +0200
Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Okt 29 2016, slyich@gmail.com wrote:
> 
> >   commit 96e1bff2513873062233a13c7fd1eea57bb8db24
> >   Author: Roland McGrath <roland@gnu.org>
> >   Date:   Wed Jul 17 18:00:33 1996 +0000  
> 
> ??? This commit touches csu/initfini.c only.
> 
> Andreas.

I was looking at this tree:

Commit https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=96e1bff2513873062233a13c7fd1eea57bb8db24
among other thing shows disappearance of 'case R_68K_NONE':

-static inline void
-elf_machine_lazy_rel (struct link_map *map, const Elf32_Rela *reloc)
-{
-  Elf32_Addr *const reloc_addr = (void *) (map->l_addr + reloc->r_offset);
-  switch (ELF32_R_TYPE (reloc->r_info))
-    {
-    case R_68K_NONE:
-      break;
-    case R_68K_JMP_SLOT:
-      *reloc_addr += map->l_addr;
-      break;
-    default:
-      assert (! "unexpected PLT reloc type");
-      break;
-    }
-}

+static inline void
+elf_machine_lazy_rel (struct link_map *map, const Elf32_Rela *reloc)
+{
+  Elf32_Addr *const reloc_addr = (void *) (map->l_addr + reloc->r_offset);
+  switch (ELF32_R_TYPE (reloc->r_info))
+    {
+    case R_68K_JMP_SLOT:
+      *reloc_addr += map->l_addr;
+      break;
+    default:
+      assert (! "unexpected PLT reloc type");
+      break;
+    }
+}

-- 

  Sergei

Re: [PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[Andreas Schwab]

On Okt 29 2016, Sergei Trofimovich <slyich@gmail.com> wrote:

> Commit https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=96e1bff2513873062233a13c7fd1eea57bb8db24
> among other thing shows disappearance of 'case R_68K_NONE':

The old history is mostly broken, you need to look at
<http://repo.or.cz/w/glibc/history.git>.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[Sergei Trofimovich]

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

On Sat, 29 Oct 2016 19:11:13 +0200
Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Okt 29 2016, Sergei Trofimovich <slyich@gmail.com> wrote:
> 
> > Commit https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=96e1bff2513873062233a13c7fd1eea57bb8db24
> > among other thing shows disappearance of 'case R_68K_NONE':  
> 
> The old history is mostly broken, you need to look at
> <http://repo.or.cz/w/glibc/history.git>.

Same removal:
    http://repo.or.cz/glibc/history.git/commitdiff/60f0b5f25d09c1ee464141ee41995d9a31a604b7

binutils accidentally generates R_68K_NONE relocations around here
    https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=bfd/elf32-m68k.c;h=7c2e0fcca3bfb35d71dc1431c69958c84f04de7b;hb=690035b1471a58db62ec27d739f124f0f58f6af2#l3356
[bfd fails to properly estimate final relocation section size and pads with zeros]

    3356       /* Allocate memory for the section contents.  */
    3357       /* FIXME: This should be a call to bfd_alloc not bfd_zalloc.
    3358          Unused entries should be reclaimed before the section's contents
    3359          are written out, but at the moment this does not happen.  Thus in
    3360          order to prevent writing out garbage, we initialise the section's
    3361          contents to zero.  */
    3362       s->contents = (bfd_byte *) bfd_zalloc (dynobj, s->size);

Curious can add "memset(s->contents, '\xFF', s->size);" to make sure
it still happens.

And indeed m68k-linux-gcc generates R_68K_NONE relocations even for
minimal programs:

    $ echo 'int main(){}' > a.c
    $ m68k-unknown-linux-gnu-gcc a.c -o a
    $ readelf -a a | grep -C4 NONE
     0x00000000 (NULL)                       0x0

    Relocation section '.rela.dyn' at offset 0x238 contains 3 entries:
     Offset     Info    Type            Sym.Value  Sym. Name + Addend
    00000000  00000000 R_68K_NONE                   0
    00000000  00000000 R_68K_NONE                   0
    80004014  00000114 R_68K_GLOB_DAT    00000000   __gmon_start__ + 0

In general relocations happen to be non-lazy.

I guess to trigger lazy case we need to build shared library with complex library
structure. At least GHC does that.

Is there a reason to allow R_68K_NONE for non-lazy relocations
in elf_machine_rela()

+    case R_68K_NONE:           /* Alright, Wilbur.  */
+      break;

but forbid R_68K_NONE for lazy relocations
in elf_machine_lazy_rel()

assuming mechanism to leak R_68K_NONE to both relocation types is the same?

Thanks!

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

Re: [PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[Andreas Schwab]

On Okt 30 2016, Sergei Trofimovich <slyich@gmail.com> wrote:

> Is there a reason to allow R_68K_NONE for non-lazy relocations
> in elf_machine_rela()
>
> +    case R_68K_NONE:           /* Alright, Wilbur.  */
> +      break;
>
> but forbid R_68K_NONE for lazy relocations
> in elf_machine_lazy_rel()

Because the latter must never happen.  Please file a binutils bug with a
self-contained test case.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] m68k: restore handling of R_68K_NONE in elf_machine_lazy_rel()

[Sergei Trofimovich]

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

On Sun, 30 Oct 2016 12:04:59 +0100
Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Okt 30 2016, Sergei Trofimovich <slyich@gmail.com> wrote:
> 
> > Is there a reason to allow R_68K_NONE for non-lazy relocations
> > in elf_machine_rela()
> >
> > +    case R_68K_NONE:           /* Alright, Wilbur.  */
> > +      break;
> >
> > but forbid R_68K_NONE for lazy relocations
> > in elf_machine_lazy_rel()  
> 
> Because the latter must never happen.  Please file a binutils bug with a
> self-contained test case.

Finally extracted one. Filed as:
    https://sourceware.org/bugzilla/show_bug.cgi?id=20796

It is triggered by a mix of -Wl,--gc-sections, -u buggy_symbol
and a shared library where 'buggy_symbol' comes from.

And while at it filed minor bug about R_68K_NONE relocations
to be present in final binaries:
    https://sourceware.org/bugzilla/show_bug.cgi?id=20797

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 163 bytes --]