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

Thu May 25 08:48:22 PDT 2017

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.