[PATCH] D43958: [llvm-readobj][ELF] Move ELF note parsing into lib/Object
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 12 09:21:29 PDT 2018
LGTM
Thanks!
Scott Linder via Phabricator <reviews at reviews.llvm.org> writes:
> scott.linder updated this revision to Diff 137241.
> scott.linder added a comment.
>
> Address feedback from Rafael and Tony.
>
> - ProcessNote now accepts a reference, thank you for the catch.
> - Removed the `assert` on `Err` in the iterator constructor; the check actually marks the `Error` as "checked", which is incorrect.
> - Moved the "whole note" bound checks up into the iterator, unifying the bound checks in once place.
>
> All of the public interfaces are still safe, and there is a bit less waste (not copying around `MaxSize`, not checking the "whole note" bounds multiple times per iteration).
>
> As for Rafael's comment on checking errors in each iteration, there is no need as the iterator is careful to always stop iteration when an error is encountered, so one check after iteration ends should suffice.
>
>
> https://reviews.llvm.org/D43958
>
> Files:
> include/llvm/BinaryFormat/ELF.h
> include/llvm/Object/ELF.h
> include/llvm/Object/ELFTypes.h
> tools/llvm-readobj/ELFDumper.cpp
>
> Index: tools/llvm-readobj/ELFDumper.cpp
> ===================================================================
> --- tools/llvm-readobj/ELFDumper.cpp
> +++ tools/llvm-readobj/ELFDumper.cpp
> @@ -92,6 +92,7 @@
> using Elf_Word = typename ELFT::Word; \
> using Elf_Hash = typename ELFT::Hash; \
> using Elf_GnuHash = typename ELFT::GnuHash; \
> + using Elf_Note = typename ELFT::Note; \
> using Elf_Sym_Range = typename ELFT::SymRange; \
> using Elf_Versym = typename ELFT::Versym; \
> using Elf_Verneed = typename ELFT::Verneed; \
> @@ -3534,62 +3535,57 @@
> const Elf_Ehdr *e = Obj->getHeader();
> bool IsCore = e->e_type == ELF::ET_CORE;
>
> - auto process = [&](const typename ELFT::Off Offset,
> - const typename ELFT::Addr Size) {
> - if (Size <= 0)
> - return;
> -
> - const auto *P = static_cast<const uint8_t *>(Obj->base() + Offset);
> - const auto *E = P + Size;
> -
> + auto PrintHeader = [&](const typename ELFT::Off Offset,
> + const typename ELFT::Addr Size) {
> OS << "Displaying notes found at file offset " << format_hex(Offset, 10)
> << " with length " << format_hex(Size, 10) << ":\n"
> << " Owner Data size\tDescription\n";
> + };
>
> - while (P < E) {
> - const Elf_Word *Words = reinterpret_cast<const Elf_Word *>(&P[0]);
> -
> - uint32_t NameSize = Words[0];
> - uint32_t DescriptorSize = Words[1];
> - uint32_t Type = Words[2];
> -
> - ArrayRef<Elf_Word> Descriptor(&Words[3 + (alignTo<4>(NameSize) / 4)],
> - alignTo<4>(DescriptorSize) / 4);
> -
> - StringRef Name;
> - if (NameSize)
> - Name =
> - StringRef(reinterpret_cast<const char *>(&Words[3]), NameSize - 1);
> -
> - OS << " " << Name << std::string(22 - NameSize, ' ')
> - << format_hex(DescriptorSize, 10) << '\t';
> -
> - if (Name == "GNU") {
> - OS << getGNUNoteTypeName(Type) << '\n';
> - printGNUNote<ELFT>(OS, Type, Descriptor, DescriptorSize);
> - } else if (Name == "FreeBSD") {
> - OS << getFreeBSDNoteTypeName(Type) << '\n';
> - } else if (Name == "AMD") {
> - OS << getAMDGPUNoteTypeName(Type) << '\n';
> - printAMDGPUNote<ELFT>(OS, Type, Descriptor, DescriptorSize);
> - } else {
> - OS << "Unknown note type: (" << format_hex(Type, 10) << ')';
> - }
> - OS << '\n';
> -
> - P = P + 3 * sizeof(Elf_Word) + alignTo<4>(NameSize) +
> - alignTo<4>(DescriptorSize);
> + auto ProcessNote = [&](const Elf_Note &Note) {
> + StringRef Name = Note.getName();
> + ArrayRef<Elf_Word> Descriptor = Note.getDesc();
> + Elf_Word Type = Note.getType();
> +
> + OS << " " << Name << std::string(22 - Name.size(), ' ')
> + << format_hex(Descriptor.size(), 10) << '\t';
> +
> + if (Name == "GNU") {
> + OS << getGNUNoteTypeName(Type) << '\n';
> + printGNUNote<ELFT>(OS, Type, Descriptor, Descriptor.size());
> + } else if (Name == "FreeBSD") {
> + OS << getFreeBSDNoteTypeName(Type) << '\n';
> + } else if (Name == "AMD") {
> + OS << getAMDGPUNoteTypeName(Type) << '\n';
> + printAMDGPUNote<ELFT>(OS, Type, Descriptor, Descriptor.size());
> + } else {
> + OS << "Unknown note type: (" << format_hex(Type, 10) << ')';
> }
> + OS << '\n';
> };
>
> if (IsCore) {
> - for (const auto &P : unwrapOrError(Obj->program_headers()))
> - if (P.p_type == PT_NOTE)
> - process(P.p_offset, P.p_filesz);
> + for (const auto &P : unwrapOrError(Obj->program_headers())) {
> + if (P.p_type != PT_NOTE)
> + continue;
> + PrintHeader(P.p_offset, P.p_filesz);
> + Error Err = Error::success();
> + for (const auto &Note : Obj->notes(P, Err))
> + ProcessNote(Note);
> + if (Err)
> + error(std::move(Err));
> + }
> } else {
> - for (const auto &S : unwrapOrError(Obj->sections()))
> - if (S.sh_type == SHT_NOTE)
> - process(S.sh_offset, S.sh_size);
> + for (const auto &S : unwrapOrError(Obj->sections())) {
> + if (S.sh_type != SHT_NOTE)
> + continue;
> + PrintHeader(S.sh_offset, S.sh_size);
> + Error Err = Error::success();
> + for (const auto &Note : Obj->notes(S, Err))
> + ProcessNote(Note);
> + if (Err)
> + error(std::move(Err));
> + }
> }
> }
>
> Index: include/llvm/Object/ELFTypes.h
> ===================================================================
> --- include/llvm/Object/ELFTypes.h
> +++ include/llvm/Object/ELFTypes.h
> @@ -40,6 +40,9 @@
> template <class ELFT> struct Elf_Hash_Impl;
> template <class ELFT> struct Elf_GnuHash_Impl;
> template <class ELFT> struct Elf_Chdr_Impl;
> +template <class ELFT> struct Elf_Nhdr_Impl;
> +template <class ELFT> class Elf_Note_Impl;
> +template <class ELFT> class Elf_Note_Iterator_Impl;
>
> template <endianness E, bool Is64> struct ELFType {
> private:
> @@ -66,6 +69,9 @@
> using Hash = Elf_Hash_Impl<ELFType<E, Is64>>;
> using GnuHash = Elf_GnuHash_Impl<ELFType<E, Is64>>;
> using Chdr = Elf_Chdr_Impl<ELFType<E, Is64>>;
> + using Nhdr = Elf_Nhdr_Impl<ELFType<E, Is64>>;
> + using Note = Elf_Note_Impl<ELFType<E, Is64>>;
> + using NoteIterator = Elf_Note_Iterator_Impl<ELFType<E, Is64>>;
> using DynRange = ArrayRef<Dyn>;
> using ShdrRange = ArrayRef<Shdr>;
> using SymRange = ArrayRef<Sym>;
> @@ -551,6 +557,127 @@
> Elf_Xword ch_addralign;
> };
>
> +/// Note header
> +template <class ELFT>
> +struct Elf_Nhdr_Impl {
> + LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
> + Elf_Word n_namesz;
> + Elf_Word n_descsz;
> + Elf_Word n_type;
> +
> + /// The alignment of the name and descriptor.
> + ///
> + /// Implementations differ from the specification here: in practice all
> + /// variants align both the name and descriptor to 4-bytes.
> + static const unsigned int Align = 4;
> +
> + /// Get the size of the note, including name, descriptor, and padding.
> + size_t getSize() const {
> + return sizeof(*this) + alignTo<Align>(n_namesz) + alignTo<Align>(n_descsz);
> + }
> +};
> +
> +/// An ELF note.
> +///
> +/// Wraps a note header, providing methods for accessing the name and
> +/// descriptor safely.
> +template <class ELFT>
> +class Elf_Note_Impl {
> + LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
> +
> + const Elf_Nhdr_Impl<ELFT> &Nhdr;
> +
> + template <class NoteIteratorELFT> friend class Elf_Note_Iterator_Impl;
> +
> + Elf_Note_Impl(const Elf_Nhdr_Impl<ELFT> &Nhdr) : Nhdr(Nhdr) {}
> +
> +public:
> + /// Get the note's name, excluding the terminating null byte.
> + StringRef getName() const {
> + if (!Nhdr.n_namesz)
> + return StringRef();
> + return StringRef(reinterpret_cast<const char *>(&Nhdr) + sizeof(Nhdr),
> + Nhdr.n_namesz - 1);
> + }
> +
> + /// Get the note's descriptor.
> + ArrayRef<Elf_Word> getDesc() const {
> + if (!Nhdr.n_descsz)
> + return ArrayRef<Elf_Word>();
> + return ArrayRef<Elf_Word>(
> + reinterpret_cast<const Elf_Word *>(
> + reinterpret_cast<const uint8_t *>(&Nhdr) + sizeof(Nhdr) +
> + alignTo<Elf_Nhdr_Impl<ELFT>::Align>(Nhdr.n_namesz)),
> + Nhdr.n_descsz);
> + }
> +
> + /// Get the note's type.
> + Elf_Word getType() const { return Nhdr.n_type; }
> +};
> +
> +template <class ELFT>
> +class Elf_Note_Iterator_Impl
> + : std::iterator<std::forward_iterator_tag, Elf_Note_Impl<ELFT>> {
> + // Nhdr being a nullptr marks the end of iteration.
> + const Elf_Nhdr_Impl<ELFT> *Nhdr = nullptr;
> + size_t RemainingSize = 0u;
> + Error *Err = nullptr;
> +
> + template <class ELFFileELFT> friend class ELFFile;
> +
> + // Stop iteration and indicate an overflow.
> + void stopWithOverflowError() {
> + Nhdr = nullptr;
> + *Err = make_error<StringError>("ELF note overflows container",
> + object_error::parse_failed);
> + }
> +
> + // Advance Nhdr by NoteSize bytes, starting from NhdrPos.
> + //
> + // Assumes NoteSize <= RemainingSize. Ensures Nhdr->getSize() <= RemainingSize
> + // upon returning. Handles stopping iteration when reaching the end of the
> + // container, either cleanly or with an overflow error.
> + void advanceNhdr(const uint8_t *NhdrPos, size_t NoteSize) {
> + RemainingSize -= NoteSize;
> + if (RemainingSize == 0u)
> + Nhdr = nullptr;
> + else if (sizeof(*Nhdr) > RemainingSize)
> + stopWithOverflowError();
> + else {
> + Nhdr = reinterpret_cast<const Elf_Nhdr_Impl<ELFT> *>(NhdrPos + NoteSize);
> + if (Nhdr->getSize() > RemainingSize)
> + stopWithOverflowError();
> + }
> + }
> +
> + Elf_Note_Iterator_Impl() {}
> + explicit Elf_Note_Iterator_Impl(Error &Err) : Err(&Err) {}
> + Elf_Note_Iterator_Impl(const uint8_t *Start, size_t Size, Error &Err)
> + : RemainingSize(Size), Err(&Err) {
> + assert(Start && "ELF note iterator starting at NULL");
> + advanceNhdr(Start, 0u);
> + }
> +
> +public:
> + Elf_Note_Iterator_Impl &operator++() {
> + assert(Nhdr && "incremented ELF note end iterator");
> + const uint8_t *NhdrPos = reinterpret_cast<const uint8_t *>(Nhdr);
> + size_t NoteSize = Nhdr->getSize();
> + advanceNhdr(NhdrPos, NoteSize);
> + return *this;
> + }
> + bool operator==(Elf_Note_Iterator_Impl Other) const {
> + return Nhdr == Other.Nhdr;
> + }
> + bool operator!=(Elf_Note_Iterator_Impl Other) const {
> + return !(*this == Other);
> + }
> + Elf_Note_Impl<ELFT> operator*() const {
> + assert(Nhdr && "dereferenced ELF note end iterator");
> + return Elf_Note_Impl<ELFT>(*Nhdr);
> + }
> +};
> +
> // MIPS .reginfo section
> template <class ELFT>
> struct Elf_Mips_RegInfo;
> Index: include/llvm/Object/ELF.h
> ===================================================================
> --- include/llvm/Object/ELF.h
> +++ include/llvm/Object/ELF.h
> @@ -67,6 +67,9 @@
> using Elf_Versym = typename ELFT::Versym;
> using Elf_Hash = typename ELFT::Hash;
> using Elf_GnuHash = typename ELFT::GnuHash;
> + using Elf_Nhdr = typename ELFT::Nhdr;
> + using Elf_Note = typename ELFT::Note;
> + using Elf_Note_Iterator = typename ELFT::NoteIterator;
> using Elf_Dyn_Range = typename ELFT::DynRange;
> using Elf_Shdr_Range = typename ELFT::ShdrRange;
> using Elf_Sym_Range = typename ELFT::SymRange;
> @@ -155,6 +158,73 @@
> return makeArrayRef(Begin, Begin + getHeader()->e_phnum);
> }
>
> + /// Get an iterator over notes in a program header.
> + ///
> + /// The program header must be of type \c PT_NOTE.
> + ///
> + /// \param Phdr the program header to iterate over.
> + /// \param Err [out] an error to support fallible iteration, which should
> + /// be checked after iteration ends.
> + Elf_Note_Iterator notes_begin(const Elf_Phdr &Phdr, Error &Err) const {
> + if (Phdr.p_type != ELF::PT_NOTE) {
> + Err = createError("attempt to iterate notes of non-note program header");
> + return Elf_Note_Iterator(Err);
> + }
> + if (Phdr.p_offset + Phdr.p_filesz > getBufSize()) {
> + Err = createError("invalid program header offset/size");
> + return Elf_Note_Iterator(Err);
> + }
> + return Elf_Note_Iterator(base() + Phdr.p_offset, Phdr.p_filesz, Err);
> + }
> +
> + /// Get an iterator over notes in a section.
> + ///
> + /// The section must be of type \c SHT_NOTE.
> + ///
> + /// \param Shdr the section to iterate over.
> + /// \param Err [out] an error to support fallible iteration, which should
> + /// be checked after iteration ends.
> + Elf_Note_Iterator notes_begin(const Elf_Shdr &Shdr, Error &Err) const {
> + if (Shdr.sh_type != ELF::SHT_NOTE) {
> + Err = createError("attempt to iterate notes of non-note section");
> + return Elf_Note_Iterator(Err);
> + }
> + if (Shdr.sh_offset + Shdr.sh_size > getBufSize()) {
> + Err = createError("invalid section offset/size");
> + return Elf_Note_Iterator(Err);
> + }
> + return Elf_Note_Iterator(base() + Shdr.sh_offset, Shdr.sh_size, Err);
> + }
> +
> + /// Get the end iterator for notes.
> + Elf_Note_Iterator notes_end() const {
> + return Elf_Note_Iterator();
> + }
> +
> + /// Get an iterator range over notes of a program header.
> + ///
> + /// The program header must be of type \c PT_NOTE.
> + ///
> + /// \param Phdr the program header to iterate over.
> + /// \param Err [out] an error to support fallible iteration, which should
> + /// be checked after iteration ends.
> + iterator_range<Elf_Note_Iterator> notes(const Elf_Phdr &Phdr,
> + Error &Err) const {
> + return make_range(notes_begin(Phdr, Err), notes_end());
> + }
> +
> + /// Get an iterator range over notes of a section.
> + ///
> + /// The section must be of type \c SHT_NOTE.
> + ///
> + /// \param Shdr the section to iterate over.
> + /// \param Err [out] an error to support fallible iteration, which should
> + /// be checked after iteration ends.
> + iterator_range<Elf_Note_Iterator> notes(const Elf_Shdr &Shdr,
> + Error &Err) const {
> + return make_range(notes_begin(Shdr, Err), notes_end());
> + }
> +
> Expected<StringRef> getSectionStringTable(Elf_Shdr_Range Sections) const;
> Expected<uint32_t> getSectionIndex(const Elf_Sym *Sym, Elf_Sym_Range Syms,
> ArrayRef<Elf_Word> ShndxTable) const;
> Index: include/llvm/BinaryFormat/ELF.h
> ===================================================================
> --- include/llvm/BinaryFormat/ELF.h
> +++ include/llvm/BinaryFormat/ELF.h
> @@ -1483,6 +1483,20 @@
> Elf64_Xword ch_addralign;
> };
>
> +// Node header for ELF32.
> +struct Elf32_Nhdr {
> + Elf32_Word n_namesz;
> + Elf32_Word n_descsz;
> + Elf32_Word n_type;
> +};
> +
> +// Node header for ELF64.
> +struct Elf64_Nhdr {
> + Elf64_Word n_namesz;
> + Elf64_Word n_descsz;
> + Elf64_Word n_type;
> +};
> +
> // Legal values for ch_type field of compressed section header.
> enum {
> ELFCOMPRESS_ZLIB = 1, // ZLIB/DEFLATE algorithm.
More information about the llvm-commits
mailing list