[PATCH] D73508: [LLD][COFF] Fix dll import for thread_local storage

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

First off, in general, this is lacking a test, and when uploading diffs, please include context (`git diff -U9999`) to make it more readable here within Phabricator.

However, I'm fairly sure this isn't the right thing to do and won't work as you hope it would. What the missing context would show is that this is from `getRuntimePseudoRelocSize`. The pseudo relocs are used for rewriting either absolute or relative addresses, to point to the actual location of the variable in a different DLL.

However, `IMAGE_REL_AMD64_SECREL` is used for getting the offset of a symbol within the enclosing section. For TLS variables, there's a single instance of the variable within the process, but every thread has a separate allocation of all TLS variables, and `IMAGE_REL_AMD64_SECREL` is used for getting the offset of the variable within the current thread's TLS allocation. Adding a relative offset to a different DLL's address space certainly won't work.

Also to respond to the summary:

> DLLs with thread local storage use the relocaction type IMAGE_REL_AMD64_SECREL

This is used for TLS in general (and debug info), whether it's used in DLLs or not.

> which have no bitsize specified leading to a linking failure with lld. This adds the bitsize from ld.bfd

The `IMAGE_REL_AMD64_SECREL` relocation is well supported in general, see https://github.com/llvm/llvm-project/blob/master/lld/COFF/Chunks.cpp#L113 and https://github.com/llvm/llvm-project/blob/master/lld/COFF/Chunks.cpp#L78-L88. So the fact that this relocation touches 32 bit in the image is well known by lld.

But it is not in the list of relocations in `getRuntimePseudoRelocSize`, as only the relocations that you sensibly can fix up that way is listed there.

In general, I would not expect raw access to TLS variables across DLL borders to work - at least not with the native TLS mechanism that clang uses by default (GCC uses emulated tls, I'm not familiar with how that works). But I would presume that use of TLS variables in such cases would produce a call to the TLS wrapper function instead, which should be able to successfully provide access to the variable from another DLL.

So the thing you're trying to fix needs a minimized reproducible testcase (for looking into the whole situation, not a LLD test just for testing that linking succeeds), to be able to reason about how you came into this situation, and whether the suggested fix really works on the whole.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D73508