[lld] r286100 - [ELF] Make InputSection<ELFT>::writeTo virtual

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 8 02:44:46 PST 2016


Why virtual and not a switch on Kind?

It is odd to have two dispatch systems in the same type. This also
adds a vtable pointer to a pretty common class. Have you benchmarked
it?

Also, why the virtual destructor? What destroys this via the base class?

At the very least we should:

* Create a SyntheticInputClass that is the base of all Synthetic clases.
* Make SyntheticInputClass the only one that is virtual.
* Make sure the destructor is not virtual.

Cheers,
Rafael


On 7 November 2016 at 04:04, Eugene Leviant via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: evgeny777
> Date: Mon Nov  7 03:04:06 2016
> New Revision: 286100
>
> URL: http://llvm.org/viewvc/llvm-project?rev=286100&view=rev
> Log:
> [ELF] Make InputSection<ELFT>::writeTo virtual
>
> Differential revision: https://reviews.llvm.org/D26281
>
> Modified:
>     lld/trunk/ELF/InputSection.cpp
>     lld/trunk/ELF/InputSection.h
>     lld/trunk/ELF/SyntheticSections.cpp
>     lld/trunk/ELF/SyntheticSections.h
>
> Modified: lld/trunk/ELF/InputSection.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=286100&r1=286099&r2=286100&view=diff
> ==============================================================================
> --- lld/trunk/ELF/InputSection.cpp (original)
> +++ lld/trunk/ELF/InputSection.cpp Mon Nov  7 03:04:06 2016
> @@ -79,13 +79,6 @@ InputSectionBase<ELFT>::InputSectionBase
>    this->Offset = Hdr->sh_offset;
>  }
>
> -template <class ELFT> size_t InputSectionBase<ELFT>::getSize() const {
> -  if (auto *D = dyn_cast<InputSection<ELFT>>(this))
> -    if (D->getThunksSize() > 0)
> -      return D->getThunkOff() + D->getThunksSize();
> -  return Data.size();
> -}
> -
>  // Returns a string for an error message.
>  template <class SectionT> static std::string getName(SectionT *Sec) {
>    return (Sec->getFile()->getName() + "(" + Sec->Name + ")").str();
> @@ -207,6 +200,11 @@ bool InputSection<ELFT>::classof(const I
>    return S->kind() == Base::Regular;
>  }
>
> +template <class ELFT> size_t InputSection<ELFT>::getSize() const {
> +  return getThunksSize() > 0 ? getThunkOff() + getThunksSize()
> +                             : this->Data.size();
> +}
> +
>  template <class ELFT>
>  InputSectionBase<ELFT> *InputSection<ELFT>::getRelocatedSection() {
>    assert(this->Type == SHT_RELA || this->Type == SHT_REL);
>
> Modified: lld/trunk/ELF/InputSection.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.h?rev=286100&r1=286099&r2=286100&view=diff
> ==============================================================================
> --- lld/trunk/ELF/InputSection.h (original)
> +++ lld/trunk/ELF/InputSection.h Mon Nov  7 03:04:06 2016
> @@ -52,6 +52,8 @@ private:
>    unsigned SectionKind : 3;
>
>  public:
> +  virtual ~InputSectionData() = default;
> +  InputSectionData(InputSectionData &&) = default;
>    Kind kind() const { return (Kind)SectionKind; }
>
>    unsigned Live : 1; // for garbage collection
> @@ -66,6 +68,9 @@ public:
>      return llvm::makeArrayRef<T>((const T *)Data.data(), S / sizeof(T));
>    }
>
> +  virtual void writeTo(uint8_t *Buf) {}
> +  virtual size_t getSize() const { return Data.size(); }
> +
>    // If a section is compressed, this has the uncompressed section data.
>    std::unique_ptr<uint8_t[]> UncompressedData;
>
> @@ -113,9 +118,6 @@ public:
>    // this but instead this->Repl.
>    InputSectionBase<ELFT> *Repl;
>
> -  // Returns the size of this section (even if this is a common or BSS.)
> -  size_t getSize() const;
> -
>    static InputSectionBase<ELFT> Discarded;
>
>    ObjectFile<ELFT> *getFile() const { return File; }
> @@ -243,7 +245,7 @@ public:
>
>    // Write this section to a mmap'ed file, assuming Buf is pointing to
>    // beginning of the output section.
> -  void writeTo(uint8_t *Buf);
> +  void writeTo(uint8_t *Buf) override;
>
>    // Relocation sections that refer to this one.
>    llvm::TinyPtrVector<const Elf_Shdr *> RelocSections;
> @@ -270,6 +272,9 @@ public:
>    // Size of chunk with thunks code.
>    uint64_t getThunksSize() const;
>
> +  // Size of section in bytes.
> +  size_t getSize() const override;
> +
>    template <class RelTy>
>    void relocateNonAlloc(uint8_t *Buf, llvm::ArrayRef<RelTy> Rels);
>
>
> Modified: lld/trunk/ELF/SyntheticSections.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=286100&r1=286099&r2=286100&view=diff
> ==============================================================================
> --- lld/trunk/ELF/SyntheticSections.cpp (original)
> +++ lld/trunk/ELF/SyntheticSections.cpp Mon Nov  7 03:04:06 2016
> @@ -95,14 +95,14 @@ BuildIdSection<ELFT>::BuildIdSection(siz
>                           ".note.gnu.build-id"),
>        HashSize(HashSize) {
>    this->Live = true;
> +}
>
> -  Buf.resize(16 + HashSize);
> +template <class ELFT> void BuildIdSection<ELFT>::writeTo(uint8_t *Buf) {
>    const endianness E = ELFT::TargetEndianness;
> -  write32<E>(Buf.data(), 4);                   // Name size
> -  write32<E>(Buf.data() + 4, HashSize);        // Content size
> -  write32<E>(Buf.data() + 8, NT_GNU_BUILD_ID); // Type
> -  memcpy(Buf.data() + 12, "GNU", 4);           // Name string
> -  this->Data = ArrayRef<uint8_t>(Buf);
> +  write32<E>(Buf, 4);                   // Name size
> +  write32<E>(Buf + 4, HashSize);        // Content size
> +  write32<E>(Buf + 8, NT_GNU_BUILD_ID); // Type
> +  memcpy(Buf + 12, "GNU", 4);           // Name string
>  }
>
>  template <class ELFT>
>
> Modified: lld/trunk/ELF/SyntheticSections.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.h?rev=286100&r1=286099&r2=286100&view=diff
> ==============================================================================
> --- lld/trunk/ELF/SyntheticSections.h (original)
> +++ lld/trunk/ELF/SyntheticSections.h Mon Nov  7 03:04:06 2016
> @@ -30,6 +30,8 @@ public:
>  // .note.gnu.build-id section.
>  template <class ELFT> class BuildIdSection : public InputSection<ELFT> {
>  public:
> +  void writeTo(uint8_t *Buf) override;
> +  size_t getSize() const override { return 16 + HashSize; }
>    virtual void writeBuildId(llvm::MutableArrayRef<uint8_t> Buf) = 0;
>    virtual ~BuildIdSection() = default;
>
> @@ -37,7 +39,6 @@ public:
>
>  protected:
>    BuildIdSection(size_t HashSize);
> -  std::vector<uint8_t> Buf;
>
>    void
>    computeHash(llvm::MutableArrayRef<uint8_t> Buf,
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list