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

Mon Nov 23 17:02:19 PST 2020

MaskRay added a comment.

> 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.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91611