[lld] r219353 - [ELF] Only mark as DT_NEEDED libs that are strictly necessary

Rui Ueyama ruiu at google.com
Wed Oct 8 17:16:32 PDT 2014


Rafael,

This seems to break buildbots:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-debian-fast/builds/18758.
These tests are failing on my machine too.

I rolled back this in r219369. Sorry for your inconvenience.

On Wed, Oct 8, 2014 at 3:53 PM, Rafael Auler <rafaelauler at gmail.com> wrote:

> Author: rafauler
> Date: Wed Oct  8 17:53:49 2014
> New Revision: 219353
>
> URL: http://llvm.org/viewvc/llvm-project?rev=219353&view=rev
> Log:
> [ELF] Only mark as DT_NEEDED libs that are strictly necessary
>
> Enhances the creation of an ELF dynamic executable by avoiding recording
> unnecessary shared libraries as NEEDED to load a program.
>
> To do this, we must keep track of not only symbols that were referenced but
> also of COPY relocations, which steal the symbol from a shared library but
> does
> not store from which lib this symbol came from. To fix this, this commit
> changes
> ObjectSymbol to store the original library from which this symbol came.
> With
> this information, we are able to build a list of the exact shared
> libraries that
> must be marked as DT_NEEDED, instead of blindly marking all shared
> libraries as
> needed.
>
> This logic originally came from the MIPS backend with some adaptation.
>
> Reviewers: atanasyan, shankar.easwaran
>
> http://reviews.llvm.org/D5574
>
> Modified:
>     lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h
>     lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64LinkingContext.h
>     lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64RelocationPass.cpp
>     lld/trunk/lib/ReaderWriter/ELF/Atoms.h
>     lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsDynamicLibraryWriter.h
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFWriters.h
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsExecutableWriter.h
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.cpp
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.h
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsRelocationPass.cpp
>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h
>     lld/trunk/lib/ReaderWriter/ELF/OutputELFWriter.h
>     lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64LinkingContext.h
>     lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64RelocationPass.cpp
>
> Modified: lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h (original)
> +++ lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h Wed Oct  8
> 17:53:49 2014
> @@ -90,6 +90,15 @@ public:
>                                     const Reference &) const {
>      return false;
>    }
> +
> +  /// \brief Is this a copy relocation?
> +  ///
> +  /// If this is a copy relocation, its target must be an ObjectAtom. We
> must
> +  /// include in DT_NEEDED the name of the library where this object came
> from.
> +  virtual bool isCopyRelocation(const Reference &) const {
> +    return false;
> +  }
> +
>    bool validateImpl(raw_ostream &diagnostics) override;
>
>    /// \brief Does the linker allow dynamic libraries to be linked with?
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64LinkingContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64LinkingContext.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64LinkingContext.h
> (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64LinkingContext.h Wed
> Oct  8 17:53:49 2014
> @@ -59,6 +59,15 @@ public:
>      }
>    }
>
> +  bool isCopyRelocation(const Reference &r) const override {
> +    if (r.kindNamespace() != Reference::KindNamespace::ELF)
> +      return false;
> +    assert(r.kindArch() == Reference::KindArch::AArch64);
> +    if (r.kindValue() == llvm::ELF::R_AARCH64_COPY)
> +      return true;
> +    return false;
> +  }
> +
>    bool isPLTRelocation(const DefinedAtom &,
>                                 const Reference &r) const override {
>      if (r.kindNamespace() != Reference::KindNamespace::ELF)
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64RelocationPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64RelocationPass.cpp?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64RelocationPass.cpp
> (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/AArch64/AArch64RelocationPass.cpp Wed
> Oct  8 17:53:49 2014
> @@ -452,7 +452,7 @@ public:
>      if (obj != _objectMap.end())
>        return obj->second;
>
> -    auto oa = new (_file._alloc) ObjectAtom(_file);
> +    auto oa = new (_file._alloc) ObjectAtom(_file, a);
>      // This needs to point to the atom that we just created.
>      oa->addReferenceELF_AArch64(R_AARCH64_COPY, 0, oa, 0);
>
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Atoms.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Atoms.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Atoms.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Atoms.h Wed Oct  8 17:53:49 2014
> @@ -689,8 +689,11 @@ public:
>  /// \brief Atom which represents an object for which a COPY relocation
> will be
>  ///   generated.
>  class ObjectAtom : public SimpleELFDefinedAtom {
> +  const SharedLibraryAtom *_sla;
> +
>  public:
> -  ObjectAtom(const File &f) : SimpleELFDefinedAtom(f) {}
> +  ObjectAtom(const File &f, const SharedLibraryAtom *sla)
> +      : SimpleELFDefinedAtom(f), _sla(sla) {}
>
>    Scope scope() const override { return scopeGlobal; }
>
> @@ -713,6 +716,8 @@ public:
>
>    StringRef name() const override { return _name; }
>
> +  const SharedLibraryAtom *getOriginalOwner() const { return _sla; }
> +
>    std::string _name;
>    uint64_t _size;
>  };
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h Wed Oct  8 17:53:49 2014
> @@ -10,6 +10,7 @@
>  #ifndef LLD_READER_WRITER_ELF_DEFAULT_LAYOUT_H
>  #define LLD_READER_WRITER_ELF_DEFAULT_LAYOUT_H
>
> +#include "Atoms.h"
>  #include "Chunk.h"
>  #include "HeaderChunks.h"
>  #include "Layout.h"
> @@ -171,6 +172,7 @@ public:
>    typedef typename std::vector<lld::AtomLayout *>::iterator
> AbsoluteAtomIterT;
>
>    typedef llvm::DenseSet<const Atom *> AtomSetT;
> +  typedef llvm::DenseSet<const SharedLibraryAtom *> SharedLibraryAtomSetT;
>
>    DefaultLayout(const ELFLinkingContext &context) : _context(context) {}
>
> @@ -303,6 +305,10 @@ public:
>      return _referencedDynAtoms.count(a);
>    }
>
> +  const SharedLibraryAtomSetT &getCopiedDynAtoms() const {
> +    return _copiedDynAtoms;
> +  }
> +
>  protected:
>    /// \brief Allocate a new section.
>    virtual AtomSection<ELFT> *createSection(
> @@ -325,6 +331,7 @@ protected:
>    LLD_UNIQUE_BUMP_PTR(RelocationTable<ELFT>) _pltRelocationTable;
>    std::vector<lld::AtomLayout *> _absoluteAtoms;
>    AtomSetT _referencedDynAtoms;
> +  SharedLibraryAtomSetT _copiedDynAtoms;
>    const ELFLinkingContext &_context;
>  };
>
> @@ -582,7 +589,14 @@ ErrorOr<const lld::AtomLayout &> Default
>        if (isa<UndefinedAtom>(reloc->target()) && isLocalReloc)
>          continue;
>
> -      _referencedDynAtoms.insert(reloc->target());
> +      if (!_context.isCopyRelocation(*reloc)) {
> +        _referencedDynAtoms.insert(reloc->target());
> +        continue;
> +      }
> +
> +      const ObjectAtom *oa = dyn_cast<ObjectAtom>(reloc->target());
> +      assert (oa != nullptr && "Targets of copy relocs must be
> ObjectAtoms");
> +      _copiedDynAtoms.insert(oa->getOriginalOwner());
>      }
>
>      return section->appendAtom(atom);
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsDynamicLibraryWriter.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsDynamicLibraryWriter.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsDynamicLibraryWriter.h
> (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsDynamicLibraryWriter.h Wed
> Oct  8 17:53:49 2014
> @@ -37,10 +37,6 @@ protected:
>      return std::error_code();
>    }
>
> -  bool isNeededTagRequired(const SharedLibraryAtom *sla) const override {
> -    return _writeHelper.isNeededTagRequired(sla);
> -  }
> -
>    LLD_UNIQUE_BUMP_PTR(DynamicTable<ELFT>) createDynamicTable();
>
>    LLD_UNIQUE_BUMP_PTR(DynamicSymbolTable<ELFT>)
> createDynamicSymbolTable();
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFWriters.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFWriters.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFWriters.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFWriters.h Wed Oct  8
> 17:53:49 2014
> @@ -69,11 +69,6 @@ public:
>      return file;
>    }
>
> -  bool isNeededTagRequired(const SharedLibraryAtom *sla) const {
> -    return _targetLayout.isReferencedByDefinedAtom(sla) ||
> -           _targetLayout.isCopied(sla);
> -  }
> -
>  private:
>    MipsLinkingContext &_ctx;
>    MipsTargetLayout<ELFT> &_targetLayout;
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsExecutableWriter.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsExecutableWriter.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsExecutableWriter.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsExecutableWriter.h Wed Oct  8
> 17:53:49 2014
> @@ -38,10 +38,6 @@ protected:
>      return std::error_code();
>    }
>
> -  bool isNeededTagRequired(const SharedLibraryAtom *sla) const override {
> -    return _writeHelper.isNeededTagRequired(sla);
> -  }
> -
>    LLD_UNIQUE_BUMP_PTR(DynamicTable<ELFT>) createDynamicTable();
>
>    LLD_UNIQUE_BUMP_PTR(DynamicSymbolTable<ELFT>)
> createDynamicSymbolTable();
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.cpp?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.cpp (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.cpp Wed Oct  8
> 17:53:49 2014
> @@ -65,6 +65,15 @@ bool MipsLinkingContext::isDynamicReloca
>    }
>  }
>
> +bool MipsLinkingContext::isCopyRelocation(const Reference &r) const {
> +  if (r.kindNamespace() != Reference::KindNamespace::ELF)
> +    return false;
> +  assert(r.kindArch() == Reference::KindArch::Mips);
> +  if (r.kindValue() == llvm::ELF::R_MIPS_COPY)
> +    return true;
> +  return false;
> +}
> +
>  bool MipsLinkingContext::isPLTRelocation(const DefinedAtom &,
>                                           const Reference &r) const {
>    if (r.kindNamespace() != Reference::KindNamespace::ELF)
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsLinkingContext.h Wed Oct  8
> 17:53:49 2014
> @@ -47,6 +47,7 @@ public:
>    bool isRelaOutputFormat() const override { return false; }
>    bool isDynamicRelocation(const DefinedAtom &,
>                             const Reference &r) const override;
> +  bool isCopyRelocation(const Reference &r) const override;
>    bool isPLTRelocation(const DefinedAtom &, const Reference &r) const
> override;
>  };
>
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsRelocationPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsRelocationPass.cpp?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsRelocationPass.cpp (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsRelocationPass.cpp Wed Oct  8
> 17:53:49 2014
> @@ -781,7 +781,7 @@ RelocationPass<ELFT>::getObjectEntry(con
>    if (obj != _objectMap.end())
>      return obj->second;
>
> -  auto oa = new (_file._alloc) ObjectAtom(_file);
> +  auto oa = new (_file._alloc) ObjectAtom(_file, a);
>    oa->addReferenceELF_Mips(R_MIPS_COPY, 0, oa, 0);
>    oa->_name = a->name();
>    oa->_size = a->size();
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsTargetHandler.h Wed Oct  8
> 17:53:49 2014
> @@ -62,26 +62,6 @@ public:
>      }
>    }
>
> -  ErrorOr<const lld::AtomLayout &> addAtom(const Atom *atom) override {
> -    // Maintain:
> -    // 1. Set of shared library atoms referenced by regular defined atoms.
> -    // 2. Set of shared library atoms have corresponding R_MIPS_COPY
> copies.
> -    if (const auto *da = dyn_cast<DefinedAtom>(atom))
> -      for (const Reference *ref : *da) {
> -        if (ref->kindNamespace() == lld::Reference::KindNamespace::ELF) {
> -          assert(ref->kindArch() == Reference::KindArch::Mips);
> -          if (ref->kindValue() == llvm::ELF::R_MIPS_COPY)
> -            _copiedDynSymNames.insert(atom->name());
> -        }
> -      }
> -
> -    return TargetLayout<ELFType>::addAtom(atom);
> -  }
> -
> -  bool isCopied(const SharedLibraryAtom *sla) const {
> -    return _copiedDynSymNames.count(sla->name());
> -  }
> -
>    /// \brief GP offset relative to .got section.
>    uint64_t getGPOffset() const { return 0x7FF0; }
>
> @@ -109,7 +89,6 @@ private:
>    MipsPLTSection<ELFType> *_pltSection;
>    llvm::Optional<AtomLayout *> _gpAtom;
>    llvm::Optional<AtomLayout *> _gpDispAtom;
> -  llvm::StringSet<> _copiedDynSymNames;
>  };
>
>  /// \brief Mips Runtime file.
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/OutputELFWriter.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/OutputELFWriter.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/OutputELFWriter.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/OutputELFWriter.h Wed Oct  8 17:53:49
> 2014
> @@ -180,11 +180,14 @@ template <class ELFT>
>  void OutputELFWriter<ELFT>::buildDynamicSymbolTable(const File &file) {
>    ScopedTask task(getDefaultDomain(), "buildDynamicSymbolTable");
>    for (const auto &sla : file.sharedLibrary()) {
> -    if (isDynSymEntryRequired(sla))
> -      _dynamicSymbolTable->addSymbol(sla, ELF::SHN_UNDEF);
> +    if (!isDynSymEntryRequired(sla))
> +      continue;
> +    _dynamicSymbolTable->addSymbol(sla, ELF::SHN_UNDEF);
>      if (isNeededTagRequired(sla))
>        _soNeeded.insert(sla->loadName());
>    }
> +  for (const auto &sla : _layout.getCopiedDynAtoms())
> +    _soNeeded.insert(sla->loadName());
>    // Never mark the dynamic linker as DT_NEEDED
>    _soNeeded.erase(sys::path::filename(_context.getInterpreter()));
>    for (const auto &loadName : _soNeeded) {
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64LinkingContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64LinkingContext.h?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64LinkingContext.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64LinkingContext.h Wed Oct
> 8 17:53:49 2014
> @@ -58,6 +58,15 @@ public:
>      }
>    }
>
> +  bool isCopyRelocation(const Reference &r) const override {
> +    if (r.kindNamespace() != Reference::KindNamespace::ELF)
> +      return false;
> +    assert(r.kindArch() == Reference::KindArch::x86_64);
> +    if (r.kindValue() == llvm::ELF::R_X86_64_COPY)
> +      return true;
> +    return false;
> +  }
> +
>    virtual bool isPLTRelocation(const DefinedAtom &,
>                                 const Reference &r) const override {
>      if (r.kindNamespace() != Reference::KindNamespace::ELF)
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64RelocationPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64RelocationPass.cpp?rev=219353&r1=219352&r2=219353&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64RelocationPass.cpp
> (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64RelocationPass.cpp Wed
> Oct  8 17:53:49 2014
> @@ -430,7 +430,7 @@ public:
>      if (obj != _objectMap.end())
>        return obj->second;
>
> -    auto oa = new (_file._alloc) ObjectAtom(_file);
> +    auto oa = new (_file._alloc) ObjectAtom(_file, a);
>      // This needs to point to the atom that we just created.
>      oa->addReferenceELF_x86_64(R_X86_64_COPY, 0, oa, 0);
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141008/edf89419/attachment.html>


More information about the llvm-commits mailing list