Re: [PATCH 5/5] PR ld/20828: Reorder the symbol sweep stage of section GC

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

On Tue, 24 Jan 2017, Alan Modra wrote:

> > 	PR ld/20828
> > 	* elflink.c (bfd_elf_record_link_assignment): Revert last 
> > 	change and don't ever clear `forced_local'.  Set `mark' 
> > 	unconditionally.
> > 	(elf_gc_sweep_symbol_info, elf_gc_sweep_symbol): Reorder within
> > 	file.
> > 	(elf_gc_sweep): Move the call to `elf_gc_sweep_symbol'...
> > 	(bfd_elf_size_dynamic_sections): ... here.
> > 	* elf32-ppc.c (ppc_elf_tls_setup): Don't clear `forced_local'
> > 	and set `mark' instead in `__tls_get_addr_opt' processing.
> > 	* elf64-ppc.c (ppc64_elf_tls_setup): Likewise.
> 
> I think this is OK for master, but please delay putting this patch on
> the branch for a while.  There is a possibility that the change might
> tickle bugs in some target check_relocs.

 OK.  I think it should be fine as version scripts do their symbol forcing 
to the local scope at around this time too (I haven't dug through the 
sources to find the exact spot though; in any case they do it after 
`bfd_elf_record_link_assignment' calls have been made or the 
`pr20828-2.so' test aka "PR ld/20828 dynamic symbols with section GC 
(version script)" wouldn't pass with my original solution for this bug) 
and their action is effectively the same, with any and all the 
consequences to relocation handling.  This is one reason why I concluded 
doing it at this stage was fine; which I actually meant to state along the 
submission, but in the end it has escaped me somehow.

 Patch committed to master only then, thanks for your review.

 BTW, in the process of making this change I have discovered that the 
linker script PROVIDE keyword causes the symbol requested to be emitted 
even in the absence of a local reference if there is an identically named 
symbol exported from a DSO present in the link.  This can be reproduced 
with the `pr20828-1.so' test and its `pr20828.ld' script modified to use 
PROVIDE with its symbols, regardless of the presence of `--gc-sections', 
as long as `libpr20828.so' is included in the link.  If the DSO is absent, 
then the symbols are not emitted.

 I believe this is due to this piece:

  /* If this symbol is being provided by the linker script, and it is
     currently defined by a dynamic object, but not by a regular
     object, then mark it as undefined so that the generic linker will
     force the correct value.  */
  if (provide
      && h->def_dynamic
      && !h->def_regular)
    h->root.type = bfd_link_hash_undefined;

in `bfd_elf_record_link_assignment', the presence of which however 
predates our repo history and therefore any justification is hard to track 
down.  Our manual however only states that:

"In some cases, it is desirable for a linker script to define a symbol
only if it is referenced and is not defined by any object included in
the link.  For example, traditional linkers defined the symbol `etext'.
However, ANSI C requires that the user be able to use `etext' as a
function name without encountering an error.  The `PROVIDE' keyword may
be used to define a symbol, such as `etext', only if it is referenced
but not defined.  The syntax is `PROVIDE(SYMBOL = EXPRESSION)'."

which given our semantics is I believe at least ambiguous as it does not 
tell static and dynamic objects apart -- it merely states "any object".

 Do you or does anyone know what the purpose of this special case is?  It 
might be to preempt the DSO symbol so that the DSO uses ours and not its 
own, however preemption is not going to happen in a `-shared' link anyway 
(and it seems against the spirit of PROVIDE).

 Shall we keep it (presumably yes, owing to its long history) and make it 
clear in documentation?

  Maciej