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

Thomas Preudhomme thomas.preudhomme@linaro.org
Mon Nov 19 13:40:00 GMT 2018


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



More information about the Binutils mailing list