This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [GAS][ARM] Fix testism for bl local v4t test
- From: "Andre Vieira (lists)" <Andre dot SimoesDiasVieira at arm dot com>
- To: Thomas Preudhomme <thomas dot preudhomme at linaro dot org>
- Cc: binutils at sourceware dot org, Nick Clifton <nickc at redhat dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>
- Date: Thu, 22 Nov 2018 17:16:40 +0000
- Subject: Re: [GAS][ARM] Fix testism for bl local v4t test
- References: <5BC9F15F.7020403@arm.com> <CAKnkMGtqyPdatnYy-D_k27Vrd9HWbs8827FDA0aa9k1awfsUaw@mail.gmail.com> <5BD3224D.5040300@arm.com> <5BD83E13.8080009@arm.com> <5BE5B6E4.7070806@arm.com> <CAKnkMGv3jr+YWTjfBqVDvkz0unmNRurvrSK8FGb=5-SLNekAbA@mail.gmail.com>
On 19/11/18 13:40, Thomas Preudhomme wrote:
> Hi Andre,
>
> Sorry for the delay. I was confused by the THUMB_PCREL_JUMP comment, I
> suppose that's an internal relocation. LGTM now but as I said I'm not
> a maintainer so cannot approve your patch.
>
> Best regards,
>
> Thomas
> On Fri, 9 Nov 2018 at 16:33, Andre Vieira (lists)
> <Andre.SimoesDiasVieira@arm.com> wrote:
>>
>> On 30/10/18 11:18, Andre Vieira (lists) wrote:
>>> 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
>>>
>> Ping
Ping