[PATCH] D30441: Fix wrong TLS symbol values.

[George Rimar via Phabricator via llvm-commits]

grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lld/ELF/SyntheticSections.cpp:1399
-        if (auto *DS = dyn_cast<DefinedRegular<ELFT>>(Body))
-          ESym->st_value = OutSec->Addr + DS->Section->getOffset(*DS);
     } else if (isa<DefinedRegular<ELFT>>(Body)) {
----------------
At least for non-mips part (gc-debuginfo-tls.s), this code does not look correct.

ELF spec says:
"In dynamic executables and shared objects, the st_value field of a STT_TLS symbol contains the assigned TLS offset for defined symbols. "
That is not what we see in gc-debuginfo-tls.s before this patch. Mips part seems have similar issue.

getSymVA, called inside Body->getVA() handled TLS specially:
```
    if (D.isTls() && !Config->Relocatable) {
      if (!Out::TlsPhdr)
        fatal(toString(D.File) +
              " has a STT_TLS symbol but doesn't have a PT_TLS section");
      return VA - Out::TlsPhdr->p_vaddr;
    }
```

The rest of logic looks almost similar to what happens for DefinedRegular inside getVA() called at line 1388. So I also feel that code doesn't make sense.



https://reviews.llvm.org/D30441