[PATCH] D85994: [LLD][PowerPC] Add check in LLD to produce an error for missing TLSGD/TLSLD

Mon Aug 17 13:03:47 PDT 2020

MaskRay added a comment.

In D85994#2222052 <https://reviews.llvm.org/D85994#2222052>, @nemanjai wrote:

> In D85994#2219062 <https://reviews.llvm.org/D85994#2219062>, @MaskRay wrote:
>
>> First, if this issue is not common enough, I'd rather we don't have a diagnostic at all.
>>
>> If we do need it, note that `*(i - 2);` may be out of bounds.

This needs to be addressed.

>> Last, we should avoid pre-built object files. You can synthesize them with yaml2obj. The preferred style is `llvm/test/tools/llvm-readobj/ELF/*.test`. lld/test/ELF has some yaml2obj tests as well.
>
> In the early days of the transition from the ELFv1 ABI that is used for big endian PowerPC Linux distributions to the ELFv2 ABI that is used for little endian PowerPC Linux distributions, there was some ambiguity in the specification of the relocations for TLS. The GNU linker has implemented support for correct handling of calls to __tls_get_addr with a missing relocation. Unfortunately, we didn't notice that the IBM XL compiler did not handle TLS according to the updated ABI until we tried linking XL compiled libraries with LLD. As a result, there is a lot of code out there in various libraries compiled with XL that have this problem.

OK. I think the justification needs to be moved to the description. Such diagnostics (for wrong relocation type usage) are not common.

The test can be converted to assembly by dropping a at tlsgd in `bl __tls_get_addr(a at tlsgd)`

> The plan is to initially add a diagnostic message regarding the missing relocation and fail to link. This will prevent run-time errors. The following step will be to implement support for correctly linking such code along with a warning. While it would be great to recompile any code that has this missing relocation, we understand that our clients have many reasons they can't easily do so.

This will add a string comparison in handling of every R_PPC64_REL24 (and make the code look ugly). I have read nearly every commit in the directory in the past 2 years. This kind of workaround is very unusual. I'd still suggest you consider whether such compatibility is really needed. Have other approaches (e.g. patching object files with a separate binary utility) been considered?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85994