[PATCH] elf: Check multiple definition when defining hidden version

[H.J. Lu]

On Thu, Dec 3, 2020 at 3:29 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Dec 02, 2020 at 05:52:05PM -0800, H.J. Lu wrote:
> > Linker should reject such inputs.
>
> Making assertions without giving any reason is not the best way to
> convince anyone you are correct.
>
> Tell me why the following is wrong.
>
> Subject: PR26978, Inconsistency for strong foo@v1 and weak foo@@v1
>
> Prior to this patch
>   ld -shared --version-script=pr26979.ver pr26978a.o pr26978b.o
> results in
>   ld: pr26978b.o: in function `foo_v1':
>   (.text+0x0): multiple definition of `foo@v1'
>   ld: pr26978b.o:(*IND*+0x0): multiple definition of `foo'
> while
>   ld -shared --version-script=pr26979.ver pr26978b.o pr26978a.o
> results in no error, but some odd dynamic symbols.
>   ... 0 NOTYPE  GLOBAL DEFAULT    7 foo@v1
>   ... 0 NOTYPE  WEAK   DEFAULT    7 foo@@v1
>
> When linking an undecorated reference to foo against such a shared
> library, ld complains about multiple definitions of foo@v1 while gold
> creates a dynamic reference to foo@v1.  That results in foo@v1 being
> used at runtime.
>
> While we could error in both cases, it is reasonable to say foo@v1 and
> foo@@v1 are in fact the same symbol.  (Same name, same version.  The
> only real difference is that foo@@v1 satisfies a reference to plain
> foo, while foo@v1 does not.)  Just as merging a weak undecorated sym
> with a strong sym results in the strong sym prevailing, so should the
> strong foo@v1 prevail.  And since there is a definition that satisfies
> plain foo, the foo@@v1 variety of dynamic symbol should be emitted at
> the foo@v1 value.  That makes the testcase that currently links
> continue to produce a shared library, and that shared library can now
> be used by both ld and gold with the same runtime behaviour as when
> using gold with the odd dynamic symbol library.
>
> bfd/
>         PR 26978
>         * elflink.c (_bfd_elf_add_default_symbol): Handle the case where
>         a new weak sym@@ver should be overridden by an existing sym@ver.
>         (elf_link_add_object_symbols): Don't _bfd_elf_add_default_symbol
>         for a new weak sym@ver when sym@@ver already exists.
>         * linker.c (link_action): Choose MIND for previous indirect,
>         current def, rather than MDEF.
>         (_bfd_generic_link_add_one_symbol <MIND>): Handle redefinition of
>         weak indirect symbol.

What your patch does are

1. Hide the weak foo@@v1.
2. Change the non-weak, hidden foo@v1 to the default foo@@v1.

It is the opposite of what @@ and @ are supposed to do.  I believe we
should reject such inputs.

-- 
H.J.