[GAS][ARM] Fix testism for bl local v4t test

Andre Vieira (lists) Andre.SimoesDiasVieira@arm.com
Tue Oct 30 11:18:00 GMT 2018


On 26/10/18 15:18, Andre Vieira (lists) wrote:
> Hi Thomas,
> 
> Thanks for the review.
> On 26/10/18 11:38, Thomas Preudhomme wrote:
>> Hi Andre,
>>
>> On Fri, 19 Oct 2018 at 15:59, Andre Vieira (lists)
>> <Andre.SimoesDiasVieira@arm.com> wrote:
>>>
>>> Hi,
>>>
>>> This patch fixes a testism.
>>> I believe this test was never really intended to test for this error
>>> message and
>>> '# stderr' never actually checked it.  This warning is tested elsewhere
>>> and to my
>>> understanding it would have not made any sense here either.
>>>
>>> I did some archaeological digging and found that the original patch that
>>> was reviewed on the ml contained a blx-thumb-local.{d,s,l}.
>>> See https://sourceware.org/ml/binutils/2009-03/msg00511.html
>>> The commit itself was missing the .s and .d files, I suspect these fell
>>> through the 'svn add' crack and they help explain why there is a
>>> blx-thumb-local.l to begin with.
>>> I re-added them in this patch and updated due to bitrot.
>>>
>>> Is this OK for trunk?
>>
>> The commit 2ac93be706418f3b2aebeb22159a328023faed52 removed support
>> for arm aout and coff targets, you can see that it simplifies the very
>> same skip line to skipping only wince and pe arm targets and would
>> therefore suggest to adapt the test you add to do the same.
>>
> Done.
> 
>> Beyond that, there is mention of relocation for foo so I'd suggest
>> adding a readelf -r action and check for those relocation.
> If you mean in bl-local-v4t's R_ARM_THM_CALL relocation on foo2, then I
> feel that is sufficiently checked with objdump as it prints relocations
> as comments.
> 
> blx-local-thumb has no relocations and I didn't expect it to have any.
> 
> Cheers,
> Andre
> 
Ping



More information about the Binutils mailing list