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

[Peter Smith via llvm-commits]

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