[AArch64] Don't generate GOT entry for large model TLS LE relocation

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[AArch64] Don't generate GOT entry for large model TLS LE relocation

Jiong Wang-4
This patch revert the following commit

commit b7a944fea3a0194d81f6de4d958f3a1d2c6ad03a
Author: Renlin Li <[hidden email]>
Date:   Fri Oct 2 17:02:53 2015 +0100

     [BFD][AARCH64]Create GOT section for TLSLE_MOVW_TPREL_G(1, 1_NC, 2).

     bfd/

     2015-10-02  Renlin Li <[hidden email]>

         * elfnn-aarch64.c (elfNN_aarch64_check_relocs): Create GOT section
         for TLSLE_MOVW_TPREL_G(1, 1_NC, 2) relocation.

We shouldn't generate GOT entries for TLS Local Executable relocation
types as they don't need GOT.

This commit was trying to fix the following problem:

   1. In large code model, the instruction sequences for TLS traditional
      general dynamic will be:

        movz a0, #:tlsgd_g1:var      R_AARCH64_TLSGD_MOVW_G1
        movk a0, #:tlsgd_g0_nc:var   R_AARCH64_TLSGD_MOVW_G0_NC
        add  a0, gp, a0
        bl   __tls_get_addr        R_AARCH64_CALL26
        nop

      While in large code model, GOT section might be very far from text
      section that is out of the addressing range of our pc-relative
      instructions, so need to initialize gp register by loading the
      address from literal pool, then the full instruction sequences will
      become the following:


         ldr     x1, .Lgot
         adr     x2, .Lgot
         add     x1, x2, x1

         movz    x0, #:tlsgd_g1:var
         movk    x0, #:tlsgd_g0_nc:var
         add     x0, x1, x0
         bl      __tls_get_addr
         nop

.Lgot:
         .dword _GLOBAL_OFFSET_TABLE_ - .  // R_AARCH64_PREL64 _GOT_


    2. When TLS relaxation happen, all GD relocations will be turned into
       LE relocation types which don't need GOT support, but the PREL64
       relocation for literal pool is still referencing the special
       symbol _GLOBAL_OFFSET_TABLE_ which is defined by linker to the
       start address of .got section.  We can't remove it, and we can't
       remove the symbol, as gp may needed in other places across this
       object, or even gp is only used in above TLS sequences, it will be
       impossible to remove the literal pool entry and the symbol under
       current BFD linker intrastructure.

       So if TLS GD -> LE relaxation happen, and if there is no
       relocation types which require GOT support, then .got section
       won't be generated, that this symbol will not be defined, we will
       end up with a undefined symbol static linking error.


   The correct approach to fix this is not by cheating linker that large
   model LE relocation require GOT support, but we should always generate
   .got section if a relocation has referenced the special symbol
   _GLOBAL_OFFSET_TABLE_ during check_relocs.  This can fix all
   situations where _GLOBAL_OFFSET_TABLE_ are referenced.

   This is also the approach ppc is using.

   build & check-ld ok.  OK for trunk and 2.26?

2015-12-23  Jiong Wang  <[hidden email]>

bfd/
   * elfnn-aarch64.c (elfNN_aarch64_check_relocs): Require .got section
   is a relocation referenced _GLOBAL_OFFSET_TABLE_, meanwhile don't
   generate GOT entry for large model TLS LE relocation types.


rb5021.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [AArch64] Don't generate GOT entry for large model TLS LE relocation

Marcus
On 23 December 2015 at 16:55, Jiong Wang <[hidden email]> wrote:

> This patch revert the following commit

> commit b7a944fea3a0194d81f6de4d958f3a1d2c6ad03a
> Author: Renlin Li <[hidden email]>
> Date:   Fri Oct 2 17:02:53 2015 +0100

I have not looked at this in detail, I'm out of the office at the
moment, but I did notice that this is more than a revert!

Please split this into two patches.

1) The revert request.
2) The new functionality you want to add.

I think the revert is safe because it is a relaxation, leaving the
original sequence in place should be fine, also it only affects large
model and the default model is small. I think the patch being reverted
here may have had test cases submitted later as a separate patch?

The revert part of this is IMO going to be suitable for 2.26, however
I don;t think we should be taking the new functionality part on 2.26,
at least not until it has baked on the trunk for a while.

Cheers
/Marcus
Reply | Threaded
Open this post in threaded view
|

Re: [AArch64] Don't generate GOT entry for large model TLS LE relocation

Jiong Wang-4


On 23/12/15 18:10, Marcus Shawcroft wrote:

> On 23 December 2015 at 16:55, Jiong Wang <[hidden email]> wrote:
>
>> This patch revert the following commit
>> commit b7a944fea3a0194d81f6de4d958f3a1d2c6ad03a
>> Author: Renlin Li <[hidden email]>
>> Date:   Fri Oct 2 17:02:53 2015 +0100
> I have not looked at this in detail, I'm out of the office at the
> moment, but I did notice that this is more than a revert!
>
> Please split this into two patches.
>
> 1) The revert request.
> 2) The new functionality you want to add.
>
> I think the revert is safe because it is a relaxation, leaving the
> original sequence in place should be fine, also it only affects large
> model and the default model is small. I think the patch being reverted
> here may have had test cases submitted later as a separate patch?

Yes. the patch being reverted was trying the fix bug exposed by testcase

  ld-aarch64/tls-relax-large-gd-le

which is submitted in the next commit. So my concern was a pure revert
of the
patch will actually cause regression on above testcase. Thus I
integrated the
correct fix from my understanding to make sure no regression will happen.

>
> The revert part of this is IMO going to be suitable for 2.26, however
> I don;t think we should be taking the new functionality part on 2.26,
> at least not until it has baked on the trunk for a while.
>
> Cheers
> /Marcus

Reply | Threaded
Open this post in threaded view
|

Re: [AArch64] Don't generate GOT entry for large model TLS LE relocation

Jiong Wang-4
In reply to this post by Marcus
On 23/12/15 18:10, Marcus Shawcroft wrote:

> On 23 December 2015 at 16:55, Jiong Wang <[hidden email]> wrote:
>
>> This patch revert the following commit
>> commit b7a944fea3a0194d81f6de4d958f3a1d2c6ad03a
>> Author: Renlin Li <[hidden email]>
>> Date:   Fri Oct 2 17:02:53 2015 +0100
> I have not looked at this in detail, I'm out of the office at the
> moment, but I did notice that this is more than a revert!
>
> Please split this into two patches.
>
> 1) The revert request.
> 2) The new functionality you want to add.
OK. Attachment is a pure revert of related commits:

the following two commits:
===

commit b7a944fea3a0194d81f6de4d958f3a1d2c6ad03a
Author: Renlin Li <[hidden email]>
Date:   Fri Oct 2 17:02:53 2015 +0100

     [BFD][AARCH64]Create GOT section for TLSLE_MOVW_TPREL_G(1, 1_NC, 2).

commit ac734732481451698ee23990aaa64907e56dd082
Author: Renlin Li <[hidden email]>
Date:   Fri Oct 2 17:22:36 2015 +0100

     [BFD][AARCH64]Add TLSGD relaxation support under large memory model.


And tls relaxation part of:
===
commit 0484b4549e9e2802e2f9db30a61f4b2a76332a8f
Author: Renlin Li <[hidden email]>
Date:   Fri Oct 2 17:43:08 2015 +0100

     [LD][AARCH64]Add TLSDESC support for large memory model.

no ld regression on both master and 2.26 branch.

OK to master and 2.26 branch?

2016-01-04  Jiong Wang  <[hidden email]>

bfd/
   Revert
   2015-10-02  Renlin Li  <[hidden email]>

         * elfnn-aarch64.c (aarch64_tls_transition_without_check):  Add
         relax transitions for TLSDESC_ADD, TLSDESC_LDR, TLSDESC_OFF_G0_NC,
         TLSDESC_OFF_G1.
         (aarch64_tls_transition_without_check): Add relaxation support.
         (elfNN_aarch64_tls_relax): Likewise.

2015-10-02  Renlin Li  <[hidden email]>

         * elfnn-aarch64.c(IS_AARCH64_TLS_RELAX_RELOC):
         Add relaxation support for TLSGD_MOVW_G0_NC and TLSGD_MOVW_G1.
         (aarch64_tls_transition_without_check): Likewise
         (elfNN_aarch64_tls_relax): Likwise.

2015-10-02  Renlin Li  <[hidden email]>

         * elfnn-aarch64.c (elfNN_aarch64_check_relocs): Create GOT section
         for TLSLE_MOVW_TPREL_G(1, 1_NC, 2) relocation.

ld/testsuite/ld-aarch64/
   Revert
   2015-10-02  Renlin Li  <[hidden email]>

         * ld-aarch64/aarch64-elf.exp: Run new test.
         * ld-aarch64/tls-relax-large-desc-ie.d: New.
         * ld-aarch64/tls-relax-large-desc-ie.s: New.
         * ld-aarch64/tls-relax-large-desc-le.d: New.
         * ld-aarch64/tls-relax-large-desc-le.s: New.

   2015-10-02  Renlin Li  <[hidden email]>

         * ld-aarch64/aarch64-elf.exp: run new test
         * ld-aarch64/tls-relax-large-gd-ie.d: New.
         * ld-aarch64/tls-relax-large-gd-ie.s: New.
         * ld-aarch64/tls-relax-large-gd-le.d: New.
         * ld-aarch64/tls-relax-large-gd-le.s: New.


newest.patch (17K) Download Attachment