[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