[PATCH] D91611: [PowerPC][LLD] Detecting and fixing missing TLS relocation on __tls_get_addr

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

I have done some testing with `gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0` and `GNU ld (GNU Binutils for Ubuntu) 2.30`.

It appears that `ld.bfd` by default fixes up the relocation if it is missing and then does the relaxation correctly. I don't need to specify any options for this to be done.
As a simple example:

  Test1:
  .LCF0:
    addis 2,12,.TOC.-.LCF0 at ha
    addi 2,2,.TOC.-.LCF0 at l
    .localentry     Test1,.-Test1
    addis 3,2,x at got@tlsgd at ha
    addi 3,3,x at got@tlsgd at l
    bl __tls_get_addr
    nop

The `bl __tls_get_addr` is missing the relocation `ld.bfd` still relaxes it correctly.
However, with a more complex example that I wrote by hand:

  Test1:
  .LCF0:
    addis 2,12,.TOC.-.LCF0 at ha
    addi 2,2,.TOC.-.LCF0 at l
    .localentry     Test1,.-Test1
    addis 3,2,x at got@tlsgd at ha
    addi 3,3,x at got@tlsgd at l
    addis 3,2,x at got@tlsgd at ha
    addi 3,3,x at got@tlsgd at l
    bl __tls_get_addr
    nop

in the above case `ld.bfd` does not recognize the pattern for the TLS and so it turns off TLS relaxations completely.
The reason I brought up these examples is because I feel that detecting the difference between the two cases in lld would be unreasonably complicated an probably not worth it. As a result I feel that it may not be feasible to do the same thing that `ld.bfd` is doing.

Here is my new proposal:
We can add an option to `lld`. Unfortunately this option will not exist in `ld.bfd` because the wouldn't need it.
The option is off by default.
If the option is off then `lld` will behave exactly the same way that it always has. No error detection for missing relocations.
If the option is on then `lld` will check all calls to `__get_tls_addr` and if it finds a missing relocation it will turn off TLS relaxations. It won't try to add the missing relocation and it won't throw an error.
This design will not cause problems to default behaviour (and it shouldn't add overhead to the default link either) but it will also allow linking with code that was generated with the legacy compiler if the user specifies the option.

How does this new idea sound?
If everyone agrees on this idea (or a variant of this idea) I will rework the patch like this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91611/new/

https://reviews.llvm.org/D91611