Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#10402 closed bug (fixed)

powerpc: unhandled ELF relocation(RelA) type 252

Reported by: cjwatson Owned by: simonmar
Priority: normal Milestone: 7.10.2
Component: Runtime System Version: 7.8.4
Keywords: Cc: simonmar, slyfox, trommler
Operating System: Linux Architecture: powerpc
Type of failure: Runtime crash Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D996
Wiki Page:

Description

Building some Debian packages on powerpc with GHC 7.8.4 fails as shown in this build log, e.g.:

  ghc: /usr/lib/haskell-packages/ghc/lib/powerpc-linux-ghc-7.8.4/unix-time-0.3.5/libHSunix-time-0.3.5.a: unhandled ELF relocation(RelA) type 252

I'm no PPC ABI expert, but I think the attached patch against master (which applies equally to 7.8.4) is reasonably plausible, and it at least allows wai-app-static to build cleanly.

Attachments (1)

0001-rts-Linker.c-add-some-more-PPC-relocations-10402.patch (1.3 KB) - added by cjwatson 4 years ago.
rts/Linker.c: add some more PPC relocations (#10402)

Download all attachments as: .zip

Change History (24)

Changed 4 years ago by cjwatson

rts/Linker.c: add some more PPC relocations (#10402)

comment:1 Changed 4 years ago by cjwatson

Apologies for attaching a vim swap file by mistake the first time round! Feel free to delete that if possible.

comment:2 Changed 4 years ago by rwbarton

Looks plausible to me too. Do we assume these new relocation types are a result of a new version of binutils?

comment:3 Changed 4 years ago by cjwatson

I'm not at all sure; it seems like the obvious candidate, but I don't know enough to be able to search the enormous haystack of changelogs for this particular needle. I figured that GHC generally took the approach here of just implementing whichever relocations happened to be needed (the only additional ones I actually saw directly were R_PPC_REL16_HA and R_PPC_PLTREL24, in fact, but the other two were trivial to do at the same time).

comment:4 Changed 4 years ago by rwbarton

No need to dig, just wanted to make sure you didn't know something to the contrary.

And yes, we tend to just implement linker features as needed.

comment:5 Changed 4 years ago by nomeata

Milestone: 7.10.2

What’s the status of this? Can we get it into 7.10.2? The Debian package already carries these patches, they seem to work.

comment:6 Changed 4 years ago by thoughtpolice

Cc: slyfox trommler added
Status: newpatch

(I deleted the .swp file).

I think this patch looks fine but to be safe I would prefer it if we could get a code review from Sergei and/or Peter.

@cjwatson, thanks for the great patch as usual, and if you don't mind, in the future please at least set these tickets to patch status so we don't lose them! (preferably if they could go through Phabricator, we'll also be more attentive and review them in short order).

comment:7 Changed 4 years ago by slyfox

I think the source of new relocations is GHC itself:

a93ab43ab5f40cadbedea2f6342b93c245e91434

It started generating proper PIC files for all (including .c) code recently.

[sf] ~:cat /tmp/a.c
long f();
long g() {
    return f();
}

[sf] /tmp:powerpc64-unknown-linux-gnu-gcc -c -m32 -O2 a.c && objdump -d -r a.o

a.o:     file format elf32-powerpc


Disassembly of section .text:

00000000 <g>:
   0:   94 21 ff f0     stwu    r1,-16(r1)
   4:   38 21 00 10     addi    r1,r1,16
   8:   48 00 00 00     b       8 <g+0x8>
                        8: R_PPC_REL24  f
[sf] /tmp:powerpc64-unknown-linux-gnu-gcc -fPIC -c -m32 -O2 a.c && objdump -d -r a.o

a.o:     file format elf32-powerpc


Disassembly of section .text:

00000000 <g>:
   0:   94 21 ff e0     stwu    r1,-32(r1)
   4:   7c 08 02 a6     mflr    r0
   8:   42 9f 00 05     bcl     20,4*cr7+so,c <g+0xc>
   c:   93 c1 00 18     stw     r30,24(r1)
  10:   90 01 00 24     stw     r0,36(r1)
  14:   7f c8 02 a6     mflr    r30
  18:   3f de 00 00     addis   r30,r30,0
                        1a: R_PPC_REL16_HA      .got2+0x800e
  1c:   3b de 00 00     addi    r30,r30,0
                        1e: R_PPC_REL16_LO      .got2+0x8012
  20:   48 00 00 01     bl      20 <g+0x20>
                        20: R_PPC_PLTREL24      f+0x8000
  24:   80 01 00 24     lwz     r0,36(r1)
  28:   83 c1 00 18     lwz     r30,24(r1)
  2c:   38 21 00 20     addi    r1,r1,32
  30:   7c 08 03 a6     mtlr    r0
  34:   4e 80 00 20     blr

What bothers me is why ghci tries o load .a/.o file manually at all. It should use system linker. by creating/dlopen()ing temp .so object.

Does debian disable dynamic GHC by default as

DYNAMIC_GHC_PROGRAMS = NO

?

From the above I suspect R_PPC_PLTREL24 might require +0x8000 offset and should be PLT relative. I'll try to look at it tomorrow.

Last edited 4 years ago by slyfox (previous) (diff)

comment:8 Changed 4 years ago by nomeata

Does debian disable dynamic GHC by default as DYNAMIC_GHC_PROGRAMS = NO?

No (ldd /usr/lib/ghc/bin/ghc lists various libHS... libraries).

comment:9 in reply to:  7 Changed 4 years ago by trommler

Replying to slyfox:

From the above I suspect R_PPC_PLTREL24 might require +0x8000 offset and should be PLT relative. I'll try to look at it tomorrow.

From the specs p 108: This relocation indicates that reference to a symbol should be resolved through a call to the symbol’s Procedure Linkage Table entry. Additionally it instructs the link editor to build a procedure linkage table for the executable or shared object if one is not created. and under heading ATR-BSS-PLT: Under the BSS-PLT ABI this relocation type may be implemented as a direct branch and link into the executable PLT slot which holds the absolute address (after resolution) of the specified symbol. There is an implicit assumption that the Procedure Linkage Table for a shared object or executable will be within ± 32 MB of an instruction that branches to it.

I guess that debian is doing the latter so no PLT entry needs to be generated by the linker which would be rts/Linker.c.

Last edited 4 years ago by trommler (previous) (diff)

comment:10 in reply to:  8 Changed 4 years ago by trommler

Replying to nomeata:

Does debian disable dynamic GHC by default as DYNAMIC_GHC_PROGRAMS = NO?

No (ldd /usr/lib/ghc/bin/ghc lists various libHS... libraries).

Are you using secure PLT? This might be done as part of hardening.

In the ABI specs p 108 relocations R_PPC_REL16*: These relocation types are used to compute the distance between a symbol address and the current address. These relocations types are used under the Secure-PLT ABI to compute the address of the .got section because the link editor knows the fixed distance between the _GLOBAL_OFFSET_TABLE_ symbol and an address in the .text section.

The specs referred to above is: Power Architecture® 32-bit Application Binary Interface Supplement 1.0 - Linux® & Embedded

comment:11 in reply to:  6 Changed 4 years ago by trommler

Replying to thoughtpolice:

(I deleted the .swp file).

I think this patch looks fine but to be safe I would prefer it if we could get a code review from Sergei and/or Peter.

The code looks fine to me.

I think it is safe to commit the code. The code in the patch will be called only if those relocations are actually present in an ELF object which is apparently the case on debian.

If you like me to double check I could apply the patch to my local ghc package on openSUSE's build service and report back here.

comment:12 in reply to:  8 Changed 4 years ago by slyfox

Replying to nomeata:

Does debian disable dynamic GHC by default as DYNAMIC_GHC_PROGRAMS = NO?

No (ldd /usr/lib/ghc/bin/ghc lists various libHS... libraries).

Do I do something incorrectly? (or the test was on non-powerpc?)

# debootstrap --arch=powerpc --include=ghc unstable .
# chroot .
# ghc --version
The Glorious Glasgow Haskell Compilation System, version 7.8.4
# ldd /usr/lib/ghc/bin/ghc
        linux-vdso32.so.1 (0x00100000)
        libtinfo.so.5 => /lib/powerpc-linux-gnu/libtinfo.so.5 (0x0ffaf000)
        librt.so.1 => /lib/powerpc-linux-gnu/librt.so.1 (0x0ff86000)
        libutil.so.1 => /lib/powerpc-linux-gnu/libutil.so.1 (0x0ff63000)
        libdl.so.2 => /lib/powerpc-linux-gnu/libdl.so.2 (0x0ff3f000)
        libpthread.so.0 => /lib/powerpc-linux-gnu/libpthread.so.0 (0x0ff04000)
        libgmp.so.10 => /usr/lib/powerpc-linux-gnu/libgmp.so.10 (0x0fe69000)
        libm.so.6 => /lib/powerpc-linux-gnu/libm.so.6 (0x0fd93000)
        libffi.so.6 => /usr/lib/powerpc-linux-gnu/libffi.so.6 (0x0fd6a000)
        libc.so.6 => /lib/powerpc-linux-gnu/libc.so.6 (0x0fbcb000)
        /lib/ld.so.1 (0x2052a000)
# ghc --info | grep -i dyn
 ,("Support dynamic-too","YES")
 ,("Dynamic by default","NO")
 ,("GHC Dynamic","NO")

It's not able to handle PIC:

// cat a.c 
#include <stdio.h>

void ffi_a_hello (int i) {
    fprintf (stderr, "WEEEEEEEE: i=%d\n", i);
}

-- cat A.hs
{-# LANGUAGE ForeignFunctionInterface #-}
module A where

import Foreign.C

foreign import ccall "ffi_a_hello" a :: CInt -> IO ()

# ghc -fPIC -c a.c -fforce-recomp
# ghc -fPIC -c A.hs -fforce-recomp
# ghci --interactive ./a.o A
GHCi, version 7.8.4: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading object (static) a.o ... done
final link ... done
[1 of 1] Compiling A                ( A.hs, interpreted ) [flags changed]
Ok, modules loaded: A.
*A> a 42
Segmentation fault

Non-PIC is fine, but it does not contain PLT-style relocations:

# ghc -c a.c -fforce-recomp
# ghc -c A.hs -fforce-recomp
# ghci --interactive ./a.o A
GHCi, version 7.8.4: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading object (static) a.o ... done
final link ... done
[1 of 1] Compiling A                ( A.hs, interpreted ) [flags changed]
Ok, modules loaded: A.
*A> a 42
WEEEEEEEE: i=42
*A> 
Leaving GHCi.

# objdump -d -r a.o | grep PLT
# objdump -d -r A.o | grep PLT

comment:13 Changed 4 years ago by nomeata

Do I do something incorrectly? (or the test was on non-powerpc?)

Sorry, was on non-powerpc. I only checked on arm64 and checked that we did not change any settings per architecture here, but it seems that ghc by default builds GHC without dynamic libraries?

comment:14 Changed 4 years ago by slyfox

Having written that I wondered who forces -fPIC on you and the answer is obvious! Rogue package:

$ cabal unpack unix-time
Unpacking to unix-time-0.3.5/

$ grep -R -C4 -i fPIC unix-time-0.3.5/

unix-time-0.3.5/unix-time.cabal-  if impl(ghc >= 7.8)
unix-time-0.3.5/unix-time.cabal:    CC-Options:         -fPIC

comment:15 Changed 4 years ago by rwbarton

While it would doubtless be better to build with -fPIC only when building a shared library, we ought to support the configuration of unix-time too, right?

comment:16 in reply to:  13 Changed 4 years ago by slyfox

Replying to nomeata:

Do I do something incorrectly? (or the test was on non-powerpc?)

Sorry, was on non-powerpc. I only checked on arm64 and checked that we did not change any settings per architecture here, but it seems that ghc by default builds GHC without dynamic libraries?

Yeah, there is a blacklist of bad platforms:

# apt-get source ghc

# grep -A10 NoSharedLibsPlatformList ghc-7.8.4/mk/config.mk.in 
NoSharedLibsPlatformList = powerpc-unknown-linux \
        x86_64-unknown-mingw32 \
        i386-unknown-mingw32 \
        sparc-sun-solaris2 \
        sparc-unknown-linux \
        mips-unknown-linux \
        mipsel-unknown-linux

On 7.10 it does not contain linux targets.

I've completely forgot that upstream ghc was released before PPC -fPIC fix went upstream fa31e8f4a0f853848d96549a429083941877bf8d after 7.8.4 release :( You might like to apply this as well 78863edbb0751f5c9694ea10c6132a87cfd0ee10

comment:17 in reply to:  15 Changed 4 years ago by slyfox

Replying to rwbarton:

While it would doubtless be better to build with -fPIC only when building a shared library, we ought to support the configuration of unix-time too, right?

Correct.

  1. ghc-7.10/HEAD/DYNAMIC_GHC_PROGRAMS=YES on PPC32 fully supports it by using dynamic linker
  2. ghc-7.10/HEAD/DYNAMIC_GHC_PROGRAMS=NO on PPC32 does not support it and attached patch needs to be adjusted a bit to work.

comment:18 Changed 4 years ago by slyfox

Differential Rev(s): Phab:D996

comment:19 Changed 4 years ago by Austin Seipp <austin@…>

In c0847967caf51ea4ca88d0ffc25fe1bd99dcabed/ghc:

powerpc: add basic support for PLT relocations (#10402)

Commit a93ab43ab5f40cadbedea2f6342b93c245e91434
enabled support for proper PIC relocations from
assembler.

Commit adds support for relocations of type:
    R_PPC_REL16_HI
    R_PPC_REL16_HA
    R_PPC_REL16_LO
    R_PPC_PLTREL24

They are used only when GHC is built in
    DYNAMIC_GHC_PROGRAMS = NO
mode.

Verified by running the following test:

    // cat a.c
    #include <stdio.h>

    void ffi_a_hello (int i) {
        fprintf (stderr, "WEEEEEEEE: i=%d\n", i);
    }

    -- cat A.hs
    {-# LANGUAGE ForeignFunctionInterface #-}
    module A where

    import Foreign.C

    foreign import ccall "ffi_a_hello" a :: CInt -> IO ()

    # ghc -fPIC -c a.c -fforce-recomp
    # ghc -fPIC -c A.hs -fforce-recomp
    # ghc --interactive ./a.o A
    ...
    *A> a 42
    WEEEEEEEE: i=42
See gory details in Trac #10402.

Signed-off-by: Colin Watson <cjwatson@debian.org>
Signed-off-by: Sergei Trofimovich <siarheit@google.com>

Reviewed By: bgamari, austin

Differential Revision: https://phabricator.haskell.org/D996

GHC Trac Issues: #10402

comment:20 Changed 4 years ago by thoughtpolice

Resolution: fixed
Status: patchclosed

Merged to ghc-7.10.

comment:21 Changed 4 years ago by nomeata

I uploaded 7.10.2-rc2 to Debian now, but it seems that something else broke here:

rts/Linker.c: In function 'do_Elf_Rela_relocations':

rts/Linker.c:5963:10:
     error: duplicate case value
              case R_PPC_PLTREL24:
              ^

rts/Linker.c:5959:10:
     error: previously used here
              case R_PPC_PLTREL24:
              ^

rts/Linker.c:5996:10:
     error: duplicate case value
              case R_PPC_REL16_LO:
              ^

rts/Linker.c:5984:10:
     error: previously used here
              case R_PPC_REL16_LO:
              ^

rts/Linker.c:6000:10:
     error: duplicate case value
              case R_PPC_REL16_HI:
              ^

rts/Linker.c:5988:10:
     error: previously used here
              case R_PPC_REL16_HI:
              ^

rts/Linker.c:6004:10:
     error: duplicate case value
              case R_PPC_REL16_HA:
              ^

rts/Linker.c:5992:10:
     error: previously used here
              case R_PPC_REL16_HA:
              ^

https://buildd.debian.org/status/fetch.php?pkg=ghc&arch=powerpc&ver=7.10.1.20150630-1&stamp=1435929642

Any ideas?

comment:22 in reply to:  21 Changed 4 years ago by slyfox

Replying to nomeata:

I uploaded 7.10.2-rc2 to Debian now, but it seems that something else broke here:

rts/Linker.c: In function 'do_Elf_Rela_relocations':

rts/Linker.c:5963:10:
     error: duplicate case value
              case R_PPC_PLTREL24:
              ^

rts/Linker.c:5959:10:
     error: previously used here
              case R_PPC_PLTREL24:
              ^

rts/Linker.c:5996:10:
     error: duplicate case value
              case R_PPC_REL16_LO:
              ^

rts/Linker.c:5984:10:
     error: previously used here
              case R_PPC_REL16_LO:
              ^

rts/Linker.c:6000:10:
     error: duplicate case value
              case R_PPC_REL16_HI:
              ^

rts/Linker.c:5988:10:
     error: previously used here
              case R_PPC_REL16_HI:
              ^

rts/Linker.c:6004:10:
     error: duplicate case value
              case R_PPC_REL16_HA:
              ^

rts/Linker.c:5992:10:
     error: previously used here
              case R_PPC_REL16_HA:
              ^

https://buildd.debian.org/status/fetch.php?pkg=ghc&arch=powerpc&ver=7.10.1.20150630-1&stamp=1435929642

Any ideas?

I think you can safely drop debian-specific patch:

dpkg-source: info: applying PPC-relocations.patch

comment:23 Changed 4 years ago by nomeata

Any ideas?

I think you can safely drop debian-specific patch:

dpkg-source: info: applying PPC-relocations.patch

Oups. Quite right. I thought I did. sorry for the noise.

Note: See TracTickets for help on using tickets.