BFD internal errors in *_finish_dynamic_symbol

Jiong Wang jiong.wang@foss.arm.com
Mon Jul 3 16:43:00 GMT 2017


On 03/07/17 15:46, Egeyar Bagcioglu wrote:
> On 07/03/2017 04:13 PM, Egeyar Bagcioglu wrote:
>> On 07/03/2017 03:24 PM, Jiong Wang wrote:
>>>>
>>> I incline to remove the assertion at the start of *finish_dynamic_symbol as this
>>> assertion brings nothing more guarantee on the correctness.  I guess it's the
>>> same on x86 and SPARC.
>>>
>>> The outer caller elf_link_output_extsym in elflink.c is a traverse function on
>>> all external symbol, and it will only call *finish_dynamic_symbol if some
>>> conditions is meet.  It is executed conditionally.
>>>
>>> If the condition to trigger that assertion is satisified, it then won't satify
>>> the outer check in finish_dynamic_symbol, so *finish_dynamic_symbol won't be
>>> called that the assertion is expected to be dead code.
>>>
>>> If elf_link_output_extsym is a traverse function that unconditionally called
>>> on external symbols decided to be exported, then an assertion to make sure these
>>> symbols are in sane status might make sense.
>>>
>>> H.J,  Egeyar, what's your opinion here?
>>>
>> Hello Jiong,
>>
>> I agree with your point. This assertion does not contribute to correctness. It was not to skip any unforseen problems about the core change of the patch we submitted. I would expect it to help detecting an issue regarding the changed relocation. Instead, it detected an issue with the condition to assert. Under these circumstances, I see no advantage of keeping it. If a problem occurs regarding the core change of our previous patches, it can easily be traced back to us via git bisect.
>>
>> Unless someone objects by then, I'll upload the corresponding mini-patch after receiving internal approval from my team.
>
> The attached patch fixes the issue for sparc.

The attached patch is a similar partial revert on AArch64.

I am not sure if it qualifies obvious after this discussion.

I will commit it by early afternoon tomorrow if there is no objection so it can be
included into 2.29 branch

bfd/
2017-07-03  Jiong Wang  <jiong.wang@arm.com>

         * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_symbol): Remove the
         sanity check at the head of this function.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: part-revert.patch
Type: text/x-patch
Size: 663 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20170703/407f1830/attachment.bin>


More information about the Binutils mailing list