[llvm-commits] PATCH: lld ReaderELF.cpp update
Michael Spencer
bigcheesegs at gmail.com
Fri Jul 13 16:16:07 PDT 2012
On Fri, Jul 13, 2012 at 11:44 AM, Sid Manning <sidneym at codeaurora.org> wrote:
>
> There are 3 attachments here for lld ELFReader support that I would like to
> get feedback on.
>
> Thanks,
>
>
> - make-get-Symbol-Section-public.diff:
>
> This change makes getSymbol and getSection public. These are used by
> ReaderELF to get access to symbol and section information. The ElfReader
> requires this change or it will not compile. Because ELF.h is in a
> different repository I made a different patch for it.
>
> Index: include/llvm/Object/ELF.h
> ===================================================================
> --- include/llvm/Object/ELF.h (revision 160133)
> +++ include/llvm/Object/ELF.h (working copy)
> @@ -499,7 +499,6 @@
> const T *getEntry(uint16_t Section, uint32_t Entry) const;
> template<typename T>
> const T *getEntry(const Elf_Shdr *Section, uint32_t Entry) const;
> - const Elf_Shdr *getSection(DataRefImpl index) const;
> const Elf_Shdr *getSection(uint32_t index) const;
> const Elf_Rel *getRel(DataRefImpl Rel) const;
> const Elf_Rela *getRela(DataRefImpl Rela) const;
> @@ -515,10 +514,11 @@
> void VerifyStrTab(const Elf_Shdr *sh) const;
>
> protected:
> - const Elf_Sym *getSymbol(DataRefImpl Symb) const; // FIXME: Should be private?
> void validateSymbol(DataRefImpl Symb) const;
>
> public:
> + const Elf_Shdr *getSection(DataRefImpl index) const;
> + const Elf_Sym *getSymbol(DataRefImpl Symb) const;
> const Elf_Dyn *getDyn(DataRefImpl DynData) const;
> error_code getSymbolVersion(SymbolRef Symb, StringRef &Version,
> bool &IsDefault) const;
>
I realize you were following the example of getDyn here. However, all
of these should take a {section,symbol,dynasymbol,relocation}_iterator
instead as COFF does. DataRefImpl should never be used by clients of
LLVMObject. There's no need for you to change getDyn over if it's not
trivial.
> - add-ElfReader-and-tests.diff:
>
> This is an updated version of Hemant Kulkarni's original submission. It
> uses the ELF structures directly, fills in some functions and hopefully
> follows the coding guidelines.
>
> lld-core is modified to take a "-reader" argument. This still defaults to
> YAML so the rest of the tests still run. I added 2 binary object files, one
> i386 and one hexagon. At the moment there is no YAML to ELF translator so
> these were added to handle interim testing.
>
> The COFF reader needed to be tweaked - see
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120625/145537.html.
> There may be a better way to deal with COFF but this patch includes the
> above change because I wanted to verify that lld-core would work with a COFF
> object file.
>
>
> - testInputs.tgz:
> This tarball has 2 files that will expand to:
> test/elf/Inputs/object-test.elf-hexagon
> test/elf/Inputs/object-test.elf-i386
>
> These samples were based on the code below, using some examples found in
> DefinedAtom.h. i386 was built with gcc -m32. hexagon was built with gcc
> -mv4 -G0.
>
> int common_symbol[0x100] __attribute__ ((aligned (16)));
> void global_func() { puts ("global"); }
> static void static_func() { puts ("static"); }
> int global_variable = 1;
> int tentative;
> static int uninitialized_static_variable;
> __attribute__((weak)) void weak_func() { puts ("weak"); }
> __attribute__((visibility("hidden"))) void hidden_func() { puts("hidden"); }
> __attribute__((used)) void no_dead_strip() {}
> __attribute__((section ("special_section"))) void special_section_func() {}
> void no_special_section_func() {}
>
> I definitely see value of keeping tests in YAML but at the moment there is
> no ELF->YAML translator so this was the only way I could include test cases
> for the reader.
> ===================================================================
> --- test/elf/check.objtxt (revision 0)
> +++ test/elf/check.objtxt (revision 0)
> @@ -0,0 +1,167 @@
> +:# RUN: lld-core -reader ELF %p/Inputs/object-test.elf-i386 | FileCheck %s -check-prefix ELF-i386
> +:# RUN: lld-core -reader ELF %p/Inputs/object-test.elf-hexagon | FileCheck %s -check-prefix ELF-hexagon
> +
What's with the : at the front of the run lines?
Index: lib/ReaderWriter/ELF/ReaderELF.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/ReaderELF.cpp (revision 160133)
> +++ lib/ReaderWriter/ELF/ReaderELF.cpp (working copy)
> @@ -1,28 +1,546 @@
> -//===- lib/ReaderWriter/ELF/ReaderELF.cpp --------------------------------===//
> +//===- lib/ReaderWriter/ELF/ReaderELF.cpp ---------------------------------===//
> //
> // The LLVM Linker
> //
> // This file is distributed under the University of Illinois Open Source
> // License. See LICENSE.TXT for details.
> //
> -//===----------------------------------------------------------------------===//
> +//===---------------------------------------------------------------------===//
> +//
> +// This file contains the ELF Reader and all helper sub classes
> +// to consume an ELF file and produces atoms out of it.
> +//
> +//===---------------------------------------------------------------------===//
These lines are the wrong length.
>
> #include "lld/ReaderWriter/ReaderELF.h"
> #include "lld/Core/File.h"
>
> +#include "llvm/ADT/ArrayRef.h"
> +#include "llvm/ADT/StringRef.h"
> +#include "llvm/Support/Allocator.h"
> #include "llvm/Support/Casting.h"
> #include "llvm/Support/ErrorHandling.h"
> #include "llvm/Support/Memory.h"
> #include "llvm/Support/MemoryBuffer.h"
> #include "llvm/Support/raw_ostream.h"
> #include "llvm/Support/system_error.h"
> +#include "llvm/Support/Endian.h"
> +#include "llvm/Support/MathExtras.h"
> +#include "llvm/Support/ELF.h"
> +#include "llvm/Object/ObjectFile.h"
> +#include "llvm/Object/ELF.h"
These need to be sorted in alphabetical order.
> +
> #include <map>
> #include <vector>
>
>
> +
> +using llvm::object::Elf_Sym_Impl;
> +using namespace lld;
> +
> +// This atom class corresponds to absolute symbol
> namespace lld {
> +namespace ELF {
>
> +class ELFAbsoluteAtom: public AbsoluteAtom {
> +
> +public:
> + ELFAbsoluteAtom(const File &F,
> + llvm::StringRef N,
> + uint64_t V)
> + : OwningFile(F)
> + , Name(N)
> + , Value(V)
> + {}
> +
> + virtual const class File& file() const {
& on the right side. A few other places need this too.
> + return OwningFile;
> + }
> +
> + virtual llvm::StringRef name() const {
> + return Name;
> + }
> +
> + virtual uint64_t value() const {
> + return Value;
> + }
> +
> +private:
> + const File &OwningFile;
> + llvm::StringRef Name;
> + uint64_t Value;
> +};
> +
> +
> +// This atom corresponds to undefined symbols.
> +template<llvm::support::endianness target_endianness, bool is64Bits>
> +class ELFUndefinedAtom: public UndefinedAtom {
> +
> + typedef llvm::object::Elf_Sym_Impl<target_endianness, is64Bits> Elf_Sym;
> +
> +public:
> + ELFUndefinedAtom(const File &F,
> + llvm::StringRef N,
> + const Elf_Sym *E)
> + : OwningFile(F)
> + , Name(N)
> + , Symbol(E)
> + {}
> +
> + virtual const class File& file() const {
> + return OwningFile;
> + }
> +
> + virtual llvm::StringRef name() const {
> + return Name;
> + }
> +
> + // FIXME What distinguishes a symbol in ELF that can help
> + // decide if the symbol is undefined only during build and not
> + // runtime? This will make us choose canBeNullAtBuildtime and
> + // canBeNullAtRuntime
> + //
> + virtual CanBeNull canBeNull() const {
> +
> + if (Symbol->getBinding() == llvm::ELF::STB_WEAK)
> + return CanBeNull::canBeNullAtBuildtime;
> + else
> + return CanBeNull::canBeNullNever;
> + }
> +
> +private:
> + const File &OwningFile;
> + llvm::StringRef Name;
> + const Elf_Sym *Symbol;
> +};
> +
> +
> +template<llvm::support::endianness target_endianness, bool is64Bits>
> +class ELFDefinedAtom: public DefinedAtom {
> +
> + typedef llvm::object::Elf_Sym_Impl<target_endianness, is64Bits> Elf_Sym;
> + typedef llvm::object::Elf_Shdr_Impl<target_endianness, is64Bits> Elf_Shdr;
> +
> +public:
> + ELFDefinedAtom(const File &F,
> + llvm::StringRef N,
> + llvm::StringRef SN,
> + const Elf_Sym *E,
> + const Elf_Shdr *S,
> + llvm::ArrayRef<uint8_t> D)
> + : OwningFile(F)
> + , SymbolName(N)
> + , SectionName(SN)
> + , Symbol(E)
> + , Section(S)
> + , Data(D) {
> + static uint64_t ordernumber = 0;
> + _ordinal = ++ordernumber;
> + }
> +
> + virtual const class File& file() const {
> + return OwningFile;
> + }
> +
> + virtual llvm::StringRef name() const {
> + return SymbolName;
> + }
> +
> + virtual uint64_t ordinal() const {
> + return _ordinal;
> + }
> +
> + virtual uint64_t size() const {
> +
> + // Common symbols are not allocated in object files so
> + // their size is zero.
> + if ((Symbol->getType() == llvm::ELF::STT_COMMON)
> + || Symbol->st_shndx == llvm::ELF::SHN_COMMON)
> + return (uint64_t)0;
> +
> + return (uint64_t) Symbol->st_size;
> + }
> +
> + virtual Scope scope() const {
> + if (Symbol->st_other == llvm::ELF::STV_HIDDEN)
> + return scopeLinkageUnit;
> + else if (Symbol->getBinding() != llvm::ELF::STB_LOCAL)
> + return scopeGlobal;
> + else
> + return scopeTranslationUnit;
> + }
> +
> + // FIXME Need to revisit this in future.
> +
> + virtual Interposable interposable() const {
> + return interposeNo;
> + }
> +
> + // FIXME What ways can we determine this in ELF?
> +
> + virtual Merge merge() const {
> +
> + if (Symbol->getBinding() == llvm::ELF::STB_WEAK)
> + return mergeAsWeak;
> +
> + if ((Symbol->getType() == llvm::ELF::STT_COMMON)
> + || Symbol->st_shndx == llvm::ELF::SHN_COMMON)
> + return mergeAsTentative;
> +
> + return mergeNo;
> + }
> +
> + virtual ContentType contentType() const {
> +
> + if (Symbol->getType() == llvm::ELF::STT_FUNC)
> + return typeCode;
> +
> + if ((Symbol->getType() == llvm::ELF::STT_COMMON)
> + || Symbol->st_shndx == llvm::ELF::SHN_COMMON)
> + return typeZeroFill;
> +
> + if (Symbol->getType() == llvm::ELF::STT_OBJECT)
> + return typeData;
> +
> + return typeUnknown;
> + }
> +
> + virtual Alignment alignment() const {
> +
> + // Unallocated common symbols specific their alignment
> + // constraints in st_value.
> + if ((Symbol->getType() == llvm::ELF::STT_COMMON)
> + || Symbol->st_shndx == llvm::ELF::SHN_COMMON) {
> + return (Alignment (Symbol->st_value));
> + }
> +
> + return Alignment(1);
> + }
> +
> + // Do we have a choice for ELF? All symbols
> + // live in explicit sections.
> + virtual SectionChoice sectionChoice() const {
> + if (Symbol->st_shndx > llvm::ELF::SHN_LORESERVE)
> + return sectionBasedOnContent;
> +
> + return sectionCustomRequired;
> + }
> +
> + virtual llvm::StringRef customSectionName() const {
> + return SectionName;
> + }
> +
> + // It isn't clear that __attribute__((used)) is transmitted to
> + // the ELF object file.
> + virtual DeadStripKind deadStrip() const {
> + return deadStripNormal;
> + }
> +
> + virtual ContentPermissions permissions() const {
> +
> + switch (Section->sh_type) {
> + // permRW_L is for sections modified by the runtime
> + // loader.
> + case llvm::ELF::SHT_REL:
> + case llvm::ELF::SHT_RELA:
> + return permRW_L;
> +
> + case llvm::ELF::SHT_DYNAMIC:
> + case llvm::ELF::SHT_PROGBITS:
> + switch (Section->sh_flags) {
> +
> + case (llvm::ELF::SHF_ALLOC | llvm::ELF::SHF_EXECINSTR):
> + return permR_X;
> +
> + case (llvm::ELF::SHF_ALLOC | llvm::ELF::SHF_WRITE):
> + return permRW_;
> +
> + case llvm::ELF::SHF_ALLOC:
> + case (llvm::ELF::SHF_ALLOC | llvm::ELF::SHF_MERGE):
> + case (llvm::ELF::SHF_ALLOC | llvm::ELF::SHF_MERGE
> + | llvm::ELF::SHF_STRINGS):
> + return permR__;
> + }
> + default:
> + return perm___;
> + }
> + }
> +
> + // Many non ARM architectures use ELF file format
> + // This not really a place to put a architecture
> + // specific method in an atom. A better approach is
> + // needed.
> + //
> + virtual bool isThumb() const {
> + return false;
> + }
> +
> + // FIXME Not Sure if ELF supports alias atoms. Find out more.
> + virtual bool isAlias() const {
> + return false;
> + }
> +
> + virtual llvm::ArrayRef<uint8_t> rawContent() const {
> + return Data;
> + }
> +
> + virtual reference_iterator begin() const {
> + return reference_iterator(*this, nullptr);
> + }
> +
> + virtual reference_iterator end() const {
> + return reference_iterator(*this, nullptr);
> + }
> +
> +private:
> + virtual const Reference* derefIterator(const void* iter) const {
> + return nullptr;
> + }
> + virtual void incrementIterator(const void*& iter) const {
> + }
> +
> + const File &OwningFile;
> + llvm::StringRef SymbolName;
> + llvm::StringRef SectionName;
> + const Elf_Sym *Symbol;
> + const Elf_Shdr *Section;
> +
> + // Data will hold the bits that make up the atom.
> + llvm::ArrayRef<uint8_t> Data;
> +
> + uint64_t _ordinal;
> +};
> +
> +
> +// FileELF will read a binary, find out based on the symbol table contents
> +// what kind of symbol it is and create corresponding atoms for it
> +
> +template<llvm::support::endianness target_endianness, bool is64Bits>
> +class FileELF: public File {
> +
> + typedef llvm::object::Elf_Sym_Impl<target_endianness, is64Bits> Elf_Sym;
> + typedef llvm::object::Elf_Shdr_Impl<target_endianness, is64Bits> Elf_Shdr;
> +
> +public:
> + FileELF(std::unique_ptr<llvm::MemoryBuffer> MB, llvm::error_code &EC) :
> + File(MB->getBufferIdentifier()) {
> +
> + llvm::OwningPtr<llvm::object::Binary> Bin;
> + EC = llvm::object::createBinary(MB.release(), Bin);
> + if (EC)
> + return;
> +
> + // Point Obj to correct class and bitwidth ELF object
> + Obj.reset(llvm::dyn_cast<llvm::object::ELFObjectFile<target_endianness,
> + is64Bits> >(Bin.get()));
> +
> + if (!Obj) {
> + EC = make_error_code(llvm::object::object_error::invalid_file_type);
> + return;
> + }
> +
> + Bin.take();
> +
> +
> + llvm::object::symbol_iterator si(Obj->begin_symbols());
> + llvm::object::symbol_iterator se(Obj->end_symbols());
> +
> + for (; si != se; si.increment(EC)) {
> + if (EC)
> + llvm::report_fatal_error("Could not read all symbols");
> + llvm::StringRef SymbolName;
> + llvm::StringRef SectionName;
> +
> + llvm::object::SymbolRef::Type type;
> + uint32_t flags = 0;
> +
> + EC = si->getName(SymbolName);
For this and all the below code, why are you using the generic
interface instead of the ELF specific interface? Quite a bit of this
would be a lot cleaner with the ELF interface.
> + if (EC)
> + llvm::report_fatal_error("Could not get symbol name");
> +
> + EC = si->getType(type);
> + if (EC)
> + llvm::report_fatal_error("Could not get symbol type");
> +
> + EC = si->getFlags(flags);
> + if (EC)
> + llvm::report_fatal_error("Could not get symbol flags");
> +
> + if ((flags & llvm::object::SymbolRef::SF_Absolute)
> + == llvm::object::SymbolRef::SF_Absolute) {
> + uint64_t AbsValue;
> +
> + EC = si->getAddress(AbsValue);
> + if (EC)
> + llvm::report_fatal_error(
> + "Could not get absolute symbol st_value");
> +
> + AbsoluteAtoms._atoms.push_back(
> + new (AtomStorage.Allocate<ELFAbsoluteAtom> ())
> + ELFAbsoluteAtom(*this, SymbolName, AbsValue));
> +
> + } else if ((flags & llvm::object::SymbolRef::SF_Undefined)
> + == llvm::object::SymbolRef::SF_Undefined) {
> +
> + // Get a pointer to the symbol:
> + const Elf_Sym *Symbol = (Obj->getSymbol(si->getRawDataRefImpl()));
> +
> + UndefinedAtoms._atoms.push_back(
> + new (AtomStorage.Allocate<ELFUndefinedAtom<
> + target_endianness, is64Bits> > ())
No space needed between the > and >. Also there should be no space
between the > and (.
> + ELFUndefinedAtom<target_endianness, is64Bits> (
> + *this, SymbolName, Symbol));
> +
> + } else {
> + uint32_t SymbolFlags;
> + EC = si->getFlags(SymbolFlags);
> + if (EC)
> + llvm::report_fatal_error("Could not get symbol flags");
> +
> + //TODO We need to think about how to put other
> + //symbol types into the defined atom model.
> +
> + if (type == llvm::object::SymbolRef::ST_Data ||
> + type == llvm::object::SymbolRef::ST_Function ||
> + type == llvm::object::SymbolRef::ST_Unknown) {
> +
> + llvm::object::SectionRef SR;
> + llvm::object::section_iterator section(SR);
> +
> + EC = si->getSection(section);
> + if (EC)
> + llvm::report_fatal_error("Could not get section iterator");
> +
> + EC = section->getName(SectionName);
> + if (EC)
> + llvm::report_fatal_error("Could not get section name");
> +
> + StringRef symbolContents;
> + EC = section->getContents (symbolContents);
> + if (EC)
> + llvm::report_fatal_error("Could not get section contents");
> +
> + uint64_t symbolSize;
> + EC = si->getSize (symbolSize);
> + if (EC)
> + llvm::report_fatal_error("Could not get symbol size");
> +
> + uint64_t symbolOffset;
> + EC = si->getAddress (symbolOffset);
> + if (EC)
> + llvm::report_fatal_error("Could not get symbol address");
> +
> + bool IsCommon = false;
> + if ((SymbolFlags & llvm::object::SymbolRef::SF_Common)
> + == llvm::object::SymbolRef::SF_Common)
> + IsCommon = true;
> +
> + // Get the symbol's content:
> + llvm::ArrayRef<uint8_t> SymbolData((uint8_t *)symbolContents.data()
> + + symbolOffset,
> + (IsCommon) ? 0 : symbolSize);
st_size is not guaranteed to be the size of the data associated with
the symbol. See the MachO and COFF reader for how to properly deal
with this. This is important because every byte in a section must be
allocated to an atom, as sections can have internal references that we
don't know about that must be maintained.
> +
> + // Get a pointer to the section the symbol lives in:
> + llvm::object::DataRefImpl ShdrRef = section->getRawDataRefImpl();
> + Elf_Shdr *Section = const_cast<Elf_Shdr*>
> + (reinterpret_cast<const Elf_Shdr *>(ShdrRef.p));
Why is there a const cast here? ELFDefinedAtom takes a const Elf_Shdr
*. Also this should not rely on the implementation details of
DataRefImpl to get the Elf_Shdr pointer.
> +
> + // Get a pointer to the symbol:
> + const Elf_Sym *Symbol = (Obj->getSymbol(si->getRawDataRefImpl()));
> +
> + DefinedAtoms._atoms.push_back(
> + new (AtomStorage.Allocate<ELFDefinedAtom<
> + target_endianness, is64Bits> > ())
> + ELFDefinedAtom<target_endianness, is64Bits> (*this,
> + SymbolName, SectionName,
> + Symbol, Section, SymbolData));
> + }
> + }
> + }
> + }
> +
> + virtual void addAtom(const Atom&) {
> + llvm_unreachable("cannot add atoms to native .o files");
> + }
> +
> + virtual const atom_collection<DefinedAtom> &defined() const {
> + return DefinedAtoms;
> + }
> +
> + virtual const atom_collection<UndefinedAtom> &undefined() const {
> + return UndefinedAtoms;
> + }
> +
> + virtual const atom_collection<SharedLibraryAtom> &sharedLibrary() const {
> + return SharedLibraryAtoms;
> + }
> +
> + virtual const atom_collection<AbsoluteAtom> &absolute() const {
> + return AbsoluteAtoms;
> + }
> +
> +private:
> + std::unique_ptr<llvm::object::ELFObjectFile<target_endianness, is64Bits> >
> + Obj;
> + atom_collection_vector<DefinedAtom> DefinedAtoms;
> + atom_collection_vector<UndefinedAtom> UndefinedAtoms;
> + atom_collection_vector<SharedLibraryAtom> SharedLibraryAtoms;
> + atom_collection_vector<AbsoluteAtom> AbsoluteAtoms;
> + llvm::BumpPtrAllocator AtomStorage;
> +
> +};
> +
> +// ReaderELF is reader object that will instantiate correct FileELF
> +// by examining the memory buffer for ELF class and bitwidth
> +
> +class ReaderELF: public Reader {
> +public:
> + ReaderELF(const ReaderOptionsELF &options) :
> + _options(options) {
> + }
> + error_code parseFile(std::unique_ptr<MemoryBuffer> mb, std::vector<
> + std::unique_ptr<File> > &result) {
> +
> + std::pair<unsigned char, unsigned char> Ident =
> + llvm::object::getElfArchType(&*mb);
> + llvm::error_code ec;
> + // Instantiate the correct FileELF template instance
> + // based on the Ident pair. Once the File is created
> + // we push the file to the vector of files already
> + // created during parser's life.
> +
> + std::unique_ptr<File> f;
> +
> + if (Ident.first == llvm::ELF::ELFCLASS32 && Ident.second
> + == llvm::ELF::ELFDATA2LSB) {
> + f.reset(new FileELF<llvm::support::little, false>(std::move(mb), ec));
> +
> + } else if (Ident.first == llvm::ELF::ELFCLASS32 && Ident.second
> + == llvm::ELF::ELFDATA2MSB) {
> + f.reset(new FileELF<llvm::support::big, false> (std::move(mb), ec));
> +
> + } else if (Ident.first == llvm::ELF::ELFCLASS64 && Ident.second
> + == llvm::ELF::ELFDATA2MSB) {
> + f.reset(new FileELF<llvm::support::big, true> (std::move(mb), ec));
> +
> + } else if (Ident.first == llvm::ELF::ELFCLASS64 && Ident.second
> + == llvm::ELF::ELFDATA2LSB) {
> + f.reset(new FileELF<llvm::support::little, true> (std::move(mb), ec));
> + }
> +
> + if (ec)
> + return ec;
> +
> + result.push_back(std::move(f));
> + return error_code::success();
> + }
> +private:
> + const ReaderOptionsELF &_options;
> +};
> +} // namespace ELF
> +
> +
> ReaderOptionsELF::ReaderOptionsELF() {
> }
>
> @@ -30,12 +548,8 @@
> }
>
>
> -
> Reader* createReaderELF(const ReaderOptionsELF &options) {
> - assert(0 && "ELF Reader not yet implemented");
> - return nullptr;
> + return new ELF::ReaderELF(options);
> }
>
> -
> -} // namespace
> -
> +} // namespace LLD
>
>
>
>
- Michael Spencer
More information about the llvm-commits
mailing list