Bug 26902 - gold powerpc inserts jump to middle of long branch stub
Summary: gold powerpc inserts jump to middle of long branch stub
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.36
: P2 normal
Target Milestone: 2.36
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-16 01:02 UTC by Michael Hudson-Doyle
Modified: 2020-11-17 11:24 UTC (History)
4 users (show)

See Also:
Host:
Target: powerpc64le-linux-gnu
Build:
Last reconfirmed: 2020-11-16 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hudson-Doyle 2020-11-16 01:02:11 UTC
When linking a call to a file in the same module that requires a long branch, gold inserts a jump to 8 bytes into the stub.

This came up with a ghc update in Ubuntu, but it's possible to recreate by hand.

Here are my test files:

(hirsute-ppc64el)root@juju-b11c42-ubuntu-26:/build/haskell-network-byte-order-RQo4iL/haskell-network-byte-order-0.1.5/gold-test-case# cat main.c
void large(void);
void local(void);

int
main(int argc, char** argv)
{
        local();
        return 0;
}

(hirsute-ppc64el)root@juju-b11c42-ubuntu-26:/build/haskell-network-byte-order-RQo4iL/haskell-network-byte-order-0.1.5/gold-test-case# cat large.S
        .machine power8
        .abiversion 2
        .section        ".text"
        .align 2
large:
#include "nops16M.h"
(hirsute-ppc64el)root@juju-b11c42-ubuntu-26:/build/haskell-network-byte-order-RQo4iL/haskell-network-byte-order-0.1.5/gold-test-case# cat target.c
#include <stdio.h>
void local(void);
int a_global;

void local(void)
{
        printf("a_global %d\n", a_global);
}

I made nops16M.h like this:

for x in `seq 1024`; do echo nop >> nops1k.h; done
for x in `seq 1024`; do echo '#include "nops1k.h"' >> nops1M.h; done
for x in `seq 16`; do echo '#include "nops1M.h"' >> nops16M.h; done


Then:

# gcc -c -o main.o main.c
# gcc -c -o large.o large.S
# gcc -c -o target.o target.c
# gcc -fuse-ld=gold -o test-gold main.c large.o target.o
# ./test-gold
Segmentation fault (core dumped)

And from objdump -D test-gold:

000000000000073c <main>:
     73c:       02 04 4c 3c     addis   r2,r12,1026
     740:       c4 78 42 38     addi    r2,r2,30916
     744:       a6 02 08 7c     mflr    r0
     748:       10 00 01 f8     std     r0,16(r1)
     74c:       f8 ff e1 fb     std     r31,-8(r1)
     750:       c1 ff 21 f8     stdu    r1,-64(r1)
     754:       78 0b 3f 7c     mr      r31,r1
     758:       78 1b 69 7c     mr      r9,r3
     75c:       20 00 9f f8     std     r4,32(r31)
     760:       e6 01 09 7c     mtfprwz f0,r9
     764:       2c 00 3f 39     addi    r9,r31,44
     768:       ae 4f 00 7c     stfiwx  f0,0,r9
     76c:       9d 00 00 48     bl      808 <00000000.long_branch.4000810+0x8>
     770:       00 00 00 60     nop
     774:       00 00 20 39     li      r9,0
     778:       78 4b 23 7d     mr      r3,r9
     77c:       40 00 3f 38     addi    r1,r31,64
     780:       10 00 01 e8     ld      r0,16(r1)
     784:       a6 03 08 7c     mtlr    r0
     788:       f8 ff e1 eb     ld      r31,-8(r1)
     78c:       20 00 80 4e     blr
     790:       00 00 00 00     .long 0x0
     794:       00 00 00 01     .long 0x1000000
     798:       80 01 00 01     .long 0x1000180
     79c:       00 00 00 00     .long 0x0

and

0000000000000800 <00000000.long_branch.4000810>:
     800:       70 80 82 e9     ld      r12,-32656(r2)
     804:       a6 03 89 7d     mtctr   r12
     808:       20 04 80 4e     bctr
     80c:       00 00 00 00     .long 0x0

This doesn't happen with -fuse-ld=bfd.

There are some recent commits that look like they might address this (specifically fa40fbe484954c560ab1c0ff4bc1b2eeb1511344 which says in part

            (Target_powerpc::Branch_info::make_stub): Don't add local
            entry offset to long branch dest passed to
            add_long_branch_entry.  Do pass st_other bits.

) but I built gold from git and this still happens.
Comment 1 Michael Hudson-Doyle 2020-11-16 01:12:18 UTC
This not-thought-through at all patch:

diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index b0d6a74bec..1ef8c47053 100644
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -11092,10 +11092,6 @@ Target_powerpc<size, big_endian>::Relocate::relocate(
                      value = (stub_table->stub_address()
                               + stub_table->plt_size()
                               + ent->off_);
-                     if (size == 64
-                         && r_type != elfcpp::R_PPC64_REL24_NOTOC)
-                       value += (elfcpp::ppc64_decode_local_entry(ent->other_)
-                                 + ent->tocoff_);
                    }
                  has_stub_value = true;
                }


fixes the issue for me. I certainly don't know if it would have other bad effects.
Comment 2 Alan Modra 2020-11-16 11:16:51 UTC
(In reply to Michael Hudson-Doyle from comment #1)
> This not-thought-through at all patch:

More thought-through than the code it removes!  If you post it to binutils@soureware.org with a ChangeLog entry I'll commit it for you.
Comment 3 Sourceware Commits 2020-11-17 10:30:40 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f1e05b19786669d29d59f48f26bc06ad67c221e2

commit f1e05b19786669d29d59f48f26bc06ad67c221e2
Author: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date:   Mon Nov 16 14:20:23 2020 +1300

    [GOLD] fix jump to long branch on powerpc
    
            PR 26902
            * powerpc.cc (Relocate::relocate): Do not include local entry
            offset of target function when computing the address of a stub.
Comment 4 Sourceware Commits 2020-11-17 11:13:18 UTC
The binutils-2_35-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f7b330574f32eb91881e56401fe0cd875a96df19

commit f7b330574f32eb91881e56401fe0cd875a96df19
Author: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date:   Mon Nov 16 14:20:23 2020 +1300

    [GOLD] fix jump to long branch on powerpc
    
            PR 26902
            * powerpc.cc (Relocate::relocate): Do not include local entry
            offset of target function when computing the address of a stub.
    
    (cherry picked from commit f1e05b19786669d29d59f48f26bc06ad67c221e2)
Comment 5 Alan Modra 2020-11-17 11:24:45 UTC
Fixed