[PATCH] D33436: [ARM] Create relocation for Thumb functions calling ARM fns.

[Peter Smith via llvm-commits]

I'm happy to go with option 1 as a simple first step.

Peter

On 25 May 2017 at 16:48, Florian Hahn <florian.hahn at arm.com> wrote:
> Following up on that, I think we have the following options going forward:
>
> (1) update this patch to only emit relocations for branches to function
> symbols that are not marked as Thumb and assume no change in execution
> mode happens for branches to non function symbols (that's the current
> behaviour I think)
>
> (2) introduce a new set to keep track of non-function symbols in Thumb
> functions and emit a warning/error in case we detect a change in
> execution mode for branches to non-function symbols
>
> (3) Add non-function symbols in Thumb functions to the ThumbFuncs set in
> MCAssembler (and rename that to ThumbSymbols). That would mean that the
> LSB for non-function symbols in Thumb functions will be set to 1 and we
> could emit relocations for branches to non-function symbols changing
> execution mode, giving the linker a better chance to do the right thing.
>
>
> Personally I think we should go with option (1) at first, as it's simple
> and should do the right thing for ABI compliant code. Maybe we could add
> a warning if the execution mode changes after that. What do you
> think/prefer?
>
>
> Cheers,
> Florian
>
>
> On 25/05/2017 12:41, Peter Smith wrote:
>>
>> I've gone back and read the ABI carefully and took a look at how some
>> existing linkers actually behave. I think I understand your concerns
>> about local labels a bit more now as I don't think that there is an
>> easy answer.
>>
>> The TL;DR is:
>> - The output from the assembler is legal.
>> - The ABI isn't very clear about what an error message should be given
>> for. In summary it seems to be if you detect you can't resolve the
>> relocation correctly you should give an error message. However
>> detecting an error in this case requires using the mapping symbols,
>> which are optional.
>> - I think it is a non-ABI compliant program if there is a B or BL to a
>> STT_NOTYPE symbol AND interworking is required. Unfortunately there
>> may be quite a few of these programs out there that happen to work
>> with some linkers.
>> - It is not always safe to output a branch type relocation to a
>> STT_NOTYPE symbol as some linkers have bugs [*] and may not get the
>> answer right by trying to interwork and getting the answer wrong.
>>
>> [*] lld (mea culpa) and gold attempt to interwork even when the symbol
>> has STT_NOTYPE and get the answer wrong for Thumb because they
>> interpret the symbol without bit 0 set as ARM. It happens to work for
>> ARM because bit 0 not set is also ARM.
>>
>> Long version within the context of the original review:
>>
>> Taking your example, it is legal according the to the ABI as to
>> paraphrase:
>> - All code symbols exported from an object file (symbols with binding
>> STB_GLOBAL) shall have type STT_FUNC
>> - The type of any other symbol defined in an executable section can be
>> STT_NOTYPE.
>> - Symbols with type STT_FUNC have the additional rules:
>> -- Bit 0 is clear if the symbol addresses ARM, Bit 0 is set if the
>> symbol addresses 1
>>
>> A branch to a STT_NOTYPE instruction is permitted to the ABI, however:
>> - "The linker is only required to provide interworking support for
>> symbols of type STT_FUNC (interworking for untyped symbols must be
>> encoded directly in the object file)."
>> So we can hope that a linker can fix up the relocation, for example by
>> using the mapping symbols to work out whether the symbol is ARM or
>> Thumb, or give an error message if it doesn't know the ARM/Thumb
>> state. Unfortunately in practice it seems like at least two linkers
>> (lld and gold) just try and interwork and give the wrong answer.
>>
>> With this in mind I think:
>> - It is always safe to output the relocation when the target symbol
>> has type STT_FUNC and we should follow the same behaviour as ARM.
>> - When the target symbol definition has type STT_NOTYPE and no
>> interworking is needed we can resolve the fixup at assembly time.
>> - When the target symbol definition has type STT_NOTYPE and we detect
>> interworking is needed then we should output the relocation and do one
>> of:
>> -- Output a warning that symbol must have type STT_FUNC to guarantee
>> ARM/Thumb interworking.
>> -- Change the symbol type to STT_FUNC and set the Thumb bit appropriately.
>>
>> For the linker:
>> - It is permissible to use additional information such as mapping
>> symbols to work out the ARM/Thumb state of the STT_NOTYPE symbol, it
>> can give an error message or perform the interworking as if the symbol
>> had type STT_FUNC.
>> - In absence of additional information the linker may trust that all
>> interworking has been done by the object producer and hence it should
>> not substitute BLX for BL or use a stub/thunk/veneer to interwork.
>>
>> Peter
>>
>> On 25 May 2017 at 00:12, Rafael Avila de Espindola
>> <rafael.espindola at gmail.com> wrote:
>>>
>>> Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:
>>>
>>>> The ABI only permits a linker to do interworking (such as changing a BL
>>>> to BLX) for a relocation if the symbol has type STT_FUNC and the relocation
>>>> is one of R_ARM_CALL, R_ARM_JUMP24, R_ARM_THM_CALL, R_ARM_THM_JUMP22,
>>>> R_ARM_THM_JUMP19, which match ARM BL/BLX, ARM B, Thumb BL/BLX, Thumb B.w,
>>>> Thumb B.w<cc>. The narrow 16-bit Thumb branch instructions are excluded. The
>>>> linker should give an error messages if it detects that an ARM/Thumb state
>>>> change is required outside of these conditions. So I think that adding a
>>>> relocation wouldn't make correct code generated by the assembler incorrect,
>>>> and it would give the linker a chance to detect the error. I note that for
>>>> ARM state callers we just mark all the branch type relocations as
>>>> IsResolved=false, and I suggest we do the same for wide Thumb branch type
>>>> relocations as well.
>>>>
>>>> If there is a BL/BLX to an internal label that doesn't have type
>>>> STT_FUNC (and may not have the thumb bit set) then the linker is not
>>>> supposed to try and do interworking. According to the ABI it is the
>>>> programmers responsibility to be in the correct ARM/Thumb state before
>>>> writing the instruction in this case. I think the ideal behaviour would be
>>>> to issue an error message, but we could rely on the linker to do it.
>>>
>>>
>>> But given simple assembly like
>>>
>>> ----------------------------
>>>          .text
>>>          .syntax unified
>>>          .globl  f
>>>          .p2align        1
>>>          .type   f,%function
>>>          .code   16
>>>          .thumb_func
>>> f:
>>> foo:
>>>          nop
>>> bar:
>>> ---------------------------
>>>
>>> only f is STT_THUMB and has the lsb set:
>>>
>>>       2: 00000002     0 NOTYPE  LOCAL  DEFAULT    2 bar
>>>       3: 00000000     0 NOTYPE  LOCAL  DEFAULT    2 foo
>>>       4: 00000001     0 FUNC    GLOBAL DEFAULT    2 f
>>>
>>> Is that a bug in the assembler or invalid assembly? Should the linker
>>> report an error if there was a branch to foo or bar?
>>>
>>> Cheers,
>>> Rafael
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.