This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Resolve weak undefined TLS symbols when no phdr is available
ClosedPublic

Authored by davide on Sep 22 2016, 9:44 AM.

Details

Summary

If we pass --gc-sections to lld and .tbss is not referenced, it gets reclaimed and we don't create a TLS program header.
R_TLS tries to access the program header -> lld crashes. Mimic what bfd/gold do in this case and resolve a weak undefined TLS symbol to the base of the TLS block, i.e. give it a value of zero.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 72185.Sep 22 2016, 9:44 AM
davide retitled this revision from to [ELF] Resolve weak undefined TLS symbols when no phdr is available.
davide updated this object.
davide added a reviewer: rafael.
davide added a subscriber: llvm-commits.
ruiu added a subscriber: ruiu.Sep 22 2016, 6:35 PM
ruiu added inline comments.
ELF/InputSection.cpp
249 ↗(On Diff #72185)

We should never call getSymVA() on a lazy symbol, so isLazy() is always false. But if so, your code cannot change the behavior of this function. So something's not correct.

I think we treat weak undefined specially. http://llvm.org/viewvc/llvm-project?view=revision&revision=261591

isLazy() returns true. At least it did when I stepped through inside a debugger.

I don't think the invariant you point out is respected (we should never call getSymVA() on a Lazy symbol).
I was lazy, and to convince myself I put the following assertion in the code:

diff --git a/ELF/InputSection.cpp b/ELF/InputSection.cpp
index 3cc6ce5..b44d2d9 100644
--- a/ELF/InputSection.cpp
+++ b/ELF/InputSection.cpp
@@ -185,6 +185,7 @@ template <class ELFT>
 static typename ELFT::uint getSymVA(uint32_t Type, typename ELFT::uint A,
                                     typename ELFT::uint P,
                                     const SymbolBody &Body, RelExpr Expr) {
+  assert (!Body.isLazy() && "patatino");
   switch (Expr) {
   case R_HINT:
     llvm_unreachable("cannot relocate hint relocs");

and saw three tests failing:

********************
Testing Time: 17.63s
********************
Failing Tests (3):
    lld :: ELF/archive.s
    lld :: ELF/lto/undef-weak.ll
    lld :: ELF/tls-in-archive.s

Still, maybe this is not the best place to check.

davide updated this revision to Diff 72315.Sep 23 2016, 11:47 AM

Update after Rafael's feedback.

rafael accepted this revision.Sep 23 2016, 11:53 AM
rafael edited edge metadata.

LGTM

This revision is now accepted and ready to land.Sep 23 2016, 11:53 AM
This revision was automatically updated to reflect the committed changes.