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

[stefanp]

Summary

This is a replacement and extension for this patch:
https://reviews.llvm.org/D85994

The standard representation of Thread Local Storage (TLS) is as follows:

addis r3, r2, x@got@tlsgd@ha // R_PPC64_GOT_TLSGD16_HA
addi r3, r3, x@got@tlsgd@l // R_PPC64_GOT_TLSGD16_LO
bl __tls_get_addr(x@tlsgd) // R_PPC64_TLSGD followed by R_PPC64_REL24
nop

The above is specified in the Power PC ELFv2 ABI in section:
3.7.3.1. General Dynamic TLS Model

However, there are two deviations form the above that have been seen in actual
code.

1. A direct call to __tls_get_addr.

bl __tls_get_addr
nop

2. Missing R_PPC64_TLSGD relocation.

addis r3, r2, x@got@tlsgd@ha // R_PPC64_GOT_TLSGD16_HA
addi r3, r3, x@got@tlsgd@l // R_PPC64_GOT_TLSGD16_LO
bl __tls_get_addr // R_PPC64_REL24
nop

A direct call does not cause lld to produce incorrect code gen. If
the user is careful enough to correctly setup the inputs to the function the
code will work. Also, it appears that some implementations of ld.so do this
direct call.

The second situation where the R_PPC64_TLSGD relocation is missing is a
different story. Generating the second code sequence will cause lld to produce
incorrect code gen when relaxing the sequence to Local Exec. The first two
relocations (R_PPC64_GOT_TLSGD16_HA and R_PPC64_GOT_TLSGD16_LO) will be relaxed
correctly but since the call is missing a relocation it will be left as-is
which will ultimately result in an incorrect TLS address being computed.

This patch tries to address the problem of the missing R_PPC64_TLSGD relocaiton.

An option has been added --fix-ppc64-tls-reloc.
By default if the option is not specified lld will behave just as it has done in the past.
If the option is specified then lld will check all calls to __tls_get_addr for the missing relocation.
If the relocation is missing then TLS relaxation will be turned off so the the incorrect
code is not produced.

[MaskRay]

I remember that I have questioned about the error checking when the patch was initially brought up. The introduction of a new linker option for an incorrect case where compiler will not generate such code makes me feel more uneasy about it. Surely the linker can detect such errors but does it really have to? With assembly the user can play all sorts of tricks which will not work, but is that the duty of the linker?

[NeHuang]

lld/test/ELF/ppc64-tls-add-missing-gdld.s

31

do we need to add two test cases here to cover?

• add missing tls relocation for __tls_get_addr@notoc(x@tlsgd)
• add missing tls relocation for __tls_get_addr@notoc(x@tlsld)

[sfertile]

lld/ELF/Relocations.cpp

1316

I don't think this is something you can reliably determine. For example consider the code:

# r30 is previously saved to the stack.
# setup r3 for the call to __tls_get_addr
 addis 3, 2, i@got@tlsgd@ha
 addi 3, 3, i@got@tlsgd@l
# Load unrelated variable into a non-volatile register so it will stay live across the call.
 addis 30, 2, j@toc@ha
 addi 30, j@toc@l(30)
 # call __tls_get_addr. Relocation I - 2 is NOT one of the relocations used to setup the argument.
 bl __tls_get_addr(i@tlsgd)
 nop

[stefanp]

In D91611#2400509, @MaskRay wrote:

I remember that I have questioned about the error checking when the patch was initially brought up. The introduction of a new linker option for an incorrect case where compiler will not generate such code makes me feel more uneasy about it. Surely the linker can detect such errors but does it really have to? With assembly the user can play all sorts of tricks which will not work, but is that the duty of the linker?

I agree that we cannot generate this sequence from C source using clang. However, the reason I am still trying to detect this problem is because we know that we have older versions of the XLC compiler that actually produced the incorrect setup with the missing relocation and we end up with bad code gen when linking in binaries with that missing relocation. These incorrectly compiled binaries do exist in real life and the users may have no idea that they have been compiled incorrectly.

lld/ELF/Relocations.cpp

1316

I agree.
However, in the above situation we are no worse off than we are now. Right now we fail to detect all such situations. If we add this patch we will detect most of these situations but as you mentioned above we can miss a few.

If we want to get them all we have to also flag the direct call to __tls_get_addr but I'm trying to avoid that situation.

Another option is to search up until I find a relocation that is either

1. @tlsgd which indicates an error.

OR

2. Function call which indicates we should stop the search without an error.
3. Finished the search and nothing was found of the other cases in which case we stop without an error.

lld/test/ELF/ppc64-tls-add-missing-gdld.s

31

Yes I can add the @notoc versions.
Good catch. I missed those.

[MaskRay]

Both forms are technically not valid according to the ABI. No new code should be producing either of the two forms.

FWIW currently all ppc64 ld.so implementations I know (glibc/musl/FreeBSD rtld-elf) need to call a raw __tls_get_addr. This is a requisite.

The second situation where the R_PPC64_TLSGD relocation is missing is a different story. Generating the second code sequence will cause lld to produce incorrect code gen when relaxing the sequence to Local Exec.

I agree with the assertion but the complexity makes me feel uneasy. If it is a property which can be checked simply/efficiently/relatively prevailing in the wild, sure we should add it. Such examples include: absolute relocations referencing a non-absolute symbol; non-absolute relocation referencing an absolute symbol in -pie/-shared mode; relocations referencing local symbols defined in a discarded COMDAT; ...

These diagnostics are all fairly common and checking have little overhead in code and runtime costs.
However, this is not true for the code added in this patch. The long enumeration of relocation types make
the readable LLD bring closer to the hard-to-read bfd/elf*-ppc.c
String comparison coming with the check as common as type == R_PPC64_REL24 is harmful to the performance.

I am still questioning the motivation behind this change. If there are indeed ABI issues from older compilers,
can they add a wrapper around the linker if the check is deemed useful?

(In addition, binutils does not have --add-missing-tls-reloc.)

[nemanjai]

In D91611#2412710, @MaskRay wrote:

I am still questioning the motivation behind this change. If there are indeed ABI issues from older compilers,
can they add a wrapper around the linker if the check is deemed useful?

I don't really see it as an improvement in usability to have to post-process object files, potentially introducing another possible point of failure vs. providing an option to the linker.

(In addition, binutils does not have --add-missing-tls-reloc.)

I think that a departure from option compatibility is reasonable here if we can provide an option that will:

• Allow us to handle the missing relocation when the option is specified
• Not introduce any performance overhead when the option is not specified (i.e. the default behaviour)

Note that I am not saying this option achieves both of those goals.
It would appear to me that we really have 3 situations here that are kind of at odds with each other:

1. Typical compiler-introduced calls to __tls_get_addr with the necessary relocations for the symbol
2. The weird usage in ld.so with a missing relocation that does not require a relocation as there is no symbol
3. A non-ABI compliant compiler-introduced call with a value associated with a symbol in the parameter register but a missing relocation

The current state is that the linker does not produce errors on any of those but produces incorrect code for 3. Adding this patch introduces performance overhead by default and handles 3. by either emitting an error or fixing up the relocation to avoid producing incorrect code. It has the further issue that it assumes the immediately preceding relocation was for setting up the parameter to __tls_get_addr which may not be the case as @sfertile pointed out.
So can we do something like this:

• Leave default behaviour as it is now
• Provide an option to search for and fix up missing relocations and error out if it cannot

Case 2. above would not work with this option, but that really doesn't seem like a problem as builds that do that will never specify the option. Furthermore, we will continue to emit incorrect code for case 3. above by default, but that is also not a problem as the documentation can suggest using the option whenever some objects are produced by older versions of the XL compiler.

[MaskRay]

In D91611#2413841, @nemanjai wrote:

In D91611#2412710, @MaskRay wrote:

I am still questioning the motivation behind this change. If there are indeed ABI issues from older compilers,
can they add a wrapper around the linker if the check is deemed useful?

I don't really see it as an improvement in usability to have to post-process object files, potentially introducing another possible point of failure vs. providing an option to the linker.

(In addition, binutils does not have --add-missing-tls-reloc.)

I think that a departure from option compatibility is reasonable here if we can provide an option that will:

• Allow us to handle the missing relocation when the option is specified
• Not introduce any performance overhead when the option is not specified (i.e. the default behaviour)

Note that I am not saying this option achieves both of those goals.
It would appear to me that we really have 3 situations here that are kind of at odds with each other:

1. Typical compiler-introduced calls to __tls_get_addr with the necessary relocations for the symbol
2. The weird usage in ld.so with a missing relocation that does not require a relocation as there is no symbol
3. A non-ABI compliant compiler-introduced call with a value associated with a symbol in the parameter register but a missing relocation

The current state is that the linker does not produce errors on any of those but produces incorrect code for 3. Adding this patch introduces performance overhead by default and handles 3. by either emitting an error or fixing up the relocation to avoid producing incorrect code. It has the further issue that it assumes the immediately preceding relocation was for setting up the parameter to __tls_get_addr which may not be the case as @sfertile pointed out.
So can we do something like this:

• Leave default behaviour as it is now
• Provide an option to search for and fix up missing relocations and error out if it cannot

Case 2. above would not work with this option, but that really doesn't seem like a problem as builds that do that will never specify the option. Furthermore, we will continue to emit incorrect code for case 3. above by default, but that is also not a problem as the documentation can suggest using the option whenever some objects are produced by older versions of the XL compiler.

Looks like the proposed option needs to be tri-state? (a) no diagnostic (the default behavior, this is needed to link glibc/musl/FreeBSD ld.so) (b) call handleTlsRelocation (this looks out-of-place to me and I would really hope we don't have this) (c) error. It looks like (b) and (c) can have false positives as well.

If GNU ld is going to add a similar option, I hope we can be consistent. So can someone also make a request on the mailing list [e-mail] ?
I would hope ppc64 is part of the option name to make it clear this is a ppc64 specific workaround.

[nemanjai]

I don't think adding an option to GNU ld is appropriate because it does the slow thing by default. Stefan is working on some test cases to illustrate GNU ld's behaviour so I think we should start with what it does and see whether we want to match behaviour and whether we want to do so under option control.

[MaskRay]

In D91611#2414869, @nemanjai wrote:

I don't think adding an option to GNU ld is appropriate because it does the slow thing by default. Stefan is working on some test cases to illustrate GNU ld's behaviour so I think we should start with what it does and see whether we want to match behaviour and whether we want to do so under option control.

My request about raising this on binutils is less about implementing the functionality for GNU ld, but more for reaching a consensus on (a) the value of this option (as you can see, I am still doubtful; but acceptance on their side can make me feel more happy to accept it) (b) the naming of the option. If they decide to add an option, that is fine; it don't, hope they will not use a different option name for the same purpose (c) getting opinions on what may be missed from the current design.

[stefanp]

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@ha
  addi 2,2,.TOC.-.LCF0@l
  .localentry     Test1,.-Test1
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@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@ha
  addi 2,2,.TOC.-.LCF0@l
  .localentry     Test1,.-Test1
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@l
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@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 they 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.

[MaskRay]

In D91611#2417611, @stefanp wrote:

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@ha
  addi 2,2,.TOC.-.LCF0@l
  .localentry     Test1,.-Test1
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@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@ha
  addi 2,2,.TOC.-.LCF0@l
  .localentry     Test1,.-Test1
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@l
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@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.

Thanks. I did not know that GNU ld already has the functionality. That does justify the motivation behind the patch.

Here is my new proposal:
We can add an option to lld. Unfortunately this option will not exist in ld.bfd because they wouldn't need it.

Thanks. This makes sense.

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.

Sounds good, if we don't implement a fullblown heuristic (which can be overkill and become maintenance burden) to reduce false positives.

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.

Oh, I missed the case that the current patch implemented TLS relaxation. This is indeed unnecessary if the idea is just to make legacy code work, not to make them also efficient.
If not implementing TLS relaxation can make the code simpler, then go for it!

Another thing: R_PPC64_REL24_NOTOC is new. Does it really need the workaround?

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.

[MaskRay]

In D91611#2417611, @stefanp wrote:

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@ha
  addi 2,2,.TOC.-.LCF0@l
  .localentry     Test1,.-Test1
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@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@ha
  addi 2,2,.TOC.-.LCF0@l
  .localentry     Test1,.-Test1
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@l
  addis 3,2,x@got@tlsgd@ha
  addi 3,3,x@got@tlsgd@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 they 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.

Instead of naming the option --add-*, we could also consider --fix-* (some arm/aarch64 specific options start with --fix-*, as you can see from Options.td)

[stefanp]

Completely changed the way that the patch works.

Added a new option to the patch --fix-ppc64-tls-reloc
The option is off by default.
If the option is off then LLD behaves the way it has in the past.
If the option is on then LLD on PPC64 will check all calls to __tls_get_addr. If
any of those calls are missing a relocation then TLS relaxation is turned off.

[sfertile]

lld/ELF/Relocations.cpp

1335

We still need to issue a diagnostic. While disabling tls relaxations allows us to produce a well formed binary, ideally the users would recompile the objects with invalid tls sequences. Without a warning they won't know some of the input files have problems.

lld/test/ELF/ppc64-call-tls-get-addr-missing.s

7

I'm trying to figure out the utility of this test. Referencing an undefined symbol should obviously be an error. Is the test meant to show we don't perform tls relaxations? If so I think the other tests cover that.

lld/test/ELF/ppc64-tls-add-missing-gdld.s

1

The name of this file should be changed to match the direction change, or if it doesn't add anything that isn't already covered by ppc64-tls-missing-gdld.s it should be removed.

[stefanp]

Since the scope of the patch has changed two of the tests were no longer
required so I have removed them.
Added a warning for the case where TLS relaxation is disabled due to a missing
relocation.

lld/test/ELF/ppc64-call-tls-get-addr-missing.s

7

Yes this test was meant to show that the relaxations were not done. However, I agree with you this is probably already covered by the other tests. I'll remove this test.

lld/test/ELF/ppc64-tls-add-missing-gdld.s

1

Yes, This test pretty much covers the same thing as ppc64-tls-missing-gdld.s.
I'm going to remove it as well.

[stefanp]

Gentle Ping

[MaskRay]

lld/ELF/Relocations.cpp

1324

The two if conditions can be combined.

1328

The parens are unneeded.

1331

config->isMips64EL -> false

lld/test/ELF/ppc64-call-tls-get-addr-exec.s

11

Add .globl __tls_get_addr to make it match the real case.

lld/test/ELF/ppc64-call-tls-get-addr.s

2

Consider combining the two tests with split-file

[stefanp]

Merged if statement and did a bit of cleanup according to comments.
Merged two test cases.

[MaskRay]

lld/ELF/InputSection.cpp

1055–1056

This is not correct. The relocation will not be handled at all. ppc64-tls-missing-gdld.s does not test -no-pie or -pie so you did not notice the issue.

Consider dropping the TLS part and I'll try taking a stab at it.

lld/test/ELF/ppc64-call-tls-get-addr-exec.s

2

Since the test now has -shared RUN lines. -exec is no longer a suitable suffix.

lld/test/ELF/ppc64-tls-missing-gdld.s

4–11

GD to IE/LE relaxation only happens for -no-pie or -pie.

7

Can some portion of the comment be retained?

[MaskRay]

I think the code should be extracted to scanRelocs: check whether there are R_TLSGD_GOT relocations without R_PPC64_TLSGD/R_PPC64_TLSLD. This may be an entire rewrite. I'm working on it.

[stefanp]

In D91611#2443178, @MaskRay wrote:

I think the code should be extracted to scanRelocs: check whether there are R_TLSGD_GOT relocations without R_PPC64_TLSGD/R_PPC64_TLSLD. This may be an entire rewrite. I'm working on it.

I think I understand what you are looking for. It's my patch so I should probably be doing the rewrite myself.
I don't want you to spend more time than you have to on this...

[MaskRay]

In D91611#2446331, @stefanp wrote:

In D91611#2443178, @MaskRay wrote:

I think the code should be extracted to scanRelocs: check whether there are R_TLSGD_GOT relocations without R_PPC64_TLSGD/R_PPC64_TLSLD. This may be an entire rewrite. I'm working on it.

I think I understand what you are looking for. It's my patch so I should probably be doing the rewrite myself.
I don't want you to spend more time than you have to on this...

... but do you think D92959 is complete now?

[stefanp]

In D91611#2446779, @MaskRay wrote:

In D91611#2446331, @stefanp wrote:

In D91611#2443178, @MaskRay wrote:

I think the code should be extracted to scanRelocs: check whether there are R_TLSGD_GOT relocations without R_PPC64_TLSGD/R_PPC64_TLSLD. This may be an entire rewrite. I'm working on it.

I think I understand what you are looking for. It's my patch so I should probably be doing the rewrite myself.
I don't want you to spend more time than you have to on this...

... but do you think D92959 is complete now?

Yes, I see you have already written it.
Thank you for your help.
In that case I'll move to your patch and add comments there.

[stefanp]

Since D92959 is now in I am going to abandon this patch.

lld/ELF/Config.h

struct Configuration {
   bool sysvHash = false;
   bool target1Rel;
   bool trace;
   bool thinLTOEmitImportsFiles;
   bool thinLTOIndexOnly;
   bool timeTraceEnabled;
   bool tocOptimize;
   bool pcRelOptimize;
+  bool fixPPC64TLSReloc;
   bool undefinedVersion;
   bool unique;
   bool useAndroidRelrTags = false;
   bool warnBackrefs;
   std::vector<llvm::GlobPattern> warnBackrefsExclude;
   bool warnCommon;
   bool warnIfuncTextrel;
   bool warnMissingEntry;

lld/ELF/Driver.cpp

if (config->fixCortexA8 && config->emachine != EM_ARM)
     error("--fix-cortex-a8 is only supported on ARM targets");
 
   if (config->tocOptimize && config->emachine != EM_PPC64)
     error("--toc-optimize is only supported on the PowerPC64 target");
 
   if (config->pcRelOptimize && config->emachine != EM_PPC64)
     error("--pcrel--optimize is only supported on the PowerPC64 target");
 
+  if (config->fixPPC64TLSReloc && config->emachine != EM_PPC64)
+    error("--fix-ppc64-tls-reloc is only supported on the PowerPC64 target");
+
   if (config->pie && config->shared)
     error("-shared and -pie may not be used together");
 
   if (!config->shared && !config->filterList.empty())
     error("-F may not be used without -shared");
 
   if (!config->shared && !config->auxiliaryList.empty())
     error("-f may not be used without -shared");
static void setConfigs(opt::InputArgList &args) {
   config->writeAddends = args.hasFlag(OPT_apply_dynamic_relocs,
                                       OPT_no_apply_dynamic_relocs, false) ||
                          !config->isRela;
 
   config->tocOptimize =
       args.hasFlag(OPT_toc_optimize, OPT_no_toc_optimize, m == EM_PPC64);
   config->pcRelOptimize =
       args.hasFlag(OPT_pcrel_optimize, OPT_no_pcrel_optimize, m == EM_PPC64);
+  config->fixPPC64TLSReloc =
+      args.hasFlag(OPT_fix_ppc64_tls_reloc, OPT_no_fix_ppc64_tls_reloc, false);
 }
 
 // Returns a value of "-format" option.
 static bool isFormatBinary(StringRef s) {
   if (s == "binary")
     return true;
   if (s == "elf" || s == "default")
     return false;

lld/ELF/InputFiles.h

public:
   // their .toc sections sorted closer to the .got section than files that do
   // not contain any small code model relocations. Thats because the toc-pointer
   // is defined to point at .got + 0x8000 and the instructions used with small
   // code model relocations support immediates in the range [-0x8000, 0x7FFC],
   // making the addressable range relative to the toc pointer
   // [.got, .got + 0xFFFC].
   bool ppc64SmallCodeModelTocRelocs = false;
 
+  // On PPC64 we may need to turn off TLS relaxation to avoid bad code
+  // generation when calls to __tls_get_adds are missing a relocation.
+  bool ppc64DisableTLSRelax = false;
+
   // groupId is used for --warn-backrefs which is an optional error
   // checking feature. All files within the same --{start,end}-group or
   // --{start,end}-lib get the same group ID. Otherwise, each file gets a new
   // group ID. For more info, see checkDependency() in SymbolTable.cpp.
   uint32_t groupId;
   static bool isInGroup;
   static uint32_t nextGroupId;
 

lld/ELF/InputSection.cpp

case R_PPC64_RELAX_TOC:
       // entry, there may be R_PPC64_TOC16_HA not paired with
       // R_PPC64_TOC16_LO_DS. Don't relax. This loses some relaxation
       // opportunities but is safe.
       if (ppc64noTocRelax.count({rel.sym, rel.addend}) ||
           !tryRelaxPPC64TocIndirection(rel, bufLoc))
         target->relocate(bufLoc, rel, targetVA);
       break;
     case R_RELAX_TLS_IE_TO_LE:
+      if (!file->ppc64DisableTLSRelax)
         target->relaxTlsIeToLe(bufLoc, rel, targetVA);
       break;
     case R_RELAX_TLS_LD_TO_LE:
     case R_RELAX_TLS_LD_TO_LE_ABS:
+      if (!file->ppc64DisableTLSRelax)
         target->relaxTlsLdToLe(bufLoc, rel, targetVA);
       break;
     case R_RELAX_TLS_GD_TO_LE:
     case R_RELAX_TLS_GD_TO_LE_NEG:
+      if (!file->ppc64DisableTLSRelax)
         target->relaxTlsGdToLe(bufLoc, rel, targetVA);
       break;
     case R_AARCH64_RELAX_TLS_GD_TO_IE_PAGE_PC:
     case R_RELAX_TLS_GD_TO_IE:
     case R_RELAX_TLS_GD_TO_IE_ABS:
     case R_RELAX_TLS_GD_TO_IE_GOT_OFF:
     case R_RELAX_TLS_GD_TO_IE_GOTPLT:
+      if (!file->ppc64DisableTLSRelax)
         target->relaxTlsGdToIe(bufLoc, rel, targetVA);
       break;
     case R_PPC64_CALL:
       // If this is a call to __tls_get_addr, it may be part of a TLS
       // sequence that has been relaxed and turned into a nop. In this
       // case, we don't want to handle it as a call.
       if (read32(bufLoc) == 0x60000000) // nop
         break;
 

lld/ELF/Options.td

 defm toc_optimize : BB<"toc-optimize",
     "(PowerPC64) Enable TOC related optimizations (default)",
     "(PowerPC64) Disable TOC related optimizations">;
 
 defm pcrel_optimize : BB<"pcrel-optimize",
     "(PowerPC64) Enable PC-relative optimizations (default)",
     "(PowerPC64) Disable PC-relative optimizations">;
 
+defm fix_ppc64_tls_reloc : BB<"fix-ppc64-tls-reloc",
+    "(PowerPC64) Check for missing TLSGD / TLSLD relocation and disable TLS relaxation if missing",
+    "(PowerPC64) Do not check for missing TLSGD / TLSLD relocation (default)">;
+
 def trace: F<"trace">, HelpText<"Print the names of the input files">;
 
 defm trace_symbol: Eq<"trace-symbol", "Trace references to symbols">;
 
 defm undefined: Eq<"undefined", "Force undefined symbol during linking">,
   MetaVarName<"<symbol>">;
 
 defm undefined_glob: EEq<"undefined-glob", "Force undefined symbol during linking">,

lld/ELF/Relocations.cpp

static void scanReloc(InputSectionBase &sec, OffsetGetter &getOffset, RelTy *&i,
   if (sym.isGnuIFunc() && !config->zText && config->warnIfuncTextrel) {
     warn("using ifunc symbols when text relocations are allowed may produce "
          "a binary that will segfault, if the object file is linked with "
          "old version of glibc (glibc 2.28 and earlier). If this applies to "
          "you, consider recompiling the object files without -fPIC and "
          "without -Wl,-z,notext option. Use -no-warn-ifunc-textrel to "
          "turn off this warning." +
          getLocation(sec, sym, offset));
   }
 
   // Read an addend.
   int64_t addend = computeAddend<ELFT>(rel, end, sec, expr, sym.isLocal());
 
   if (config->emachine == EM_PPC64) {
+    // If the option is set we should check all calls to __tls_get_addr for
+    // missing relocations.
+    if (config->fixPPC64TLSReloc) {
+      if ((type == R_PPC64_REL24 || type == R_PPC64_REL24_NOTOC) &&
+          sym.getName() == "__tls_get_addr") {
+
+        bool disableTLSRelax = (i - start < 2);
+        if (!disableTLSRelax) {
+          const RelTy &prevRel = *(i - 2);
+          const RelType prevType = prevRel.getType(config->isMips64EL);
+          disableTLSRelax =
+              (prevType != R_PPC64_TLSGD && prevType != R_PPC64_TLSLD);
+        }
+        if (disableTLSRelax)
+          sec.file->ppc64DisableTLSRelax = true;
+      }
+    }
     // We can separate the small code model relocations into 2 categories:
     // 1) Those that access the compiler generated .toc sections.
     // 2) Those that access the linker allocated got entries.
     // lld allocates got entries to symbols on demand. Since we don't try to
     // sort the got entries in any way, we don't have to track which objects
     // have got-based small code model relocs. The .toc sections get placed
     // after the end of the linker allocated .got section and we do sort those
     // so sections addressed with small code model relocations come first.

lld/test/ELF/ppc64-call-tls-get-addr-exec.s

+# REQUIRES: ppc
+# RUN: llvm-mc --triple=powerpc64le %s --filetype=obj -o %t1.o
+# RUN: llvm-mc --triple=powerpc64 %s --filetype=obj -o %t2.o
+# RUN: ld.lld %t1.o -o %t1
+# RUN: ld.lld %t2.o -o %t2
+# RUN: llvm-objdump -dr --no-show-raw-insn %t1 | FileCheck %s
+# RUN: llvm-objdump -dr --no-show-raw-insn %t2 | FileCheck %s
+
+## Since this is not a shared object we need to include a definition.
+__tls_get_addr:
+  blr
+
+## Without all of the TLS relocations that come before the __tls_get_addr call
+## this call should be treated as we treat all other calls.
+# CHECK-LABEL: <CallOnly>:
+# CHECK:       bl
+# CHECK-NEXT:  nop
+# CHECK-NEXT:  blr
+CallOnly:
+  bl __tls_get_addr
+  nop
+  blr

lld/test/ELF/ppc64-call-tls-get-addr-missing.s

+# REQUIRES: ppc
+# RUN: llvm-mc --triple=powerpc64le %s --filetype=obj -o %t1.o
+# RUN: not ld.lld %t1.o -o %t1 2>&1 | FileCheck %s
+
+## Since this is being compiled without --shared we should see the linker
+## complain about the callee being missing.
+# CHECK: error: undefined symbol: __tls_get_addr
+CallOnly:
+  bl __tls_get_addr
+  nop
+  blr

lld/test/ELF/ppc64-call-tls-get-addr.s

+# REQUIRES: ppc
+# RUN: llvm-mc --triple=powerpc64le %s --filetype=obj -o %t1.o
+# RUN: llvm-mc --triple=powerpc64 %s --filetype=obj -o %t2.o
+# RUN: ld.lld --shared %t1.o -o %t1.so
+# RUN: ld.lld --shared %t2.o -o %t2.so
+# RUN: llvm-objdump -dr --no-show-raw-insn %t1.so | FileCheck %s
+# RUN: llvm-objdump -dr --no-show-raw-insn %t2.so | FileCheck %s
+
+## Without all of the TLS relocations that come before the __tls_get_addr call
+## this call should be treated as we treat all other calls.
+## Also note that the nop is replaced with a TOC restore here.
+# CHECK-LABEL: <CallOnly>:
+# CHECK:       bl
+# CHECK-NEXT:  ld 2, 24(1)
+# CHECK-NEXT:  blr
+CallOnly:
+  bl __tls_get_addr
+  nop
+  blr

lld/test/ELF/ppc64-tls-add-missing-gdld.s

+# REQUIRES: ppc
+# RUN: split-file %s %t
+
+# RUN: llvm-mc --triple=powerpc64le %t/data --filetype=obj -o %t_data.o
+# RUN: llvm-mc --triple=powerpc64le %t/text --filetype=obj -o %t_text.o
+# RUN: ld.lld --fix-ppc64-tls-reloc %t_text.o %t_data.o -o %t_exec
+# RUN: llvm-objdump -dr %t_exec --no-show-raw-insn --mcpu=pwr10 | FileCheck %s
+
+#--- data
+.section .tbss,"awT",@nobits
+.globl  x
+x:
+  .long 0
+.globl  y
+y:
+  .long 0
+
+## Add fake definition for __tls_get_addr
+.section .text, "ax", %progbits
+.globl __tls_get_addr
+__tls_get_addr:
+  blr
+
+#--- text
+
+.section .text, "ax", %progbits
+# CHECK-LABEL: <GeneralDynamic>:
+# CHECK:         addis 3, 2, 0
+# CHECK-NEXT:    addi 3, 3, 0
+# CHECK-NEXT:    bl
+# CHECK-NEXT:    nop
+# CHECK-NEXT:    addis 3, 2, 0
+# CHECK-NEXT:    addi 3, 3, 0
+# CHECK-NEXT:    bl
+# CHECK-NEXT:    nop
+# CHECK-NEXT:    blr
+GeneralDynamic:
+  addis 3, 2, x@got@tlsgd@ha
+  addi 3, 3, x@got@tlsgd@l
+  bl __tls_get_addr
+  nop
+  addis 3, 2, y@got@tlsgd@ha
+  addi 3, 3, y@got@tlsgd@l
+  bl __tls_get_addr
+  nop
+  blr
+
+# CHECK-LABEL: <LocalDynamic>:
+# CHECK:         addis 3, 2, 0
+# CHECK-NEXT:    addi 3, 3, 0
+# CHECK-NEXT:    bl
+# CHECK-NEXT:    nop
+# CHECK-NEXT:    addis 3, 2, 0
+# CHECK-NEXT:    addi 3, 3, 0
+# CHECK-NEXT:    bl
+# CHECK-NEXT:    nop
+# CHECK-NEXT:    blr
+LocalDynamic:
+  addis 3, 2, x@got@tlsld@ha
+  addi 3, 3, x@got@tlsld@l
+  bl __tls_get_addr
+  nop
+  addis 3, 2, y@got@tlsld@ha
+  addi 3, 3, y@got@tlsld@l
+  bl __tls_get_addr
+  nop
+  blr

lld/test/ELF/ppc64-tls-missing-gdld.s

 # REQUIRES: ppc
 # RUN: llvm-mc --triple=powerpc64le %s --filetype=obj -o %t1.o
 # RUN: llvm-mc --triple=powerpc64 %s --filetype=obj -o %t2.o
-# RUN: ld.lld --shared --fatal-warnings %t1.o -o /dev/null
-# RUN: ld.lld --shared --fatal-warnings %t2.o -o /dev/null
-
-## User code can call __tls_get_addr by specifying the tls_index parameter.
-## We need to allow R_PPC64_REL24/R_PPC64_REL24_NOTOC referencing __tls_get_addr
-## without a pairing R_PPC64_TLSGD/R_PPC64_TLSLD.
-
+# RUN: ld.lld --shared %t1.o --soname=t1 -o %t1.so
+# RUN: ld.lld --shared %t2.o --soname=t2 -o %t2.so
+# RUN: ld.lld --shared %t1.o --fix-ppc64-tls-reloc --soname=t1-fix -o %t1-fix.so
+# RUN: ld.lld --shared %t2.o --fix-ppc64-tls-reloc --soname=t2-fix -o %t2-fix.so
+# RUN: llvm-objdump --no-show-raw-insn --mcpu=pwr10 -dr %t1.so | FileCheck %s
+# RUN: llvm-objdump --no-show-raw-insn --mcpu=pwr10 -dr %t2.so | FileCheck %s
+# RUN: llvm-objdump --no-show-raw-insn --mcpu=pwr10 -dr %t1-fix.so | FileCheck %s
+# RUN: llvm-objdump --no-show-raw-insn --mcpu=pwr10 -dr %t2-fix.so | FileCheck %s
+
+#CHECK-LABEL: <GeneralDynamic>:
+#CHECK:         addis 3, 2, 0
+#CHECK-NEXT:    addi 3, 3, -32768
+#CHECK-NEXT:    bl
+#CHECK-NEXT:    ld 2, 24(1)
+#CHECK-NEXT:    blr
 GeneralDynamic:
   addis 3, 2, x@got@tlsgd@ha
   addi 3, 3, x@got@tlsgd@l
   bl __tls_get_addr
+  nop
   blr
 
+#CHECK-LABEL: <GeneralDynamic_NOTOC>:
+#CHECK:         addis 3, 2, 0
+#CHECK-NEXT:    addi 3, 3, -32768
+#CHECK-NEXT:    bl
+#CHECK-NEXT:    blr
 GeneralDynamic_NOTOC:
   addis 3, 2, x@got@tlsgd@ha
   addi 3, 3, x@got@tlsgd@l
   bl __tls_get_addr@notoc
   blr
 
+#CHECK-LABEL: <LocalDynamic>:
+#CHECK:         addis 3, 2, 0
+#CHECK-NEXT:    addi 3, 3, -32752
+#CHECK-NEXT:    bl
+#CHECK-NEXT:    ld 2, 24(1)
+#CHECK-NEXT:    blr
 LocalDynamic:
   addis 3, 2, x@got@tlsld@ha
   addi 3, 3, x@got@tlsld@l
   bl __tls_get_addr
+  nop
   blr
 
+#CHECK-LABEL: <LocalDynamic_NOTOC>:
+#CHECK:         addis 3, 2, 0
+#CHECK-NEXT:    addi 3, 3, -32752
+#CHECK-NEXT:    bl
+#CHECK-NEXT:    blr
 LocalDynamic_NOTOC:
   addis 3, 2, x@got@tlsld@ha
   addi 3, 3, x@got@tlsld@l
   bl __tls_get_addr@notoc
   blr
 
+#CHECK-LABEL: <CallOnly>:
+#CHECK:         bl
+#CHECK-NEXT:    ld 2, 24(1)
+#CHECK-NEXT:    blr
 CallOnly:
   bl __tls_get_addr
+  nop
   blr