Re: [PATCH] PR ld/21233: Avoid sweeping forced-undefined symbols in section GC

["Maciej W. Rozycki" <macro at imgtec dot com>]

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