[PATCH] D45536: Do not keep shared symbols to garbage-collected eliminated DSOs.

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola accepted this revision.
espindola added a comment.

LGTM with a small change.



================
Comment at: lld/ELF/Driver.cpp:1069
+    if (auto *S = dyn_cast<SharedSymbol>(Sym))
+      if (S->isWeak() && !S->getFile<ELFT>().IsNeeded)
+        replaceSymbol<Undefined>(S, nullptr, S->getName(), STB_WEAK, S->StOther,
----------------
ruiu wrote:
> espindola wrote:
> > Is this doing the right thing if the symbol in the shared library is weak? Is it necessary? Doesn't IsNeeded already include all the required information?
> I believe this does the right thing for weak symbols in shared libraries, because if it is in use, IsNeeded will never be false. Otherwise, the symbol should be handled as if it weren't present from the beginning, so replacing it with an (unresolved) weak undefined seems like the right thing to do.
> 
> We could technically ignore shared symbols with a dead file everywhere, but I think purging such symbols all at once before processing further is less error-prone.
So the isWeak is really redundant. Please simplify this if to

if (!S->getFile<ELFT>().IsNeeded)



https://reviews.llvm.org/D45536