[PATCH] D62577: [ELF] Support Local Dynamic style TLSDESC for x86-64

Wed May 29 05:15:00 PDT 2019

grimar added inline comments.

================
Comment at: ELF/Writer.cpp:1609

+  if (Config->EMachine == EM_X86_64) {
+    // _TLS_MODULE_BASE_ is defined in such a way that _TLS_MODULE_BASE_ at tpoff =
----------------
MaskRay wrote:
> grimar wrote:
> > i.e. how much reasonable this check is?
> In GNU linkers, `_TLS_MODULE_BASE_` is defined on demand on i386/x86-64/arm/aarch64/s390. Many targets don't have an ABI for TLSDESC. If you feel we don't have to special case the TLSDESC targets that support TLSDESC, I can make this unconditional.
I am not sure. Let see what Rui think. Perhaps whitelisting was fine. Without any conditions, I think arm/aarch64 will be just broken with this change (i.e. LLD will produce broken output instead of link error it seems).

================
Comment at: ELF/Writer.cpp:1613
+  // _TLS_MODULE_BASE_ relative to the first TLS section.
+  Symbol *tlsBase = Symtab->find("_TLS_MODULE_BASE_");
+  if (tlsBase && !tlsBase->isDefined())
----------------
`tlsBase` -> `TlsBase`

================
Comment at: test/ELF/x86-64-tlsdesc-ld.s:4
+
+# RUN: ld.lld -shared %t.o -o %t.so
+# RUN: llvm-readobj -r %t.so | FileCheck --check-prefix=LD-REL %s
----------------
I would suggest adding a short description.

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D62577