Re: [16/16][binutils][AARCH64]Add relocation support for large memory model. [LD]Add TLSIE relaxation support.

[Marcus Shawcroft <marcus dot shawcroft at gmail dot com>]

On 8 September 2015 at 19:04, Renlin Li <renlin.li@arm.com> wrote:
> Hi all,
>
> This patch add the TLSIE to TLSLE relax for large memory model. We can do
> the following relaxation if the symbol is local.
>
>      mrs tp, tpidr_el0
>      movz tx, #:gottprel_g1:var       => movz tx, #:tprel_g2:var, lsl #32
>      movk tx, #:gottprel_g0_nc:var  => movk tx, #:tprel_g1_nc:var, lsl #16
>      ldr tx, [gp, tx]                             => movk tx,
> #:tprel_g0_nc:var
>      add tx, tx, tp
>
> At the moment, the offset from TP is already known. One addition argument is
> added to elfNN_aarch64_tls_relax function to pass this information inside.
> And during the relaxation, it's directly encoded into the instruction word,
> and the relocation are removed, mark them as resolved.
>
> binutils, ld regresstion test Okay without issues.


Hi,

This patch causes  https://sourceware.org/bugzilla/show_bug.cgi?id=19188.

The underlying issue is that when this relaxation was added it became
apparent that the existing structure was inadequate to handle one
particular relaxation.  The code is structured such that relaxations
are applied in elfNN_aarch64_tls_relax which modifies the instruction
stream and when necessary  mutates an existing relocation on a
modified instruction into an alternative relocation (or simply nulls
out the relocation).  All relocations are then handled under the final
link_relocate function. This structure has been adequate up until the
point at which a relaxation was implemented that needs to apply more
relocations on the relaxed code than were applied on the input
sequence.  In this situation  elfNN_aarch64_tls_relax has insufficient
relocation slots available.  The solution chosen in this patch was to
special case this relaxation and pipe in a final relocation value in
order to allow elfNN_aarch64_tls_relax to both modify the instruction
stream and fixup one relocation in this particular case.

This seems like a dubious choice.  The existing separation of
elfNN_aarch64_tls_relax and final_link_relocate has proved inadequate
and needs re-thinking / re-designing. Unfortunately I missed this
patch going through review.

We should rethink the implementation of relaxation such that all
relaxations can be handled without resorting to special case point
fixes, having looked at the code I see no quick / simple refactor.

In order to deal with 19188 we have two choices:

1) We can add further special casing of the offending relocation.
2) We can revert the patch while looking for a more appropriate
general solution.

Given that this patch implements large memory model IE->LE relaxation
(the default memory model on aarch64 is small memory model). The
number of folks / apps that are going to miss this relaxation is
likely miniscule.

I am aware that 19188 causes wide spread failures in all memory models
and is causing significant pain to at least the debian folk.

I'm in favour of 2) rather than 1) on the basis that we can live
without this relaxation  and option 1) is a band aid solution.

Nick do you have any objections to this course of action?

Cheers
/Marcus

> Regards,
> Renlin Li
>
> bfd/ChangeLog:
>
> 2015-09-08  Renlin Li <renlin.li@arm.com>
>
>     * elfnn-aarch64.c (IS_AARCH64_TLS_RELAX_RELOC): Add
>         TLSIE_MOVW_GOTTPREL_G1.
>     (aarch64_tls_transition_without_check): Add
>         TLSIE_MOVW_GOTTPREL_G1 to TLSLE_MOVW_TPREL_G2
>         transition for local symbol.
>     (elfNN_aarch64_tls_relax): Add a argument to pass tp offset.
>         Add TLSIE_MOVW_GOTTPREL_G1 relaxation.
>     (elfNN_aarch64_relocate_section): Call elfNN_aarch64_tls_relax
>         with new argument.
>
> ld/testsuite/ChangeLog:
>
> 2015-09-08  Renlin Li <renlin.li@arm.com>
>
>     * ld-aarch64/aarch64-elf.exp (tls-relax-large-le-ie): Run new test.
>     * ld-aarch64/tls-relax-large-ie-le.d: New.
>     * ld-aarch64/tls-relax-large-ie-le.s: New.