[lld] r230191 - [ELF] Add symbol to ELFReference.

Rui Ueyama ruiu at google.com
Mon Feb 23 10:31:42 PST 2015


My point is that usually when you want to know the name of a symbol to
which a reference is pointing, you'd do ref->getTarget()->name() instead of
adding a name to a reference. In LLD's model, symbol name is a property of
the node (atom) and not a property of edge (reference).

My question is that why you had to add a name to the reference, instead of
using the usual way? What's special about the feature that you were trying
to implement?

On Mon, Feb 23, 2015 at 4:59 AM, Shankar Easwaran <shankare at codeaurora.org>
wrote:

> Hi Rui,
>
> Thanks for the review.
>
> This is used to lookup the symbol/section, for a reference.
>
> This is used to check if the symbol/section that points to the reference
> is from outside the group (or) not in the same section group.
>
> More details are here FYI. https://mentorembedded.github.
> io/cxx-abi/abi/prop-72-comdat.html
>
> Shankar Easwaran
>
>
> On 2/22/2015 9:34 PM, Rui Ueyama wrote:
>
>> This patch looks really weird. Basically in the atom model a reference is
>> a
>> relocation, and an atom is a symbol + data. However, looks like this patch
>> made a reference to have an item that it is pointing to. If so, that
>> doesn't sound correct.
>>
>> Send this kind of nontrivial changes to pre-commit review first, please.
>>
>> On Sun, Feb 22, 2015 at 3:46 PM, Shankar Easwaran <
>> shankare at codeaurora.org>
>> wrote:
>>
>>  Author: shankare
>>> Date: Sun Feb 22 17:46:21 2015
>>> New Revision: 230191
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=230191&view=rev
>>> Log:
>>> [ELF] Add symbol to ELFReference.
>>>
>>> Relocation handling need more information about the Symbol that we are
>>> creating
>>> references for.
>>>
>>> No change in functionality.
>>>
>>> Modified:
>>>      lld/trunk/lib/ReaderWriter/ELF/Atoms.h
>>>      lld/trunk/lib/ReaderWriter/ELF/ELFFile.h
>>>      lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h
>>>
>>> Modified: lld/trunk/lib/ReaderWriter/ELF/Atoms.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/
>>> ReaderWriter/ELF/Atoms.h?rev=230191&r1=230190&r2=230191&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/lib/ReaderWriter/ELF/Atoms.h (original)
>>> +++ lld/trunk/lib/ReaderWriter/ELF/Atoms.h Sun Feb 22 17:46:21 2015
>>> @@ -35,23 +35,27 @@ template <typename ELFT> class ELFFile;
>>>   template <class ELFT> class ELFReference : public Reference {
>>>     typedef llvm::object::Elf_Rel_Impl<ELFT, false> Elf_Rel;
>>>     typedef llvm::object::Elf_Rel_Impl<ELFT, true> Elf_Rela;
>>> +  typedef llvm::object::Elf_Sym_Impl<ELFT> Elf_Sym;
>>> +
>>>   public:
>>> -  ELFReference(const Elf_Rela *rela, uint64_t off, Reference::KindArch
>>> arch,
>>> -               Reference::KindValue relocType, uint32_t idx)
>>> -      : Reference(Reference::KindNamespace::ELF, arch, relocType),
>>> +  ELFReference(const Elf_Sym *sym, const Elf_Rela *rela, uint64_t off,
>>> +               Reference::KindArch arch, Reference::KindValue relocType,
>>> +               uint32_t idx)
>>> +      : Reference(Reference::KindNamespace::ELF, arch, relocType),
>>> _sym(sym),
>>>           _target(nullptr), _targetSymbolIndex(idx), _offsetInAtom(off),
>>>           _addend(rela->r_addend) {}
>>>
>>> -  ELFReference(uint64_t off, Reference::KindArch arch,
>>> +  ELFReference(const Elf_Sym *sym, uint64_t off, Reference::KindArch
>>> arch,
>>>                  Reference::KindValue relocType, uint32_t idx)
>>> -      : Reference(Reference::KindNamespace::ELF, arch, relocType),
>>> +      : Reference(Reference::KindNamespace::ELF, arch, relocType),
>>> _sym(sym),
>>>           _target(nullptr), _targetSymbolIndex(idx), _offsetInAtom(off),
>>>           _addend(0) {}
>>>
>>>     ELFReference(uint32_t edgeKind)
>>>         : Reference(Reference::KindNamespace::all,
>>> Reference::KindArch::all,
>>>                     edgeKind),
>>> -        _target(nullptr), _targetSymbolIndex(0), _offsetInAtom(0),
>>> _addend(0) {}
>>> +        _sym(nullptr), _target(nullptr), _targetSymbolIndex(0),
>>> +        _offsetInAtom(0), _addend(0) {}
>>>
>>>     uint64_t offsetInAtom() const override { return _offsetInAtom; }
>>>
>>> @@ -70,7 +74,10 @@ public:
>>>
>>>     void setTarget(const Atom *newAtom) override { _target = newAtom; }
>>>
>>> +  const Elf_Sym *symbol() const { return _sym; }
>>> +
>>>   private:
>>> +  const Elf_Sym *_sym;
>>>     const Atom *_target;
>>>     uint64_t _targetSymbolIndex;
>>>     uint64_t _offsetInAtom;
>>>
>>> Modified: lld/trunk/lib/ReaderWriter/ELF/ELFFile.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/
>>> ReaderWriter/ELF/ELFFile.h?rev=230191&r1=230190&r2=230191&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/lib/ReaderWriter/ELF/ELFFile.h (original)
>>> +++ lld/trunk/lib/ReaderWriter/ELF/ELFFile.h Sun Feb 22 17:46:21 2015
>>> @@ -181,12 +181,12 @@ protected:
>>>     std::error_code doParse() override;
>>>
>>>     /// \brief Iterate over Elf_Rela relocations list and create
>>> references.
>>> -  virtual void createRelocationReferences(const Elf_Sym &symbol,
>>> +  virtual void createRelocationReferences(const Elf_Sym *symbol,
>>>                                             ArrayRef<uint8_t> content,
>>>                                             range<Elf_Rela_Iter> rels);
>>>
>>>     /// \brief Iterate over Elf_Rel relocations list and create
>>> references.
>>> -  virtual void createRelocationReferences(const Elf_Sym &symbol,
>>> +  virtual void createRelocationReferences(const Elf_Sym *symbol,
>>>                                             ArrayRef<uint8_t> symContent,
>>>                                             ArrayRef<uint8_t> secContent,
>>>                                             range<Elf_Rel_Iter> rels);
>>> @@ -796,12 +796,12 @@ ELFDefinedAtom<ELFT> *ELFFile<ELFT>::cre
>>>     // Add Rela (those with r_addend) references:
>>>     auto rari = _relocationAddendReferences.find(sectionName);
>>>     if (rari != _relocationAddendReferences.end())
>>> -    createRelocationReferences(*symbol, symContent, rari->second);
>>> +    createRelocationReferences(symbol, symContent, rari->second);
>>>
>>>     // Add Rel references.
>>>     auto rri = _relocationReferences.find(sectionName);
>>>     if (rri != _relocationReferences.end())
>>> -    createRelocationReferences(*symbol, symContent, secContent,
>>> rri->second);
>>> +    createRelocationReferences(symbol, symContent, secContent,
>>> rri->second);
>>>
>>>     // Create the DefinedAtom and add it to the list of DefinedAtoms.
>>>     return *handleDefinedSymbol(symbolName, sectionName, symbol,
>>> section,
>>> @@ -810,35 +810,35 @@ ELFDefinedAtom<ELFT> *ELFFile<ELFT>::cre
>>>   }
>>>
>>>   template <class ELFT>
>>> -void ELFFile<ELFT>::createRelocationReferences(const Elf_Sym &symbol,
>>> +void ELFFile<ELFT>::createRelocationReferences(const Elf_Sym *symbol,
>>>                                                  ArrayRef<uint8_t>
>>> content,
>>>                                                  range<Elf_Rela_Iter>
>>> rels)
>>> {
>>>     bool isMips64EL = _objFile->isMips64EL();
>>> -  const auto symValue = getSymbolValue(&symbol);
>>> +  const auto symValue = getSymbolValue(symbol);
>>>     for (const auto &rel : rels) {
>>>       if (rel.r_offset < symValue ||
>>>           symValue + content.size() <= rel.r_offset)
>>>         continue;
>>>       _references.push_back(new (_readerStorage) ELFReference<ELFT>(
>>> -        &rel, rel.r_offset - symValue, kindArch(),
>>> +        symbol, &rel, rel.r_offset - symValue, kindArch(),
>>>           rel.getType(isMips64EL), rel.getSymbol(isMips64EL)));
>>>     }
>>>   }
>>>
>>>   template <class ELFT>
>>> -void ELFFile<ELFT>::createRelocationReferences(const Elf_Sym &symbol,
>>> +void ELFFile<ELFT>::createRelocationReferences(const Elf_Sym *symbol,
>>>                                                  ArrayRef<uint8_t>
>>> symContent,
>>>                                                  ArrayRef<uint8_t>
>>> secContent,
>>>                                                  range<Elf_Rel_Iter>
>>> rels) {
>>>     bool isMips64EL = _objFile->isMips64EL();
>>> -  const auto symValue = getSymbolValue(&symbol);
>>> +  const auto symValue = getSymbolValue(symbol);
>>>     for (const auto &rel : rels) {
>>>       if (rel.r_offset < symValue ||
>>>           symValue + symContent.size() <= rel.r_offset)
>>>         continue;
>>>       _references.push_back(new (_readerStorage) ELFReference<ELFT>(
>>> -        rel.r_offset - symValue, kindArch(),
>>> -        rel.getType(isMips64EL), rel.getSymbol(isMips64EL)));
>>> +        symbol, rel.r_offset - symValue, kindArch(),
>>> rel.getType(isMips64EL),
>>> +        rel.getSymbol(isMips64EL)));
>>>       int32_t addend = *(symContent.data() + rel.r_offset - symValue);
>>>       _references.back()->setAddend(addend);
>>>     }
>>>
>>> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/
>>> MipsELFFile.h?rev=230191&r1=230190&r2=230191&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h (original)
>>> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h Sun Feb 22
>>> 17:46:21
>>> 2015
>>> @@ -199,17 +199,17 @@ private:
>>>       return std::error_code();
>>>     }
>>>
>>> -  void createRelocationReferences(const Elf_Sym &symbol,
>>> +  void createRelocationReferences(const Elf_Sym *symbol,
>>>                                     ArrayRef<uint8_t> symContent,
>>>                                     ArrayRef<uint8_t> secContent,
>>>                                     range<Elf_Rel_Iter> rels) override {
>>>       for (Elf_Rel_Iter rit = rels.begin(), eit = rels.end(); rit != eit;
>>> ++rit) {
>>> -      if (rit->r_offset < symbol.st_value ||
>>> -          symbol.st_value + symContent.size() <= rit->r_offset)
>>> +      if (rit->r_offset < symbol->st_value ||
>>> +          symbol->st_value + symContent.size() <= rit->r_offset)
>>>           continue;
>>>
>>>         this->_references.push_back(new (this->_readerStorage)
>>> ELFReference<ELFT>(
>>> -          rit->r_offset - symbol.st_value, this->kindArch(),
>>> +          symbol, rit->r_offset - symbol->st_value, this->kindArch(),
>>>             rit->getType(isMips64EL()), rit->getSymbol(isMips64EL())));
>>>
>>>         auto addend = getAddend(*rit, secContent);
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150223/269fc2f4/attachment.html>


More information about the llvm-commits mailing list