llvm dev - Jun 2016 - [LLD] thunk implementation correctness depends on order of input section.

First of all thanks for finding the bug.

I agree with Rui that right now we can manage without general thunk
infrastructure. Let's provide at least a few "thunk"
implementation
and after that reconsider necessity of common thunk framework. As to
MIPS there is one more type of thunk (keyword is .MIPS.stubs) and one
more optimization of current thunk (putting a thunk in the beginning
of the section if the section contains the only function required the
thunk). But I do not plan to implement these features in the near
future.

On Wed, Jun 22, 2016 at 5:29 AM, Rui Ueyama <ruiu at google.com>
wrote:
> The first think I'd want to say is to not try too hard to use the
existing
> "framework" in LLD. The current thunk mechanism was made just for
MIPS
> non-PIC/PIC function calls and are not proven to be generic or useful for
> other purposes. We are cool with that because it just works for MIPS
(except
> the bug you found) and satisfies our needs. And the amount of code for MIPS
> thunk is so small that we are not serious about reusing it. Probably the
> name given to it ("thunk") is too generic -- maybe we should
rename it
> MipsLA25Thunk or something like that. I think you want to create a new type
> of thunk for ARM.
>
> The bug that we sometimes create broken MIPS thunks seems to have
introduced
> in r265673 which Rafael made. Before that patch, we didn't assume that
> section VAs are available in scanRelocs. I think we want to revert that
> change (although it cannot simply be reverted because the patch was
> submitted in April, and many changes has been made on it since then.)
>
> Rafael, can you take at that change?
>
> On Tue, Jun 21, 2016 at 9:38 PM, Peter Smith <peter.smith at
linaro.org> wrote:
>>
>> I've been working on supporting ARM/Thumb interworking thunks in
LLD
>> and have encountered a limitation that I think it is worth bringing up
>> in a wider context. This is all LLD specific, apologies if I've
abused
>> llvm-dev here.
>>
>> TL;DR summary:
>> - Thunks in lld may not work if they are added to InputSections that
>> have already been scanned.
>> - There is a short term fix, but in the longer term I think that we
>> will want to allow multiple passes of the relocations.
>> - I'd like to know if there are any preferences on a solution.
-- 
Simon Atanasyan

Thank you both for the feedback.

I think that there will be a good deal of commonality between
architectures that need thunks. I agree that it will be easier to
generalise later when we need to.

ARM also has an optimisation of a thunk at the start of the section.
In effect change state and fall through to a function at the start of
the section. It is also quite a bit down the priority list.

Peter

On 22 June 2016 at 06:40, Simon Atanasyan <simon at atanasyan.com>
wrote:
> First of all thanks for finding the bug.
>
> I agree with Rui that right now we can manage without general thunk
> infrastructure. Let's provide at least a few "thunk"
implementation
> and after that reconsider necessity of common thunk framework. As to
> MIPS there is one more type of thunk (keyword is .MIPS.stubs) and one
> more optimization of current thunk (putting a thunk in the beginning
> of the section if the section contains the only function required the
> thunk). But I do not plan to implement these features in the near
> future.
>
> On Wed, Jun 22, 2016 at 5:29 AM, Rui Ueyama <ruiu at google.com>
wrote:
>> The first think I'd want to say is to not try too hard to use the
existing
>> "framework" in LLD. The current thunk mechanism was made just
for MIPS
>> non-PIC/PIC function calls and are not proven to be generic or useful
for
>> other purposes. We are cool with that because it just works for MIPS
(except
>> the bug you found) and satisfies our needs. And the amount of code for
MIPS
>> thunk is so small that we are not serious about reusing it. Probably
the
>> name given to it ("thunk") is too generic -- maybe we should
rename it
>> MipsLA25Thunk or something like that. I think you want to create a new
type
>> of thunk for ARM.
>>
>> The bug that we sometimes create broken MIPS thunks seems to have
introduced
>> in r265673 which Rafael made. Before that patch, we didn't assume
that
>> section VAs are available in scanRelocs. I think we want to revert that
>> change (although it cannot simply be reverted because the patch was
>> submitted in April, and many changes has been made on it since then.)
>>
>> Rafael, can you take at that change?
>>
>> On Tue, Jun 21, 2016 at 9:38 PM, Peter Smith <peter.smith at
linaro.org> wrote:
>>>
>>> I've been working on supporting ARM/Thumb interworking thunks
in LLD
>>> and have encountered a limitation that I think it is worth bringing
up
>>> in a wider context. This is all LLD specific, apologies if I've
abused
>>> llvm-dev here.
>>>
>>> TL;DR summary:
>>> - Thunks in lld may not work if they are added to InputSections
that
>>> have already been scanned.
>>> - There is a short term fix, but in the longer term I think that we
>>> will want to allow multiple passes of the relocations.
>>> - I'd like to know if there are any preferences on a solution.
>
> --
> Simon Atanasyan

Not exactly fall through :-)  It has to form the address of the other entry
point (+1 for Thumb) in a temp register and then BX to it. It just means
the constant added to the PC is very small...


On Wed, Jun 22, 2016 at 12:06 PM, Peter Smith via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Thank you both for the feedback.
>
> I think that there will be a good deal of commonality between
> architectures that need thunks. I agree that it will be easier to
> generalise later when we need to.
>
> ARM also has an optimisation of a thunk at the start of the section.
> In effect change state and fall through to a function at the start of
> the section. It is also quite a bit down the priority list.
>
> Peter
>
> On 22 June 2016 at 06:40, Simon Atanasyan <simon at atanasyan.com>
wrote:
> > First of all thanks for finding the bug.
> >
> > I agree with Rui that right now we can manage without general thunk
> > infrastructure. Let's provide at least a few "thunk"
implementation
> > and after that reconsider necessity of common thunk framework. As to
> > MIPS there is one more type of thunk (keyword is .MIPS.stubs) and one
> > more optimization of current thunk (putting a thunk in the beginning
> > of the section if the section contains the only function required the
> > thunk). But I do not plan to implement these features in the near
> > future.
> >
> > On Wed, Jun 22, 2016 at 5:29 AM, Rui Ueyama <ruiu at google.com>
wrote:
> >> The first think I'd want to say is to not try too hard to use
the
> existing
> >> "framework" in LLD. The current thunk mechanism was made
just for MIPS
> >> non-PIC/PIC function calls and are not proven to be generic or
useful
> for
> >> other purposes. We are cool with that because it just works for
MIPS
> (except
> >> the bug you found) and satisfies our needs. And the amount of code
for
> MIPS
> >> thunk is so small that we are not serious about reusing it.
Probably the
> >> name given to it ("thunk") is too generic -- maybe we
should rename it
> >> MipsLA25Thunk or something like that. I think you want to create a
new
> type
> >> of thunk for ARM.
> >>
> >> The bug that we sometimes create broken MIPS thunks seems to have
> introduced
> >> in r265673 which Rafael made. Before that patch, we didn't
assume that
> >> section VAs are available in scanRelocs. I think we want to revert
that
> >> change (although it cannot simply be reverted because the patch
was
> >> submitted in April, and many changes has been made on it since
then.)
> >>
> >> Rafael, can you take at that change?
> >>
> >> On Tue, Jun 21, 2016 at 9:38 PM, Peter Smith <peter.smith at
linaro.org>
> wrote:
> >>>
> >>> I've been working on supporting ARM/Thumb interworking
thunks in LLD
> >>> and have encountered a limitation that I think it is worth
bringing up
> >>> in a wider context. This is all LLD specific, apologies if
I've abused
> >>> llvm-dev here.
> >>>
> >>> TL;DR summary:
> >>> - Thunks in lld may not work if they are added to
InputSections that
> >>> have already been scanned.
> >>> - There is a short term fix, but in the longer term I think
that we
> >>> will want to allow multiple passes of the relocations.
> >>> - I'd like to know if there are any preferences on a
solution.
> >
> > --
> > Simon Atanasyan
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160622/e74f1c8d/attachment.html>