[PATCH] D62055: [ARM][AArch64] Revert Android Bionic PT_TLS overaligning hack

Fri May 17 19:45:43 PDT 2019

MaskRay added a comment.

Now that (a) we get `.reloc` support (D61973 <https://reviews.llvm.org/D61973> D61992 <https://reviews.llvm.org/D61992>), (b) `R_*_NONE` on Elf*_Rel targets are compatible with `ld.lld -r` D62052 <https://reviews.llvm.org/D62052> (c) p_memsz of PT_TLS is no longer aligned (D62059 <https://reviews.llvm.org/D62059>; Ryan had noticed the difference in https://reviews.llvm.org/D53906#1326683 but it had not been implemented... D62059 <https://reviews.llvm.org/D62059> was tricky as it broke x86-{32,64} but I think they are good now), the workaround in `crtbegin_{dynamic_static}.o` should be feasible and it should have very little overhead.

Just waiting for @ruiu and/or @srhines to weigh in :) This revert should immediately unbreak glibc ARM/AArch64 users. I've describe the crash scenario in the description (PR41527 says it is for initial exec, but the bug affects the generic dynamic (most common; when you access an `extern __thread` in a DSO) model as well):

> For a thread local variable defined in the executable, lld computed TLS offset (local exec) is different from glibc computed TLS offset from another module (initial exec/generic dynamic).

Since Ryan's patch affects only executables (`if (!Config->Shared && (Config->EMachine == EM_ARM || Config->EMachine == EM_AARCH64)`), I assume there is some code in Bionic that fires if the executable doesn't have PT_TLS but one of the shared objects at startup (part of static TLS) has. I think that piece of code can be deleted now.

  # crtbegin_{dynamic,static}.o or some other object file that defines _start. I know little about Bionic
  .section .tdata,"awT", at progbits
  .p2align 6  # 5 for ARM
  .zero 1 # delete if you don't care about ld.bfd (ld.bfd strips empty PT_TLS)

  .text
  .globl _start
  _start:
    ...
    .reloc 0, R_AARCH64_NONE, .tdata  # prevent --gc-sections from discarding .tdata

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D62055