Re: [PATCH] bfd: Mark symbols with mismatching types

[Alan Modra <amodra at gmail dot com>]

On Fri, Apr 14, 2017 at 04:57:18PM +0200, Egeyar Bagcioglu wrote:
> On 04/14/2017 01:27 AM, Alan Modra wrote:
> >On Wed, Apr 12, 2017 at 10:11:42PM -0700, Egeyar Bagcioglu wrote:
> >>Introduce flag for defined symbols whose type is mismatched with a
> >>dynamic definition. Mark such symbols. Prevent such symbols from being
> >>made dynamic in sparc.
> >>
> >>Sparc makes use of dynamic symbols during relocations. Unless detected,
> >>above-mentioned symbols are confused with library symbols by the dynamic
> >>linker.
> >Please provide a C testcase.  I can see what you're trying to prevent
> >but the testcase I wrote to investigate the problem on sparc and other
> >targets does not result in bad dynamic symbols, even though the linker
> >does hit the code where you are setting conflict_with_library.
> >Obviously I'm missing some detail.
> >
> >A testcase is necessary to prove that you really do need a new flag in
> >elf_link_hash_entry.  If the problem only shows up on sparc, then a
> >different fix in the sparc backend is likely better.  If the problem
> >is generic then yes, we may need a new flag, but even then there is
> >likely a better fix..  I'm not asking for a patch that integrates a
> >test into the ld testsuite, just a set of C files and gcc command
> >lines to build it.
> >
> Hello Alan,
> 
> There is already a failing test case due to this problem. You can see "FAIL:
> Run pr2404 with PIE" when running ld-elf/shared.exp on sparc. The test is
> for the specific case of having conflicting symbols. '_bfd_elf_merge_symbol'
> currently just skips the merge of such symbols without any further action.

OK, other targets pass this test.  x86_64-linux and powerpc64le-linux
I've tested natively myself.  This says you do not need a new flag,
but rather a sparc backend change..

> However, on sparc backend, we take the liberty of making some symbols
> dynamic while adjusting and optimizing relocations.

Demonstrating that it isn't safe to copy x86_64 backend code.  The
testcase shows the sparc backend making bfd_link_hash_defined symbols
dynamic in allocate_dynrelocs.  That's just plain wrong.  As the
comment says "Undefined weak syms won't yet be marked as dynamic".
You should *only* be changing bfd_link_hash_undefweak there.  Fix that
and you won't need conflict_with_library.  Note that the fix likely
will require changes to _bfd_sparc_elf_relocate_section too.

-- 
Alan Modra
Australia Development Lab, IBM