Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Apr 6, 2017 at 6:07 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 06, 2017 at 03:44:47PM -0700, H.J. Lu wrote:
>> On Thu, Apr 6, 2017 at 3:40 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Thu, Apr 06, 2017 at 10:45:32AM -0700, H.J. Lu wrote:
>> >> Since common symbols that are turned into definitions don't have the
>> >> DEF_REGULAR flag set, we need to check ELF_COMMON_DEF_P for common
>> >> symbols.
>> >>
>> >> bfd/
>> >>
>> >>       PR ld/19579
>> >>       PR ld/21306
>> >>       * elf32-s390.c (elf_s390_finish_dynamic_symbol): Check
>> >>       ELF_COMMON_DEF_P for common symbols.
>> >>       * elf64-s390.c (elf_s390_finish_dynamic_symbol): Likewise.
>> >>       * elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.
>> >>       * elflink.c (_bfd_elf_merge_symbol): Revert commits
>> >>       202ac193bbbecc96a4978d1ac3d17148253f9b01 and
>> >>       07492f668d2173da7a2bda3707ff0985e0f460b6.
>> >>
>> >> ld/
>> >>
>> >>       PR ld/19579
>> >>       PR ld/21306
>> >>       * testsuite/ld-elf/pr19579a.c (main): Updated.
>> >
>> > This is OK.  Thanks for looking at s390 too.  Which other targets do
>> > you have cross-compilers installed or test natively in order to see
>> > 19579 failures?  (I'm assuming that's why you made the s390 changes,
>>
>> 19579 test needs a target compiler to compile.  I updated s390
>> since the s390 code in question is in PR 19579.
>>
>> > and would like to know the targets that might yet need attention.)
>> >
>>
>> It is hard to tell if other targets are affected since we don't need to
>> check ELF_COMMON_DEF_P everywhere.
>
> Right.  If I find myself bored with nothing else to do, I may audit
> all the uses of def_regular in code that runs after lang_common, in
> order to see whether we can delete ELF_COMMON_DEF_P and set
> def_regular for commons instead.  It isn't hard to make your previous
> patch work; You just need to define bfd_elfNN_bfd_define_common_symbol
> conditionally in elfxx-target.h.  Search for "generic linker" there.
>
> For now, I think adding ELF_COMMON_DEF_P as necessary is the best we
> can do.
>

I checked it in.  OK to backport to 2,28 branch?

-- 
H.J.