[PATCH] D31274: Remove an optimization to set the TLS module index to 1.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Hello, thanks for giving me the chance to review.

The reason I put the optimization in was for static linking, there are no relaxations for TLS for ARM, and we can't use a dynamic relocation to set the module index. So according to the specification we need to set the index to 1 when static linking. Implementing the optimization seemed like the cleanest way of doing it at the time, the alternative was to go through the writeTo() part of the GOT and hard code the slot to 1.

I thought I'd try and write a test case to show why it was required, only to find that when static linking with glibc, the __tls_get_addr that we pass the address of the module-index to, hard codes the module index to 1, so for glibc it doesn't matter what we put in the module index. However not all the world uses glibc so being a purist I'd prefer to stick to the specification just in case other C libraries rely on the static linker to set the module index. Pragmatically given the low user-base I think we could temporarily remove it if we were to put it back at least for static linking soon.

Sadly while writing the test case

  #include <stdio.h>
  
  extern __thread int val __attribute__((tls_model("global-dynamic")));
  extern __thread int val2 __attribute__((tls_model("global-dynamic")));
  extern __thread int val3 __attribute__((tls_model("global-dynamic")));
  
  int main(void)
  {
      printf("%d %d %d\n", val, val2, val3);
      return 0;
  }

file2.c

  __thread int val = 10;
  __thread int val2 = 20;
  __thread int val3;

lld is giving the wrong result for both static and dynamic linking, and printing out 10, 10, 10 rather than 10, 20, 0. What seems to be happening is that the offset from the TLS block is always being written as 0. I don't know whether this has been broken recently and the tests didn't pick it up or never worked in the first place. Back when I wrote the ARM TLS I seem to remember the getVA() when writing the GOT picked up that the symbol was TLS and put the correct offset in. I'll need to work out what happened and fix it. For what I can easily test (X86_64, ARM and AArch64) this problem looks ARM specific, although it may impact Mips as well as ARM is most similar to the Mips TLS model.

I'm very near to finishing a reviewable range extension Thunks implementation, I'm hoping to get this finished soon after Euro LLVM, when I've done that I'll look at the TLS offset problem.


https://reviews.llvm.org/D31274