This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR ld/21233: Avoid sweeping forced-undefined symbols in section GC
- From: "Maciej W. Rozycki" <macro at imgtec dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: Nick Clifton <nickc at redhat dot com>, <binutils at sourceware dot org>
- Date: Tue, 4 Apr 2017 23:43:31 +0100
- Subject: Re: [PATCH] PR ld/21233: Avoid sweeping forced-undefined symbols in section GC
- Authentication-results: sourceware.org; auth=none
- References: <alpine.DEB.2.00.1703231630330.5644@tp.orcam.me.uk> <20170404084648.GD16711@bubble.grove.modra.org>
On Tue, 4 Apr 2017, Alan Modra wrote:
> > PR ld/21233
> > * ldlang.c (insert_undefined): Set `mark' for ELF symbols.
> > * testsuite/ld-elf/pr21233.sd: New test.
> > * testsuite/ld-elf/pr21233-l.sd: New test.
> > * testsuite/ld-elf/pr21233.ld: New test linker script.
> > * testsuite/ld-elf/pr21233-e.ld: New test linker script.
> > * testsuite/ld-elf/pr21233.s: New test source.
> > * testsuite/ld-elf/pr21233-l.s: New test source.
> > * testsuite/ld-elf/shared.exp: Run the new tests.
>
> OK.
Committed, thanks for your review!
> > I have identified the cause of this phenomenon to be the reverse order
> > `elf_gc_sweep' and `_bfd_elf_link_check_relocs' are called in on these
> > targets, causing `->non_got_ref' to be set for the symbol even though the
> > reference is later discarded. The possibility to change the order has
> > been introduced with commit d968975277ba ("Check ELF relocs after opening
> > all input files"), using CHECK_RELOCS_AFTER_OPEN_INPUT, after a discussion
> > started at <https://www.sourceware.org/ml/binutils/2016-04/msg00295.html>
> > which indicates the intent to gradually swap the order for all targets.
> > After the initial change for x86 this has only been since done for SH (cf
> > PR ld/17739), i.e. I gather we are still in the middle of the move.
>
> Well, powerpc hasn't changed where check_relocs is called, so this
> isn't the whole story. ie. The powerpc backend code shows that it is
> possible to get this case right without delaying check_relocs.
Right, I haven't investigated PowerPC, or indeed any target that already
passes these test cases, however I suspect that just like MIPS PowerPC
does not require copy relocations in the first place for simple static
symbol references (i.e. pointers) from data, which usually just end up
being deferred to the dynamic load time in the form of dynamic relocations
(e.g. R_MIPS_REL32 for the MIPS target, whether it uses the original SVR4
psABI or the PLT/copy-reloc extension). These do not set `->non_got_ref'
at all.
I suspect some targets might not have a psABI as simple as that however
and might require (or just want) a copy relocation even where the only
reference is a static pointer from data, and these might indeed rely on
the relative order `elf_gc_sweep' and `_bfd_elf_link_check_relocs' are
called in.
NB the avoidance of such odd cases is what I gather is implied by your
very observation made in the thread of discussion referred, specifically
here: <https://www.sourceware.org/ml/binutils/2016-04/msg00318.html>.
Of course it might be a plain backend bug too.
> > Which brings me a question to our general maintainers: which of the
> > following 3 options shall I pick for the purpose of this test case:
> >
> > 1. Leave the new failures as they are and let maintainers handle them as
> > they find need or time; there may be more to be done for individual
> > targets beyond the lone change to CHECK_RELOCS_AFTER_OPEN_INPUT.
>
> Like this, I think. Your testcase demonstrate a bug in the affected
> backends. Best make it visible.
Committed unchanged then. Any issue with backporting it to 2.28, as per
the situation with the problem report?
Maciej