[lld] r219353 - [ELF] Only mark as DT_NEEDED libs that are strictly necessary
Rafael Auler
rafaelauler at gmail.com
Wed Oct 8 17:20:50 PDT 2014
Hi Rui,
No problem, thanks for reverting this. This memory bug didn't manifest when
compiling in optimized builds and that's why I missed it, but it does when
compiling in debug mode and I was able to reproduce it now. I'm currently
analyzing this and it looks like a memory bug due to an ELFUndefinedSymbol
being created with an uninitialized ELF_Sym.
On Wed, Oct 8, 2014 at 9:16 PM, Rui Ueyama <ruiu at google.com> wrote:
> 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/159672d1/attachment.html>
More information about the llvm-commits
mailing list