Re: [PATCH] elf: Don't load archive element after dynamic definition

[Alan Modra <amodra@gmail.com>]

On Wed, Sep 02, 2020 at 04:56:48AM -0700, H.J. Lu wrote:
> On Wed, Sep 2, 2020 at 1:12 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Wed, Sep 02, 2020 at 08:52:06AM +0200, Martin Liška wrote:
> > > On 8/28/20 4:50 PM, H.J. Lu via Binutils wrote:
> > > > PR ld/26530 test still failed.
> > >
> > > Hello.
> > >
> > > Is there any progress on this please?
> >
> > I think what we want is the following, and some tweaking of the
> > testsuite to remove FAIL: PR ld/15146 (1).  That test relied on not
> > loading a shared lib due to an IR symbol reference, but the logic
> > added by git commit 3d5bef4c0871 has already been reverted.
> >
> > diff --git a/bfd/elflink.c b/bfd/elflink.c
> > index 5c085b14b7..346f534911 100644
> > --- a/bfd/elflink.c
> > +++ b/bfd/elflink.c
> > @@ -4977,11 +4977,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
> >              object and a shared object.  */
> >           bfd_boolean dynsym = FALSE;
> >
> > -         /* Plugin symbols aren't normal.  Don't set def_regular or
> > -            ref_regular for them, or make them dynamic.  */
> > -         if ((abfd->flags & BFD_PLUGIN) != 0)
> > -           ;
> > -         else if (! dynamic)
> > +         if (! dynamic)
> >             {
> >               if (! definition)
> >                 {
> > @@ -5162,10 +5158,6 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
> >               && !bfd_link_relocatable (info))
> >             dynsym = FALSE;
> >
> > -         /* Nor should we make plugin symbols dynamic.  */
> > -         if ((abfd->flags & BFD_PLUGIN) != 0)
> > -           dynsym = FALSE;
> > -
> >           if (definition)
> >             {
> >               h->target_internal = isym->st_target_internal;
> > @@ -5192,7 +5184,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
> >                 }
> >             }
> >
> > -         if (dynsym && h->dynindx == -1)
> > +         if (dynsym && (abfd->flags & BFD_PLUGIN) == 0 && h->dynindx == -1)
> >             {
> >               if (! bfd_elf_link_record_dynamic_symbol (info, h))
> >                 goto error_free_vers;
> 
> Will it add a shared library to DT_NEEDED even if the IR symbol reference is
> removed by LTO?

Yes.  That can't be helped.  I know we worried about unnecessary
--as-needed shared libraries in the past when IR symbols disappear
after LTO, but I can't see a simple and reliable way for the linker to
be correct.

It's reasonably obvious that we need to load archive elements when
they define IR referenced symbols, because the archive element might
be an LTO object.  What's not so obvious is whether loading of shared
libraries should follow the same rule.  I think they should, in order
for LTO to get symbol resolution correct in corner cases.  Basically
LTO needs to know what shared libraries are loaded before
recompilation.  See commit a896df97b952 log comments.

-- 
Alan Modra
Australia Development Lab, IBM