[PATCH] D56089: [ELF] A shared object is needed if any of its occurrences is needed

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 26 14:19:40 PST 2018


MaskRay marked an inline comment as done.
MaskRay added a comment.

The test case suggests its usage: `ld.lld %t.o %tb.so --as-needed %ta.so --no-as-needed %ta.so -o %t`

The behavior I observed: both ld.bfd and gold consider a DSO needed if any occurrence of it is not wrapped by `--as-needed`. However, lld doesn't do so: we uniquify DSOs by their SONAME, and may drop a `DT_NEEDED` entry if the first occurrence is wrapped by `--as-needed`.

This patch makes it consistent with ld.bfd/gold in this aspect and moves a bit away from the not-so-good position-dependent behavior.
(An as-needed DSO resembles an archive file to some extent. I understand it is difficult to make it consistent with ld.bfd/gold in every case (gold diverges from ld.bfd in some aspects). But this behavior is what I think reasonable)



================
Comment at: ELF/SymbolTable.cpp:97
     F->parseSoName();
-    if (errorCount() || !SoNames.insert(F->SoName).second)
+    auto R = SoNames.try_emplace(F->SoName, F);
+    cast<SharedFile<ELFT>>(R.first->second)->IsNeeded |= F->IsNeeded;
----------------
ruiu wrote:
> Please replace `auto` with the actual type.
The actual type of `R` is very long `std::pair<llvm::DenseMapIterator<llvm::StringRef, lld::elf::InputFile *, llvm::DenseMapInfo<llvm::StringRef>, llvm::detail::DenseMapPair<llvm::StringRef, lld::elf::InputFile *>, false>, bool>`

It is a pair consisting of an iterator to the element, and a bool denoting whether the insertion took place (the same as https://en.cppreference.com/w/cpp/container/map/emplace).


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56089





More information about the llvm-commits mailing list