Bug 28827 - [2.38 Regression] ld hits assertion building LLVM 9 on powerpc64le-linux-gnu
Summary: [2.38 Regression] ld hits assertion building LLVM 9 on powerpc64le-linux-gnu
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: 2.38
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-27 11:33 UTC by Matthias Klose
Modified: 2022-02-07 03:20 UTC (History)
1 user (show)

See Also:
Host:
Target: powerpc64le-linux-gnu
Build:
Last reconfirmed:
Project(s) to access:
ssh public key:


Attachments
handle pathological stub sizing case (2.70 KB, patch)
2022-01-28 01:52 UTC, Alan Modra
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Klose 2022-01-27 11:33:32 UTC
seen with the 2.38 branch 20220126 on powerpc64le-linux-gnu. last successful build of LLVM 9 was with trunk 20220106.

[100%] Linking CXX shared library ../../../../lib/libclang-cpp.so
[...]
/usr/bin/ld: BFD (GNU Binutils for Ubuntu) 2.37.90.20220126 assertion fail ../../bfd/elf64-ppc.c:11674
/usr/bin/ld: BFD (GNU Binutils for Ubuntu) 2.37.90.20220126 assertion fail ../../bfd/elf64-ppc.c:11674

complete build log at
https://launchpad.net/ubuntu/+source/llvm-toolchain-9/1:9.0.1-20ubuntu4/+build/23098912
Comment 1 Alan Modra 2022-01-27 12:43:49 UTC
Can you try that link again without -z relro?  Or using a binutils with commit 8b7c6a1fbab5 and 2f83249c13d8 reverted?
Comment 2 Matthias Klose 2022-01-27 13:37:14 UTC
I'll check that
Comment 3 Alan Modra 2022-01-28 01:52:53 UTC
Created attachment 13936 [details]
handle pathological stub sizing case

On thinking about the nature of the assert, I don't think I can blame the relro changes.  Even if reverting them makes the problem go away, that would likely be a case of layout changes just happening to avoid the trigger for a real bug.  And generating a testcase for the sort of problem I think we may have hit is difficult.  So that exact set of objects for the failing link is valuable..  If I could have them, that would be good.  Alternatively, you could test this patch for me without the relro changes or any other 2.38 branch changes (or gcc changes!) from where you hit the failure.
Comment 4 Sourceware Commits 2022-01-28 06:32:28 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 0441f94fba61998b4bd18487aacf70a672df099c
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Jan 28 09:46:13 2022 +1030

    PR28827, assertion building LLVM 9 on powerpc64le-linux-gnu
    
    The assertion is this one in ppc_build_one_stub
      BFD_ASSERT (stub_entry->stub_offset >= stub_entry->group->stub_sec->size);
    It is checking that a stub doesn't overwrite the tail of a previous
    stub, so not something trivial.
    
    Normally, stub sizing iterates until no stubs are added, detected by
    no change in stub section size.  Iteration also continues if no stubs
    are added but one or more stubs increases in size, which also can be
    detected by a change in stub section size.  But there is a
    pathological case where stub section sizing decreases one iteration
    then increases the next.  To handle that situation, stub sizing also
    stops at more than STUB_SHRINK_ITER (20) iterations when calculated
    stub section size is smaller.  The previous larger size is kept for
    the actual layout (so that building the stubs, which behaves like
    another iteration of stub sizing, will see the stub section sizes
    shrink).  The problem with that stopping condition is that it assumes
    that stub sizing is only affected by addresses external to the stub
    sections, which isn't always true.
    
    This patch fixes that by also keeping larger individual stub_offset
    addresses past STUB_SHRINK_ITER.  It also catches a further
    pathological case where one stub shrinks and another expands in such a
    way that no stub section size change is seen.
    
            PR 28827
            * elf64-ppc.c (struct ppc_link_hash_table): Add stub_changed.
            (STUB_SHRINK_ITER): Move earlier in file.
            (ppc_size_one_stub): Detect any change in stub_offset.  Keep
            larger one if past STUB_SHRINK_ITER.
            (ppc64_elf_size_stubs): Iterate on stub_changed too.
Comment 5 Sourceware Commits 2022-01-29 06:50:50 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 2405fc4016feadea33cb747d5654514f62b74ff4
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Jan 29 12:27:31 2022 +1030

    Re: PR28827, assertion building LLVM 9 on powerpc64le-linux-gnu
    
    The previous patch wasn't quite correct.  The size and padding depends
    on offset used in the current iteration, and if we're fudging the
    offset past STUB_SHRINK_ITER then we'd better use that offset.  We
    can't have plt_stub_pad using stub_sec->size as the offset.
    
            PR 28827
            * elf64-ppc.c (plt_stub_pad): Add stub_off param.
            (ppc_size_one_stub): Set up stub_offset to value used in this
            iteration before sizing the stub.  Adjust plt_stub_pad calls.
Comment 6 Sourceware Commits 2022-01-31 03:42:27 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 9ff8aa7d418bc508dbd429576b93e30ed9dc5891
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Jan 31 09:38:17 2022 +1030

    Re: PR28827, assertion building LLVM 9 on powerpc64le-linux-gnu
    
    In trying to find a testcase for PR28827, I managed to hit a linker
    error in bfd_set_section_contents with a .branch_lt input section
    being too large for the output .branch_lt.
    
    bfd/
            PR 28827
            * elf64-ppc.c (ppc64_elf_size_stubs): Set section size to
            maxsize past STUB_SHRINK_ITER before laying out.  Remove now
            unnecessary conditional setting of maxsize at start of loop.
    ld/
            * testsuite/ld-powerpc/pr28827-2.d,
            * testsuite/ld-powerpc/pr28827-2.lnk,
            * testsuite/ld-powerpc/pr28827-2.s: New test.
            * testsuite/ld-powerpc/powerpc.exp: Run it.
Comment 7 Alan Modra 2022-02-04 08:12:03 UTC
Matthias, thanks for making the object files available.  That let me reproduce the failure, and it isn't anthing to do with what I thought might be going on at past 20 stub sizing iterations.  The patches already applied for this bug on mainline don't fix the problem.

We're getting a section layout quite different when sizing stubs to that when building stubs.  In particular the gap at the of the relro segment changes, .plt is separated from .got.  That means the offsets from the toc pointer reg to access addresses in the plt change.  When the offsets change, the size of the required code to load those addresses can change.  That's why the assertion triggers.  A plt call stub increases in size, which then results in the next stub overwriting the end of the plt call stub.

Reverting HJ's relro patches avoids the bug.
Comment 8 Alan Modra 2022-02-04 10:29:05 UTC
The major reason for layout to change is editing of .eh_frame and sizing of .eh_frame_hdr.  Obviously we want to continue doing that, but what should *not* happen is changes in .got/.plt relative layout.  Prior to HJ's two lang_size_relro_segment patches that didn't happen.
Comment 9 Sourceware Commits 2022-02-05 07:09:12 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 9810db10f726f47c8e878ca4b0b4b4f5e9c16a5d
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Feb 5 15:36:58 2022 +1030

    PR28827 testcase
    
    This testcase triggers a stub sizing error with the patches applied
    for PR28743 (commit 2f83249c13d8 and c804c6f98d34).
    
            PR 28827
            * testsuite/ld-powerpc/pr28827-1.s,
            * testsuite/ld-powerpc/pr28827-1.d: New test.
            * testsuite/ld-powerpc/powerpc.exp: Run it.
Comment 10 Sourceware Commits 2022-02-05 08:28:05 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 0acf434a23768449cbb4b3732355f3f2febecaee
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Feb 5 18:19:21 2022 +1030

    Tweak assembler invocation for pr28827-1 test
    
            PR 28827
            * testsuite/ld-powerpc/pr28827-1.d: Pass -a64 to gas.
Comment 11 Sourceware Commits 2022-02-05 08:30:01 UTC
The binutils-2_38-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit d3ec1c514429b8c695974e785e47af8f71c388a2
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Jan 28 09:46:13 2022 +1030

    PR28827, assertion building LLVM 9 on powerpc64le-linux-gnu
    
    The assertion is this one in ppc_build_one_stub
      BFD_ASSERT (stub_entry->stub_offset >= stub_entry->group->stub_sec->size);
    It is checking that a stub doesn't overwrite the tail of a previous
    stub, so not something trivial.
    
    Normally, stub sizing iterates until no stubs are added, detected by
    no change in stub section size.  Iteration also continues if no stubs
    are added but one or more stubs increases in size, which also can be
    detected by a change in stub section size.  But there is a
    pathological case where stub section sizing decreases one iteration
    then increases the next.  To handle that situation, stub sizing also
    stops at more than STUB_SHRINK_ITER (20) iterations when calculated
    stub section size is smaller.  The previous larger size is kept for
    the actual layout (so that building the stubs, which behaves like
    another iteration of stub sizing, will see the stub section sizes
    shrink).  The problem with that stopping condition is that it assumes
    that stub sizing is only affected by addresses external to the stub
    sections, which isn't always true.
    
    This patch fixes that by also keeping larger individual stub_offset
    addresses past STUB_SHRINK_ITER.  It also catches a further
    pathological case where one stub shrinks and another expands in such a
    way that no stub section size change is seen.
    
            PR 28827
            * elf64-ppc.c (struct ppc_link_hash_table): Add stub_changed.
            (STUB_SHRINK_ITER): Move earlier in file.
            (ppc_size_one_stub): Detect any change in stub_offset.  Keep
            larger one if past STUB_SHRINK_ITER.
            (ppc64_elf_size_stubs): Iterate on stub_changed too.
    
    (cherry picked from commit 0441f94fba61998b4bd18487aacf70a672df099c)
    
    Re: PR28827, assertion building LLVM 9 on powerpc64le-linux-gnu
    
    The previous patch wasn't quite correct.  The size and padding depends
    on offset used in the current iteration, and if we're fudging the
    offset past STUB_SHRINK_ITER then we'd better use that offset.  We
    can't have plt_stub_pad using stub_sec->size as the offset.
    
            PR 28827
            * elf64-ppc.c (plt_stub_pad): Add stub_off param.
            (ppc_size_one_stub): Set up stub_offset to value used in this
            iteration before sizing the stub.  Adjust plt_stub_pad calls.
    
    (cherry picked from commit 2405fc4016feadea33cb747d5654514f62b74ff4)
    
    Re: PR28827, assertion building LLVM 9 on powerpc64le-linux-gnu
    
    In trying to find a testcase for PR28827, I managed to hit a linker
    error in bfd_set_section_contents with a .branch_lt input section
    being too large for the output .branch_lt.
    
    bfd/
            PR 28827
            * elf64-ppc.c (ppc64_elf_size_stubs): Set section size to
            maxsize past STUB_SHRINK_ITER before laying out.  Remove now
            unnecessary conditional setting of maxsize at start of loop.
    ld/
            * testsuite/ld-powerpc/pr28827-2.d,
            * testsuite/ld-powerpc/pr28827-2.lnk,
            * testsuite/ld-powerpc/pr28827-2.s: New test.
            * testsuite/ld-powerpc/powerpc.exp: Run it.
    
    (cherry picked from commit 9ff8aa7d418bc508dbd429576b93e30ed9dc5891)
Comment 12 Sourceware Commits 2022-02-05 08:30:21 UTC
The binutils-2_38-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 0e24f6bd7f40acb1a6cf8046ba66e09e325e6f3b
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Feb 5 15:36:58 2022 +1030

    PR28827 testcase
    
    This testcase triggers a stub sizing error with the patches applied
    for PR28743 (commit 2f83249c13d8 and c804c6f98d34).
    
            PR 28827
            * testsuite/ld-powerpc/pr28827-1.s,
            * testsuite/ld-powerpc/pr28827-1.d: New test.
            * testsuite/ld-powerpc/powerpc.exp: Run it.
    
    (cherry picked from commit 9810db10f726f47c8e878ca4b0b4b4f5e9c16a5d)
    (cherry picked from commit 0acf434a23768449cbb4b3732355f3f2febecaee)
Comment 13 H.J. Lu 2022-02-06 15:12:48 UTC
A workaround patch is at

https://sourceware.org/pipermail/binutils/2022-February/119651.html
Comment 14 Alan Modra 2022-02-07 03:20:43 UTC
Fixed by reverting the offending relro patches.