Re: [BFD][PR21703]Override the new defined symbol with the old normal symbol when --allow-multiple-definition is provided

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

On Tue, Jul 11, 2017 at 10:31 AM, Renlin Li <renlin.li@foss.arm.com> wrote:
> Hi Alan,
>
> On 08/07/17 03:11, Alan Modra wrote:
>>
>> On Fri, Jul 07, 2017 at 05:41:04PM +0100, Renlin Li wrote:
>>>
>>> diff --git a/bfd/elflink.c b/bfd/elflink.c
>>> index 471e8ad..e4ab670 100644
>>> --- a/bfd/elflink.c
>>> +++ b/bfd/elflink.c
>>> @@ -1547,6 +1547,17 @@ _bfd_elf_merge_symbol (bfd *abfd,
>>>         sec = *psec;
>>>       }
>>>
>>> +  /* There are multiple definitions of a normal symbol.  */
>>> +  if (olddef && !olddyn && !oldweak && newdef && !newdyn && !newweak
>>> +      && !*matched)
>>
>>
>> I think !*matched is wrong, and will likely mean the patch doesn't fix
>> your PR.
>
> I meant to skip the case for versioned symbol. This function will be called
> with short name for versioned symbol. That's should be skipped.
>
> I correct the condition and add one more condition for ld plugin case.
>>
>>
>>> +    {
>>> +      /* Handle a multiple definition.  */
>>> +      (*info->callbacks->multiple_definition) (info, &h->root,
>>> +                                              abfd, sec, sym->st_value);
>>
>>
>> Please use *pvalue here rather than sym->st_value.
>
>
> Updated.
> A new test case is added. The symbol foo in the final object should be of
> size 4 which comes from the first object file.
>
>
> Here, the symbol types are not required to be the same. I think this is the
> desired behavior?
>
> ld cross and native checked Okay for arm environment. glibc builds Okay with
> the patched linker.
>
> Okay?
>
> Regards,
> Renlin
>
> ld/ChangeLog:
>
> 2017-07-11  Renlin Li  <renlin.li@arm.com>
>
>         PR ld/21703
>         * testsuite/ld-elf/elf.exp : Run new test case.
>         * testsuite/ld-elf/pr21703-1.s: New.
>         * testsuite/ld-elf/pr21703-2.s: New.
>         * testsuite/ld-elf/pr21703.sd: New.
>
> bfd/ChangeLog:
>
> 2017-07-11  Renlin Li  <renlin.li@arm.com>
>
>
>         PR ld/21703
>         * elflink.c (_bfd_elf_merge_symbol): Handle multiple symbol
> definition
>         case.

Please add some a test to cover versioned_sym in:

  /* There are multiple definitions of a normal symbol.  */
+  if (olddef && !olddyn && !oldweak && newdef && !newdyn && !newweak
+      && !versioned_sym && h->def_regular)
+    {

What should happen if symbol is defined in linker script or command
line?

-- 
H.J.