[PATCH] D85081: [lld-macho] Support dynamic linking of thread-locals

[Jez Ng via Phabricator via llvm-commits]

int3 marked 5 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/Arch/X86_64.cpp:228
+    if (sym.isTlv())
+      error("GOT relocations must not reference thread-local variables");
     break;
----------------
smeenai wrote:
> Would be nice to include information about the file, section and offset of the incorrect relocation, if possible.
offset is a bit tricky due to subsection splitting, but including the other two is straightforward enough


================
Comment at: lld/test/MachO/tlv-dylib.s:4
+
+# RUN: echo ".section	__DATA,__thread_vars,thread_local_variables; .globl _foo, _bar; _foo:; _bar:" |\
+# RUN:   llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/libtlv.o
----------------
smeenai wrote:
> The split-file utility was landed last week (rGbcea3a7a288). It might be nicer for some of these tests than the long echo lines.
ooh, that is a sweet tool


================
Comment at: lld/test/MachO/tlv-dylib.s:20
+# CHECK-LABEL: Bind table:
+# CHECK-DAG: __DATA __thread_ptrs  0x{{0*}}[[#%x, FOO]] pointer 0   libtlv   _foo
+# CHECK-DAG: __DATA __thread_ptrs  0x{{0*}}[[#%x, BAR]] pointer 0   libtlv   _bar
----------------
smeenai wrote:
> Is this DAG just because you don't care about the order of the bind table entries, or because there's a potential for the order to be different? (I would hope the latter isn't the case?) 
The former -- the ordering should be arbitrary (but deterministic)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85081