[llvm-commits] [PATCH] [lld] patch for ELF Writer to get it to 'Hello world'

Michael Spencer bigcheesegs at gmail.com
Fri Dec 21 19:54:07 PST 2012


On Fri, Dec 21, 2012 at 2:23 PM, Shankar Easwaran
<shankare at codeaurora.org> wrote:
> Hi Michael,
>
> Attached is the new diff after the review with the test case changes.
>
> You would need to apply the other changes that were in the previous patches
> to build.
>
> a) ELFAtoms.h
> b) ReaderELF changes
> c) HexagonRelocation changes
>
> OK to commit ?
>
> Thanks
>
> Shankar Easwaran
>
>
> On 12/21/2012 11:11 AM, Michael Spencer wrote:
>>
>>
>> ================
>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:80-86
>> @@ +79,9 @@
>> +  /// \brief Describes the type of Chunk
>> +  enum Kind {
>> +    ELFHeader, // ELF Header
>> +    ProgramHeader, // Program Header
>> +    Segment, // Segment
>> +    Section, // Section
>> +    SectionHeader // Section header
>> +  };
>> +  Chunk(StringRef name, Kind kind) : _name(name),
>> ----------------
>> Shankar Kalpathi Easwaran wrote:
>>>
>>> Michael Spencer wrote:
>>>>
>>>> This either needs to be an enum class, or the members need to follow the
>>>> naming convention. Currently this breaks MSVC because it doesn't do two
>>>> phase name lookup and therefore sees Section and Segment in the derived
>>>> class (Chunk) instead of the templates Section and Segment.
>>>
>>> Changed the names to add ELF in the front
>>
>>
>> http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
>>
>> They should start with K_
>>
>> ================
>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:121-123
>> @@ +120,5 @@
>> +  int64_t            fileOffset() const { return _fileoffset; }
>> +  void               setFileOffset(int64_t offset) { _fileoffset =
>> offset; }
>> +  // Output start address of the chunk
>> +  void               setStart(int64_t start) { _start = start; }
>> +  int64_t            startAddr() const { return _start; }
>> ----------------
>> Shankar Kalpathi Easwaran wrote:
>>>
>>> Michael Spencer wrote:
>>>>
>>>> What is the difference between these two function? Is one the virtual
>>>> address?
>>>
>>> yes.
>>
>> Then the names could be improved and a \brief added.
>>
>> ================
>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:549
>> @@ -235,1 +548,3 @@
>> +  void setOrdinal(int64_t ordinal) {
>> +    ordinal = ordinal;
>>     }
>> ----------------
>> Shankar Kalpathi Easwaran wrote:
>>>
>>> Michael Spencer wrote:
>>>>
>>>>    _ordinal = ordinal;
>>>
>>> Nice catch.
>>
>> Clang -Wall caught it for me :P
>>
>> ================
>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:1008
>> @@ +1007,3 @@
>> +  static inline bool classof(const Chunk<target_endianness, is64Bits> *c)
>> {
>> +    return c->kind() == Section<target_endianness,
>> is64Bits>::StringTable;
>> +  }
>> ----------------
>> Shankar Kalpathi Easwaran wrote:
>>>
>>> Michael Spencer wrote:
>>>>
>>>> This is broken. Chunk already has a Kind with the same value.
>>>
>>> Chunk doesnot have such a type called StringTable.
>>
>> Not named StringTable. But StringTable shares the same underlying value as
>> Chunk::Segment
>>
>> ================
>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:1491-1502
>> @@ -826,1 +1490,14 @@
>> +      case DefinedAtom::typeCode:
>> +          if (name.startswith(".eh_frame_hdr"))
>> +            return ORDER_EH_FRAMEHDR;
>> +          if (name.startswith(".eh_frame"))
>> +            return ORDER_EH_FRAME;
>> +          else if (name.startswith(".init"))
>> +            return ORDER_INIT;
>> +          else if (name.startswith(".fini"))
>> +            return ORDER_FINI;
>> +          else if (name.startswith(".hash"))
>> +            return ORDER_HASH;
>> +          else
>> +            return ORDER_TEXT;
>>   ----------------
>> Shankar Kalpathi Easwaran wrote:
>>>
>>> Michael Spencer wrote:
>>>>
>>>> llvm::StringSwitch in llvm/ADT/StringSwitch.h
>>>
>>> How do you do startswith with StringSwitch ?
>>
>> .StartsWith("") instead of .Case("").
>>
>>
>> https://github.com/Bigcheese/llvm/blob/master/lib/Object/COFFObjectFile.cpp#L214
>>
>>
>> http://llvm-reviews.chandlerc.com/D222
>> _______________________________________________
>> 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
>

> Index: WriterELF.cpp
> ===================================================================
> --- WriterELF.cpp (revision 170923)
> +++ WriterELF.cpp (working copy)
> @@ -11,20 +11,20 @@
>  #include "ReferenceKinds.h"
>
>  #include "lld/Core/DefinedAtom.h"
> +#include "llvm/Object/ELF.h"

This needs to be sorted. It should come right before the Support headers.

>  #include "lld/Core/File.h"
>  #include "lld/Core/InputFiles.h"
>  #include "lld/Core/Reference.h"
>  #include "lld/Core/SharedLibraryAtom.h"
> -
>  #include "llvm/ADT/ArrayRef.h"
>  #include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/Hashing.h"
>  #include "llvm/ADT/OwningPtr.h"
>  #include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/StringExtras.h"
>  #include "llvm/ADT/StringMap.h"
>  #include "llvm/ADT/StringRef.h"
> -
> -#include "llvm/Object/ELF.h"
> -
> +#include "llvm/ADT/StringSwitch.h"
>  #include "llvm/Support/Allocator.h"
>  #include "llvm/Support/Debug.h"
>  #include "llvm/Support/ELF.h"
> @@ -34,1382 +34,2020 @@
>  #include "llvm/Support/MathExtras.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include "llvm/Support/system_error.h"
> +#include "ELFAtoms.h"
>
>  #include <map>
> +#include <unordered_map>
>  #include <tuple>
>  #include <vector>
> -#include <algorithm>
>
>  using namespace llvm;
>  using namespace llvm::object;
>  namespace lld {
>  namespace elf {
>
> -// The group decides where sections of similar permissions reside
> -// inside a file. This is also used to create program headers. Each group
> -// will be in a new segment.
> +template<support::endianness target_endianness, bool is64Bits>
> +class ELFExecutableWriter;
>
> -/// \name Chunk Groups
> -/// Each "Chunk" should be arranged in a weak strict order
> -/// This is done to minimize memory utilization when creating program segments.
> -/// This will remain in place till a more felxible mechanism of moving chunks
> -/// around and packing for memory efficiency is put in place.
> -/// The Chunks are arranged by group numbers. Every switch from group A to B
> -/// triggers a formation of new segment. Additionally segments are also made
> -/// within a group if there are vast regions (multiple pages) of 0 for
> -/// alignment constraints.
> -enum {
> -/// @{
> -///Invalid group (default on creation)
> -  CG_INVALID = 0x0,
> -///This is a "header" kind of chunk that goes at beginning of ELF file
> -  CG_HEADER = 0x1,
> -///This is a section chunk with read write and execute permissions
> -  CG_RWX = 0x3,
> -///This is a section chunk with read and execute permissions
> -  CG_RX = 0x4,
> -///This is a section chunk with read permission
> -  CG_R = 0x5,
> -///This is a section chunk with read and write permissions
> -  CG_RW = 0x6,
> -///This is a section chunk with write and execute permissions
> -  CG_WX = 0x7,
> -///This is a section chunk with write permission
> -  CG_W = 0x8,
> -///This is a section chunk with execute permission
> -  CG_X = 0x9,
> -///This is a section which is no loaded by program loader
> -  CG_NO_LOAD = 0xFE,
> -///This is ELF file metadata that goes at end of file
> -  CG_FILE_END = 0xFF
> -/// @}
> -};
> +/// \brief The ELFWriter class is a base class for the linker to write
> +///        various kinds of ELF files.
> +class ELFWriter : public Writer {
> +  public:

This shouldn't be indented.

> +    ELFWriter() { }
>
> -template<support::endianness target_endianness, bool is64Bits>
> -class ELFWriter;
> +  public:
> +    /// \brief builds the chunks that needs to be written to the output
> +    ///        ELF file
> +    virtual void buildChunks(const lld::File &file) = 0;
>
> -template<support::endianness target_endianness, bool is64Bits>
> -class StockSectionChunk;
> +    /// \brief Writes the chunks into the output file specified by path
> +    virtual error_code writeFile(const lld::File &File, StringRef path) = 0;
>
> -/// \brief A Chunk is a contiguous range of space.
> +    /// \brief Writes the chunks into the output file specified by path
> +    virtual int64_t addressOfAtom(const Atom *atom) = 0;
> +
> +    /// \brief Return the processing function to apply Relocations
> +    virtual KindHandler *kindHandler()  = 0;
> +};
> +
> +/// \brief A chunk is a contiguous region of space
>  template<support::endianness target_endianness, bool is64Bits>
>  class Chunk {
>  public:
> -  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +
> +  /// \brief Describes the type of Chunk
> +  enum Kind {
> +    K_ELFHeader, // ELF Header
> +    K_ELFProgramHeader, // Program Header
> +    K_ELFSegment, // Segment
> +    K_ELFSection, // Section
> +    K_ELFSectionHeader // Section header
> +  };
> +  Chunk(StringRef name, Kind kind)
> +    : _name(name)
> +    , _kind(kind)
> +    , _fsize(0)
> +    , _msize(0)
> +    , _align2(0)
> +    , _order(0)
> +    , _ordinal(1)
> +    , _start(0)
> +    , _fileoffset(0) {}
>    virtual             ~Chunk() {}
> -  virtual StringRef   segmentName() const = 0;
> -  virtual bool        occupiesNoDiskSpace();
> -  virtual void        write(uint8_t *fileBuffer) = 0;
> -  void                assignFileOffset(uint64_t &curOff, uint64_t &curAddr);
> -  virtual const char *info() = 0;
> -  uint64_t            size() const;
> -  uint64_t            address() const;
> -  uint64_t            fileOffset() const;
> -  uint64_t            align2() const;
> -  static uint64_t     alignTo(uint64_t value, uint8_t align2);
> -  uint64_t            ordinal() const { return _ordinal;}
> -  void                setOrdinal(uint64_t newVal) { _ordinal = newVal;}
> -  void                setGroup(uint16_t val) { _group = val;}
> -  uint16_t            group() const { return _group;}
> -  void                init();
> -  bool                isLoadable() { return _isLoadable; }
> -  void                isLoadable(uint64_t val) {  _isLoadable = val; }
> -  enum class Kind {
> -    Header,      // This is a header chunk
> -    Section      // chunk represents a section
> -  };
> -  Kind getChunkKind() const { return _cKind; }
> -private:
> -  const Kind _cKind;
> +  // Does the chunk occupy disk space
> +  virtual bool        occupiesNoDiskSpace() const {
> +    return false;
> +  }
> +  // The name of the chunk
> +  StringRef name() const { return _name; }
> +  // Kind of chunk
> +  Kind kind() const { return _kind; }
> +  int64_t            fileSize() const { return _fsize; }
> +  int64_t            align2() const { return _align2; }
> +  void                appendAtom() const;
> +
> +  // The ordinal value of the chunk
> +  int64_t            ordinal() const { return _ordinal;}
> +  void               setOrdinal(int64_t newVal) { _ordinal = newVal;}
> +  // The order in which the chunk would appear in the output file
> +  int64_t            order() const { return _order; }
> +  void               setOrder(int32_t order) { _order = order; }
> +  // Output file offset of the chunk
> +  int64_t            fileOffset() const { return _fileoffset; }
> +  void               setFileOffset(int64_t offset) { _fileoffset = offset; }
> +  // Output start address of the chunk
> +  void               setVAddr(int64_t start) { _start = start; }
> +  int64_t            virtualAddr() const { return _start; }
> +  // Does the chunk occupy memory during execution ?
> +  int64_t            memSize() const { return _msize; }
> +  void               setMemSize(int64_t msize) { _msize = msize; }
> +  // Writer the chunk
> +  virtual void       write(ELFWriter *writer,
> +                           OwningPtr<FileOutputBuffer> &buffer) = 0;
> +  // Finalize the chunk before writing
> +  virtual void       finalize() = 0;
> +
>  protected:
> -  Chunk();
> -  Chunk(Kind K): _cKind(K) { this->init();}
> -  uint64_t _size;
> -  uint64_t _address;
> -  uint64_t _fileOffset;
> -  uint64_t _align2;
> -  uint64_t _ordinal;
> -  uint16_t _group;
> -  bool     _isLoadable;
> +  StringRef _name;
> +  Kind _kind;
> +  int64_t _fsize;
> +  int64_t _msize;
> +  int64_t _align2;
> +  int32_t  _order;
> +  int64_t _ordinal;
> +  int64_t _start;
> +  int64_t _fileoffset;

These should all be unsigned.

>  };
>
> -template<support::endianness target_endianness, bool is64Bits>
> -static void swapChunkPositions(Chunk<target_endianness, is64Bits>&a,
> -                               Chunk<target_endianness, is64Bits>&b) {
> -  uint64_t tempOrdinal;
> -  if (a.ordinal() == b.ordinal()) return;
> -  tempOrdinal = a.ordinal();
> -  a.setOrdinal(b.ordinal());
> -  b.setOrdinal(tempOrdinal);
> -}
> +/// \brief The ELFLayoutOptions encapsulates the options used by all Layouts
> +///        Examples of the ELFLayoutOptions would be a script that would be used
> +///        to drive the layout
> +class ELFLayoutOptions {
> +public:
> +  ELFLayoutOptions() { }
>
> -template<support::endianness target_endianness, bool is64Bits>
> -bool chunkGroupSort(Chunk<target_endianness, is64Bits> *A,
> -                    Chunk<target_endianness, is64Bits> *B) {
> -  if (A->group() == CG_INVALID || B->group() == CG_INVALID)
> -    llvm_unreachable("Invalid group number");
> -  return A->group() < B->group();
> -}
> +  ELFLayoutOptions(StringRef &linker_script) : _script(linker_script)
> +  {}
>
> +  /// parse the linker script
> +  error_code parseLinkerScript();
> +
> +  /// Is the current section present in the linker script
> +  bool isSectionPresent();
> +
> +private:
> +  StringRef _script;
> +};
> +
> +/// \brief The ELFLayout is an abstract class for managing the final layout for
> +///        the kind of binaries(Shared Libraries / Relocatables / Executables 0
> +///        Each architecture (Hexagon, PowerPC, MIPS) would have a concrete
> +///        subclass derived from ELFLayout for generating each binary thats
> +//         needed by the lld linker
> +class ELFLayout {
> +public:
> +  typedef int32_t SectionOrder;
> +  typedef int32_t SegmentType;
> +  typedef int32_t Flags;

Unsigned.

> +
> +public:
> +  /// Return the order the section would appear in the output file
> +  virtual SectionOrder getSectionOrder
> +                        (const StringRef name,
> +                         int32_t contentType,
> +                         int32_t contentPerm) = 0;
> +  /// append the Atom to the layout and create appropriate sections
> +  virtual error_code addAtom(const Atom *atom) = 0;
> +  /// find the Atom Address in the current layout
> +  virtual bool findAtomAddrByName(const StringRef name, int64_t &addr) = 0;
> +  /// associates a section to a segment
> +  virtual void assignSectionsToSegments() = 0;
> +  /// associates a virtual address to the segment, section, and the atom
> +  virtual void assignVirtualAddress() = 0;
> +  /// associates a file offset to the segment, section and the atom
> +  virtual void assignFileOffsets() = 0;
> +
> +public:
> +  ELFLayout() {}
> +  ELFLayout(WriterOptionsELF &writerOptions,
> +            ELFLayoutOptions &layoutOptions)
> +    : _writerOptions(writerOptions)
> +    , _layoutOptions(layoutOptions) {}
> +  virtual ~ELFLayout() { }
> +
> +private:
> +  WriterOptionsELF _writerOptions;
> +  ELFLayoutOptions _layoutOptions;
> +};
> +
> +
> +/// \brief A section contains a set of atoms that have similiar properties
> +///        The atoms that have similiar properties are merged to form a section
>  template<support::endianness target_endianness, bool is64Bits>
> -struct ChunkComparator {
> -  bool operator()(uint16_t A, Chunk<target_endianness, is64Bits> *B) {
> -    return A < B->group();
> +class Section : public Chunk<target_endianness, is64Bits> {
> +public:
> +  // The Kind of section that the object represents
> +  enum Kind {
> +    K_Default = 0x100,
> +    K_SymbolTable,
> +    K_StringTable,
> +  };

I see now that this isn't the using the same kind field as Chunk is.
So no need for the 0x100. Although it does hide Chunk::Kind, so it
needs a different name.

> +  // Create a section object, the section is set to the default type if the
> +  // caller doesnot set it
> +  Section(const StringRef sectionName,
> +          const int32_t contentType,
> +          const int32_t contentPermissions,
> +          const int32_t order,
> +          const Kind kind = K_Default)
> +    : Chunk<target_endianness, is64Bits>
> +      (sectionName, Chunk<target_endianness, is64Bits>::K_ELFSection)
> +    , _contentType(contentType)
> +    , _contentPermissions(contentPermissions)
> +    , _sectionKind(kind)
> +    , _entSize(0)
> +    , _shInfo(0)
> +    , _link(0) {
> +    this->setOrder(order);
>    }
> -  bool operator()(Chunk<target_endianness, is64Bits> *A, uint16_t B) {
> -    return A->group() < B;
> +
> +  /// return the section kind
> +  Kind sectionKind() const {
> +    return _sectionKind;
>    }
> -#if defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL == 2
> - bool operator()(Chunk<target_endianness, is64Bits> *A,
> -        Chunk<target_endianness, is64Bits> *B) {
> -   return A->group() < B->group();
> - }
> -#endif
> -};
>
> -/// Pair of atom and offset in section.
> -typedef std::tuple<const DefinedAtom*, uint64_t> AtomInfo;
> +  /// Align the offset to the required modulus defined by the atom alignment
> +  uint64_t alignOffset(uint64_t offset, DefinedAtom::Alignment &atomAlign) {
> +    uint64_t requiredModulus = atomAlign.modulus;
> +    int64_t align2 = 1 << atomAlign.powerOf2;

Unsigned. Won't warn if you do 1u <<.

> +    uint64_t currentModulus = (offset % align2);
> +    uint64_t retOffset = offset;
> +    if (currentModulus != requiredModulus) {
> +      if (requiredModulus > currentModulus)
> +        retOffset += requiredModulus - currentModulus;
> +      else
> +        retOffset += align2 + requiredModulus - currentModulus;
> +    }
> +    return retOffset;
> +  }
>

> +
> +  void finalize() {
> +    // sh_info should be one greater than last symbol with STB_LOCAL binding
> +    // we sort the symbol table to keep all local symbols at the beginning
> +    std::stable_sort(_symbolTable.begin(), _symbolTable.end(),
> +                    [](const Elf_Sym *A, const Elf_Sym *B) {
> +                     return A->getBinding() < B->getBinding();
> +                     });

This needs to be reformatted.

> +    uint16_t shInfo = 0;
> +    for (auto i : _symbolTable) {
> +      if (i->getBinding() != ELF::STB_LOCAL)
> +        break;
> +      shInfo++;
> +    }
> +    this->_shInfo = shInfo;
> +    this->setLink(_stringSection->ordinal());
> +  }
> +

> +  // HashKey for the Segment
> +  class SegmentHashKey {
> +  public:
> +    int64_t operator() (const SegmentKey &k) const {
> +      // k.first = SegmentName
> +      // k.second = SegmentFlags
> +      return llvm::hash_combine(k.first, k.second);
>      }
> -    symbol->st_value = aa->value();
> - } else {
> -   symbol->st_value = 0;
> -   t = ELF::STT_NOTYPE;
> -   b = ELF::STB_LOCAL;
> - }
> - symbol->setBindingAndType(b, t);
> +  };
>
> - _symbolTable.push_back(symbol);
> - this->_size += sizeof(Elf_Sym);
> -}
> +  typedef std::unordered_map<Key,
> +          Section<target_endianness, is64Bits>*, HashKey> SectionMapT;
> +  typedef std::unordered_map<SegmentKey,
> +                             Segment<target_endianness, is64Bits>*,
> +                             SegmentHashKey> SegmentMapT;
>

- Michael Spencer



More information about the llvm-commits mailing list