[llvm-commits] LLD: patch WriterELF patch

Hemant Kulkarni khemant at codeaurora.org
Mon Sep 10 11:24:47 PDT 2012


I have changed the loops to range based loops, taken care of spaces and comma, removed #if 0 .. #endif, made sure to use BumpPtrAllocate for fixing leaks and removed the enums and directly use Support/ELF.h file enums.

I have also removed Elf_Ehdr from ELFObjectFile class (Object/ELF.h) to make it consistent with other classes. I have changed the code that dependent on this change. Please take a look and comment on the changes.

Thanks.

--
Hemant Kulkarni
khemant at codeaurora.org
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation


-----Original Message-----
From: Michael Spencer [mailto:bigcheesegs at gmail.com] 
Sent: Wednesday, September 05, 2012 2:37 PM
To: Hemant Kulkarni
Cc: llvm-commits at cs.uiuc.edu; kledzik at apple.com; Clow, Marshall
Subject: Re: LLD: patch WriterELF patch

On Tue, Sep 4, 2012 at 12:30 PM, Hemant Kulkarni <khemant at codeaurora.org> wrote:
> This patch adds ELF Writer basic infrastructure. It is based off of Mach-O
> writer in lld tree. Some of the comments are a little verbose, they might
> become irrelevant in future when they will be removed.
>
> ELFHeaderChunk is the class that sets the correct fields in Elf_Hdr
> structure.
> SectionChunk class is container for each section that will be in writer
> output
> ELFStringSectionChunk implements the string section as per SCO SYS V ABI
> ELFSectionHeaderChunk is container to work with SectionChunk and other
> custom sections made by linker to create a table of Elf_Shdr structures.
>
> Issues:
> Symbol tables are not yet implemented; I will put in a new patch for symbol
> table update. This might also involve modifying ReaderELF and deal with
> TODO/FIXME in it.
> Relocation fixups are not implemented yet.
> The test infrastructure for LLD will need to be modified to read binary
> files and test writer. I will add them so we can test Writer in make check
> like test. For now, I run a modified lld-core utility that has options to
> choose writer and I emit ELF binary. I then examine the file with readelf.
>
>
>
> --
> Hemant Kulkarni
> khemant at codeaurora.org
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> the Linux Foundation
>
>

> Index: include/lld/ReaderWriter/WriterELF.h
> ===================================================================
> --- include/lld/ReaderWriter/WriterELF.h	(revision 163142)
> +++ include/lld/ReaderWriter/WriterELF.h	(working copy)
> @@ -13,13 +13,15 @@
>  #include "lld/ReaderWriter/Writer.h"
>  #include "lld/Core/LLVM.h"
>  #include "llvm/ADT/StringRef.h"
> +#include "llvm/Support/Endian.h"
>
> +#include <utility>
>
>  namespace lld {
>
>  ///
>  /// The WriterOptionsELF class encapsulates options needed
> -/// to process mach-o files.  You can create an WriterOptionsELF
> +/// to process ELF files.  You can create an WriterOptionsELF
>  /// instance from command line arguments or by subclassing and setting the
>  /// instance variables in the subclass's constructor.
>  ///
> @@ -28,7 +30,7 @@
>    virtual ~WriterOptionsELF();
>
>    ///
> -  /// Creates a Options object from darwin linker command line arguments.
> +  /// Creates a Options object from elf linker command line arguments.
>    /// FIXME: to be replaced with new option processing mechanism.
>    ///
>    WriterOptionsELF(int argc, const char* argv[]);
> @@ -39,7 +41,52 @@
>    ///
>    WriterOptionsELF();
>
> +  enum OutputKind {
> +    outputDynamicExecutable,  // ET_EXEC
> +    outputStaticExecutable,   // ET_EXEC
> +    outputObjectFile,         // ET_REL
> +    outputDylib,              // ET_DYN

ELF has the following types:

ET_REL 1 Relocatable object file
ET_EXEC 2 Executable file
ET_DYN 3 Shared object file
ET_CORE 4 Core file

Which should be in llvm/Support/ELF.h

> +  };
> +
> +  enum Architecture {
> +    arch_x86_64,
> +    arch_x86,
> +    arch_armv6,
> +    arch_armv7,
> +    arch_hexagon,
> +  };

Use values from llvm/Support/ELF.h

> +  ///
> +  /// Create a specific instance of an architecture
> +  ///
> +  WriterOptionsELF(const Architecture arch,
> +                   const llvm::support::endianness endian,
> +                   const bool is64Bit);
> +  WriterOptionsELF(const OutputKind OK, const Architecture ARCH,
> +                   const bool REL,
> +                   const llvm::support::endianness endian,
> +                   const bool is64Bit);

These constructors should be in terms of e_ident (broken up), e_type, and
e_machine.

> +
> +  OutputKind   outputKind() const       { return _outputkind; }
> +  Architecture architecture() const     { return _architecture; }
> +  bool         is64Bit() const          { return _is64Bit; }
> +  llvm::support::endianness endianness() const { return _endian; }
> +  StringRef    archName() const;
> +  uint32_t     cpuType() const;
> +  uint32_t     cpuSubtype() const;
> +  bool         noTextRelocations() const { return _noTextRelocations; }
> +  bool         addEntryPointLoadCommand() const;
> +  bool         addUnixThreadLoadCommand() const;

These two make no sense for ELF.

> +  StringRef    entryPointName() const;
> +  std::pair<llvm::support::endianness, bool>    getArchTypeClass() const;
> +
>  protected:
> +  OutputKind                _outputkind;
> +  Architecture              _architecture;
> +  bool                      _noTextRelocations;
> +  llvm::support::endianness _endian;
> +  bool                      _is64Bit;
> +  StringRef                 _customEntryPointName;
> +
>  };
>
>
> Index: lib/ReaderWriter/ELF/WriterOptionsELF.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/WriterOptionsELF.cpp	(revision 0)
> +++ lib/ReaderWriter/ELF/WriterOptionsELF.cpp	(revision 0)
> @@ -0,0 +1,164 @@
> +//===- lib/ReaderWriter/ELF/WriterOptionsELF.cpp ----------------------===//
> +//
> +//                             The LLVM Linker
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "lld/ReaderWriter/WriterELF.h"
> +
> +#include "llvm/Support/Debug.h"
> +#include "llvm/Support/ELF.h"
> +#include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Support/system_error.h"
> +
> +#include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/StringRef.h"
> +
> +// TODO: MachOFormat.hpp is a MachO variant of what is include/Support/ELF.h
> +//       Will try for the time being to use ELF.h
> +// #include "MachOFormat.hpp"

Remove.

> +
> +namespace lld {
> +
> +WriterOptionsELF::WriterOptionsELF()
> +  : _outputkind(outputDynamicExecutable)
> +  , _architecture(arch_x86)
> +  , _noTextRelocations(true)
> +  , _endian (llvm::support::little)
> +  , _is64Bit(false){
> +}
> +
> +// This interface was added to allow a user to select a particular
> +// output architecture.
> +// FIXME: We need to add probably more parameters such as PageZero address/size?
> +WriterOptionsELF::WriterOptionsELF(const WriterOptionsELF::Architecture ARCH,
> +                                   const llvm::support::endianness endian,
> +                                   const bool is64Bit)
> +  : _outputkind(outputStaticExecutable)
> +  , _architecture(ARCH)
> +  , _noTextRelocations(true)
> +  , _endian (endian)
> +  , _is64Bit(is64Bit){
> +}
> +
> +WriterOptionsELF::WriterOptionsELF(const WriterOptionsELF::OutputKind OK,
> +                                   const WriterOptionsELF::Architecture ARCH,
> +                                   const bool REL,
> +                                   const llvm::support::endianness endian,
> +                                   const bool is64Bit)
> +  : _outputkind(OK)
> +  , _architecture(ARCH)
> +  , _noTextRelocations(REL)
> +  , _endian (endian)
> +  , _is64Bit(is64Bit){
> +}
> +
> +WriterOptionsELF::~WriterOptionsELF() {
> +}
> +
> +StringRef WriterOptionsELF::archName() const {
> +  switch ( _architecture ) {
> +    case arch_x86_64:
> +      return StringRef("x86_64");
> +    case arch_x86:
> +       return StringRef("i386");
> +    case arch_armv6:
> +       return StringRef("armv6");
> +    case arch_armv7:
> +       return StringRef("armv7");
> +    case arch_hexagon:
> +       return StringRef("hexagon");
> +  }
> +  llvm_unreachable("unknown arch");
> +}
> +
> +uint32_t WriterOptionsELF::cpuType() const {
> +  switch ( _architecture ) {
> +    case arch_x86_64:
> +      return llvm::ELF::EM_X86_64;
> +    case arch_x86:
> +      return llvm::ELF::EM_386;
> +    case arch_armv6:
> +    case arch_armv7:
> +      return llvm::ELF::EM_ARM;
> +    case arch_hexagon:
> +      return llvm::ELF::EM_HEXAGON;
> +  }
> +  llvm_unreachable("unknown arch");
> +}
> +
> +uint32_t WriterOptionsELF::cpuSubtype() const {
> +  switch ( _architecture ) {
> +    case arch_x86_64:
> +      return llvm::ELF::EM_X86_64;
> +    case arch_x86:
> +      return llvm::ELF::EM_386;
> +    case arch_armv6:
> +    case arch_armv7:
> +      return llvm::ELF::EM_ARM;
> +//TODO sub-type will probably mean hexagon arch version/v[26], this
> +// will have to be locally enumerated.

What do you mean by enumerated?

> +    case arch_hexagon:
> +       return llvm::ELF::EM_HEXAGON;
> +  }
> +  llvm_unreachable("unknown arch");
> +}
> +
> +bool WriterOptionsELF::addEntryPointLoadCommand() const {
> +  switch ( _outputkind ) {
> +    case outputDynamicExecutable:
> +    case outputStaticExecutable:
> +      // Only main executables have an entry point
> +      return true;
> +    case outputDylib:
> +    case outputObjectFile:
> +      return false;
> +  }
> +  llvm_unreachable("unknown outputkind");
> +}
> +
> +bool WriterOptionsELF::addUnixThreadLoadCommand() const {
> +  switch ( _outputkind ) {
> +    case outputDynamicExecutable:
> +    case outputStaticExecutable:
> +      // Only main executables have an entry point
> +      return true;
> +    case outputDylib:
> +    case outputObjectFile:
> +      return false;
> +  }
> +  llvm_unreachable("unknown outputkind");
> +}
> +
> +StringRef WriterOptionsELF::entryPointName() const {
> +  switch ( _outputkind ) {
> +    case outputDynamicExecutable:
> +    case outputStaticExecutable:
> +      // Only main executables have an entry point
> +      if ( ! _customEntryPointName.empty() ) {
> +        return _customEntryPointName;
> +      }
> +      else {
> +          return StringRef("start");
> +      }
> +      break;
> +    case outputDylib:
> +    case outputObjectFile:
> +      return StringRef();
> +  }
> +  llvm_unreachable("unknown outputkind");
> +}
> +
> +std::pair<llvm::support::endianness, bool>
> +    WriterOptionsELF::getArchTypeClass() const {
> +  return (std::pair<llvm::support::endianness, bool>)
> +		  std::make_pair(_endian,_is64Bit);
> +}
> +
> +
> +
> +} // namespace lld
> +
> Index: lib/ReaderWriter/ELF/WriterELF.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/WriterELF.cpp	(revision 163142)
> +++ lib/ReaderWriter/ELF/WriterELF.cpp	(working copy)
> @@ -9,27 +9,1143 @@
>
>  #include "lld/ReaderWriter/WriterELF.h"
>
> +#include "llvm/Object/ELF.h"
> +
>  #include "llvm/Support/Debug.h"
> +#include "llvm/Support/ELF.h"
> +#include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Support/FileOutputBuffer.h"
> +#include "llvm/Support/Format.h"
> +#include "llvm/Support/raw_ostream.h"
> +#include "llvm/Support/system_error.h"
> +#include "llvm/Support/MathExtras.h"
>
> +#include "llvm/ADT/ArrayRef.h"
> +#include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/OwningPtr.h"
> +#include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/StringMap.h"
> +#include "llvm/ADT/StringRef.h"
>
> +#include "lld/Core/DefinedAtom.h"
> +#include "lld/Core/File.h"
> +#include "lld/Core/InputFiles.h"
> +#include "lld/Core/Reference.h"
> +#include "lld/Core/SharedLibraryAtom.h"
> +
> +#include <vector>
> +#include <map>
> +#include <string.h>
> +
> +#include "ReferenceKinds.h"
> +
> +using namespace llvm;
> +using namespace llvm::object;
>  namespace lld {
>  namespace elf {
>
> -// define ELF writer class here
>
> +template <support::endianness target_endianness, bool is64Bits>
> +class ELFWriter;
>
> -} // namespace elf
> +template  <support::endianness target_endianness, bool is64Bits>
> +struct Elf_Ehdr {
> +    LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +    unsigned char e_ident[ELF::EI_NIDENT]; // ELF Identification bytes
> +    Elf_Half e_type;     // Type of file (see ET_*)
> +    Elf_Half e_machine;  // Required architecture for this file (see EM_*)
> +    Elf_Word e_version;  // Must be equal to 1
> +    Elf_Addr e_entry;    // Address to jump to in order to start program
> +    Elf_Off  e_phoff;    // Program header table's file offset, in bytes
> +    Elf_Off  e_shoff;    // Section header table's file offset, in bytes
> +    Elf_Word e_flags;    // Processor-specific flags
> +    Elf_Half e_ehsize;   // Size of ELF header, in bytes
> +    Elf_Half e_phentsize;// Size of an entry in the program header table
> +    Elf_Half e_phnum;    // Number of entries in the program header table
> +    Elf_Half e_shentsize;// Size of an entry in the section header table
> +    Elf_Half e_shnum;    // Number of entries in the section header table
> +    Elf_Half e_shstrndx; // Section header table index of section name
> +                                  // string table
> +    bool checkMagic() const {
> +      return (memcmp(e_ident, ELF::ElfMagic, strlen(ELF::ElfMagic))) == 0;
> +    }
> +    unsigned char getFileClass() const { return e_ident[ELF::EI_CLASS]; }
> +    unsigned char getDataEncoding() const { return e_ident[ELF::EI_DATA]; }
> +  };

Why is this being coppied from llvm/Object/ELF.h?

>
> -Writer* createWriterELF(const WriterOptionsELF &options) {
> -  assert(0 && "ELF support not implemented yet");
> -  return nullptr;
> +//
> +// A Chunk is a contiguous range of space.  This idea is copied from the,
> +// "Chunk" used by Mach-O
> +//
> +template <support::endianness target_endianness, bool is64Bits>
> +class Chunk {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +  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);
> +
> +protected:
> +                      Chunk();
> +
> +  uint64_t             _size;
> +  uint64_t            _address;
> +  uint64_t             _fileOffset;
> +  uint64_t            _align2;
> +};
> +
> +
> +
> +// This could functionally be inside the SectionChunk, but c++ standard does not
> +// allow the vector of local types.

C++11 does.

> +
> +template <support::endianness target_endianness, bool is64Bits>
> +  struct AtomInfo {
> +    LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +    const DefinedAtom  *atom;
> +    uint64_t            offsetInSection;
> +  };
> +//
> +// A Section represents a set of Atoms assigned to a specific
> +// ELF Section
> +//
> +template <support::endianness target_endianness, bool is64Bits>
> +class SectionChunk : public Chunk <target_endianness, is64Bits> {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)

LLVM_ELF_IMPORT_TYPES isn't needed here.

> +  SectionChunk  (DefinedAtom::ContentType,
> +                StringRef sectionName,
> +                const WriterOptionsELF &options,
> +                class ELFWriter<target_endianness, is64Bits> &writer);
> +
> +  virtual StringRef          segmentName() const;
> +  virtual bool               occupiesNoDiskSpace();
> +  virtual void               write(uint8_t *fileBuffer);
> +  virtual const char*        info();
> +  StringRef                  sectionName();
> +  uint32_t                   flags() const;
> +  uint32_t                   type() const;
> +  uint32_t                   permissions();
> +  void                       appendAtom(const DefinedAtom*);
> +
> +
> +
> +  const std::vector<AtomInfo<target_endianness,is64Bits> >& atoms() const;
> +
> +private:
> +  StringRef                 _segmentName;
> +  StringRef                 _sectionName;
> +  const WriterOptionsELF    &_options;
> +  class ELFWriter<target_endianness, is64Bits>           &_writer;
> +  uint32_t                  _flags;
> +  uint32_t                  _type;
> +  uint32_t                  _permissions;
> +
> +  std::vector<AtomInfo<target_endianness,is64Bits> >
> +                            _atoms;
> +};
> +
> +//
> +// An ELFHeaderChunk represents the Elf[32/64]_Ehdr structure at the start
> +// of an ELF executable file.
> +//
> +template<support::endianness target_endianness, bool is64Bits>
> +class ELFHeaderChunk : public Chunk <target_endianness, is64Bits> {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)

LLVM_ELF_IMPORT_TYPES isn't needed here.

> +
> +  ELFHeaderChunk(const WriterOptionsELF &options,
> +                 const File &file);
> +
> +  void e_ident(int i, unsigned char c)      { _eh.e_ident[i] = c; }
> +  void e_type(uint16_t type)                { _eh.e_type = type; }
> +  void e_machine(uint16_t machine)          { _eh.e_machine = machine; }
> +  void e_version(uint32_t version)          { _eh.e_version = version; }
> +  void e_entry(uint64_t entry)              { _eh.e_entry = entry; }
> +  void e_phoff(uint64_t phoff)               { _eh.e_phoff = phoff; }
> +  void e_shoff(uint64_t shoff)               { _eh.e_shoff = shoff; }
> +  void e_flags(uint32_t flags)              { _eh.e_flags = flags; }
> +  void e_ehsize(uint16_t ehsize)            { _eh.e_ehsize = ehsize; }
> +  void e_phentsize(uint16_t phentsize)      { _eh.e_phentsize = phentsize; }
> +  void e_phnum(uint16_t phnum)              { _eh.e_phnum = phnum; }
> +  void e_shentsize(uint16_t shentsize)      { _eh.e_shentsize = shentsize; }
> +  void e_shnum(uint16_t shnum)              { _eh.e_shnum = shnum; }
> +  void e_shstrndx(uint16_t shstrndx)        { _eh.e_shstrndx = shstrndx; }
> +
> +  uint64_t  size() { return sizeof (Elf_Ehdr<target_endianness, is64Bits>); }
> +
> +  virtual StringRef     segmentName() const;
> +  virtual void          write(uint8_t *fileBuffer);
> +  virtual const char*   info();
> +
> +private:
> +  uint32_t              filetype(WriterOptionsELF::OutputKind kind);
> +  Elf_Ehdr<target_endianness, is64Bits>             _eh;
> +};
> +
> +//
> +// An ELFSectionHeaderChunk represents the Elf[32/64]_Shdr structure that
> +// is placed right after the ELFHeader.
> +// When this is finished it will need to update the header with the size
> +// and number of section headers, e_shentsize, e_shnum
> +//
> +template <support::endianness target_endianness, bool is64Bits>
> +class ELFSectionHeaderChunk : public Chunk <target_endianness, is64Bits> {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +  typedef object::Elf_Shdr_Impl<target_endianness, is64Bits> Elf_Shdr;
> +  ELFSectionHeaderChunk(const WriterOptionsELF &options,
> +                        class ELFWriter<target_endianness, is64Bits>&);
> +
> +  virtual StringRef     segmentName() const;
> +  virtual void          write(uint8_t *fileBuffer);
> +  virtual const char*   info();
> +  void                  computeSize(const lld::File &file);
> +  uint16_t              count();
> +  uint16_t              size();
> +
> +  const std::vector<Elf_Shdr*> sectionInfo(){
> +    return _sectionInfo;
> +  }
> +
> +//  void                  append(Elf_Shdr *shdr);
> +  bool                  is64Bit() { return _options.is64Bit(); }
> +
> +private:
> +  const WriterOptionsELF   &_options;
> +  class ELFWriter<target_endianness, is64Bits>          &_writer;
> +
> +  std::vector<Elf_Shdr*> _sectionInfo;

Who owns the memory pointed to?

> +};
> +
> +
> +// ELFStringSectionChunk represents the shstr section
> +// This is a contiguous memory that has all the symbol strings
> +// each ending with null character. We might need more than one such chunks
> +// shstrtab for setting e_shstrndx in ELHHeaderChunk and strtab for use with
> +// symtab
> +//
> +template <support::endianness target_endianness, bool is64Bits>
> +class ELFStringSectionChunk : public Chunk <target_endianness, is64Bits> {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +  ELFStringSectionChunk(const WriterOptionsELF &options,
> +                        class ELFWriter<target_endianness, is64Bits> &writer,
> +                        StringRef SecName);
> +  uint64_t addString(StringRef SymName);
> +
> +
> +  virtual StringRef          segmentName() const;
> +  virtual void               write(uint8_t *fileBuffer);
> +  virtual const char*        info();
> +  StringRef                  sectionName();
> +
> +
> +
> +private:
> +  StringRef _segName;
> +  std::vector<StringRef> _StringSection;
> +  StringRef _sectionName;
> +  class ELFWriter<target_endianness, is64Bits> &_writer;
> +  const WriterOptionsELF &_options;
> +
> +};
> +
> +#if 0
> +//
> +// ELFSymbolTableChunk represents the Symbol table as per ELF ABI
> +// This is a table with Elf[32/64]_Sym entries in it.
> +//
> +template <support::endianness target_endianness, bool is64Bits>
> +class ELFSymbolTableChunk : public Chunk<target_endianness, i64Bits> {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +  ELFSymbolTableChunk(class WriterOptionsELF &options,
> +                      class ELFWriter<target_endianness, is64Bits> &writer,
> +                      StringRef SecName,
> +                      class ELFStringSectionChunk<target_endianness, is64Bits>
> +                      &ShStr);
> +  void addSymbol(const Atom *a);
> +
> +
> +  virtual StringRef          segmentName() const;
> +  virtual void               write(uint8_t *fileBuffer);
> +  virtual const char*        info();
> +  StringRef                  sectionName();
> +
> +
> +
> +private:
> +  StringRef _segName;
> +  std::vector<Elf_Sym*> _SymbolTable;
> +  StringRef _sectionName;
> +  class ELFWriter<target_endianness, is64Bits> &_writer;
> +  class WriterOptionsELF &_options;
> +  class ELFStringSectionChunk<target_endianness, is64Bits> &_stringSection;
> +  uint64_t _offsetInStringTable;
> +};
> +
> +#endif
> +//
> +// An ELFProgramHeaderChunk represents the Elf[32/64]_Phdr structure near
> +// the start of an ELF executable file. Will need to update ELFHeader's
> +// e_phentsize/e_phnum when done.
> +//
> +template <support::endianness target_endianness, bool is64Bits>
> +class ELFProgramHeaderChunk : public Chunk <target_endianness, is64Bits> {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +  ELFProgramHeaderChunk(ELFHeaderChunk<target_endianness, is64Bits>&,
> +                        const WriterOptionsELF &options,
> +                        const File &file);
> +
> +
> +  virtual StringRef     segmentName() const;
> +  virtual void          write(uint8_t *fileBuffer);
> +  virtual const char*   info();
> +
> +private:
> +  uint32_t              filetype(WriterOptionsELF::OutputKind kind);
> +  Elf_Ehdr<target_endianness, is64Bits>             _ph;
> +};
> +
> +
> +//===----------------------------------------------------------------------===//
> +//  Chunk
> +//===----------------------------------------------------------------------===//
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +Chunk<target_endianness, is64Bits>::Chunk()
> + : _size(0), _address(0), _fileOffset(0), _align2(0) {
>  }
>
> -WriterOptionsELF::WriterOptionsELF() {
> +template <support::endianness target_endianness, bool is64Bits>
> +bool Chunk<target_endianness, is64Bits>::occupiesNoDiskSpace() {
> +  return false;
>  }
>
> -WriterOptionsELF::~WriterOptionsELF() {
> +template <support::endianness target_endianness, bool is64Bits>
> +uint64_t Chunk<target_endianness, is64Bits>::size() const {
> +  return _size;
>  }
>
> +template <support::endianness target_endianness, bool is64Bits>
> +uint64_t Chunk<target_endianness, is64Bits>::align2() const {
> +  return _align2;
> +}
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +uint64_t Chunk<target_endianness, is64Bits>::address() const {
> +  return _address;
> +}
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +uint64_t Chunk<target_endianness, is64Bits>::fileOffset() const {
> +  return _fileOffset;
> +}
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +uint64_t Chunk<target_endianness, is64Bits>::
> +         alignTo(uint64_t value, uint8_t align2) {
> +
> +  uint64_t align = 1 << align2;
> +  return ( (value + (align-1)) & (-align) );
> +}
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +void Chunk<target_endianness, is64Bits>::
> +     assignFileOffset(uint64_t &curOffset, uint64_t &curAddress) {
> +  if ( this->occupiesNoDiskSpace() ) {
> +    // FileOffset does not change, but address space does change.

s/address space/virtual address/

> +    uint64_t alignedAddress = alignTo(curAddress,
> +                            _align2?(uint8_t) llvm::Log2_64(_align2):0);
> +   _address = alignedAddress;
> +   curAddress = alignedAddress + _size;
> +  }
> +  else {
> +    // FileOffset and address both move by _size amount after alignment.
> +    uint64_t alignPadding = alignTo(curAddress, _align2?
> +                            (uint8_t) llvm::Log2_64(_align2):0) - curAddress;
> +    _fileOffset = curOffset + alignPadding;
> +    _address = curAddress + alignPadding;
> +    curOffset = _fileOffset + _size;
> +    curAddress = _address + _size;
> +  }
> +
> +  DEBUG_WITH_TYPE("WriterELF-layout", dbgs()
> +                      << "   fileOffset="
> +                      << format("0x%08X", _fileOffset)
> +                      << " address="
> +                      << format("0x%016X", _address)
> +                      << " info=" << this->info() << "\n");
> +}
> +
> +//===----------------------------------------------------------------------===//
> +//  SectionChunk
> +//===----------------------------------------------------------------------===//
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +SectionChunk<target_endianness,is64Bits>::SectionChunk
> +                                        (DefinedAtom::ContentType type,
> +                                        StringRef sectionName,
> +                                        const WriterOptionsELF &options,
> +                                        ELFWriter<target_endianness, is64Bits>
> +                                        &writer)
> + :  _options(options)
> + , _writer(writer) {
> +  switch ( type ) {
> +
> +    case DefinedAtom::typeCode:
> +                              _segmentName = StringRef("PT_LOAD");
> +                              _sectionName = sectionName;
> +                              _flags       = ELF::SHF_ALLOC|ELF::SHF_EXECINSTR;
> +                              _type        = ELF::SHT_PROGBITS;
> +                              break;
> +
> +    case DefinedAtom::typeData:
> +                              _segmentName = StringRef("PT_LOAD");
> +                              _sectionName = sectionName;
> +                              _flags       = ELF::SHF_ALLOC|ELF::SHF_WRITE;
> +                              _type        = ELF::SHT_PROGBITS;
> +                              break;
> +
> +    case DefinedAtom::typeZeroFill:
> +                              _segmentName = StringRef("PT_LOAD");
> +                              _sectionName = StringRef(".bss");
> +                              _flags       = ELF::SHF_ALLOC|ELF::SHF_WRITE;
> +                              _type        = ELF::SHT_NOBITS;
> +                              break;
> +
> +    default:
> +                              _segmentName = StringRef("PT_NULL");
> +                              _sectionName = StringRef("");
> +                              _flags       = 0;
> +                              _type        = ELF::SHT_NULL;
> +      //assert(0 && "TO DO: add support for more sections");
> +     }
> + }

The whitespace here seems rather messed up.

> +
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +bool SectionChunk<target_endianness,is64Bits>::occupiesNoDiskSpace() {
> +  //return ( (_flags & SECTION_TYPE) == S_ZEROFILL );
> +  return false;
> +}
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +StringRef SectionChunk<target_endianness,is64Bits>::segmentName() const {
> +  return _segmentName;
> +}
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +StringRef SectionChunk<target_endianness,is64Bits>::sectionName() {
> +  return _sectionName;
> +}
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +uint32_t SectionChunk<target_endianness,is64Bits>::flags() const {
> +  return _flags;
> +}
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +uint32_t SectionChunk<target_endianness,is64Bits>::type()
> +      const {
> +  return _type;
> +}
> +template< support::endianness target_endianness, bool is64Bits>
> +uint32_t SectionChunk<target_endianness,is64Bits>::permissions() {
> +  return _permissions;
> +}
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +const char* SectionChunk<target_endianness,is64Bits>::info() {
> +  return _sectionName.data();
> +}
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +const std::vector<AtomInfo<target_endianness,is64Bits> >&
> +//const std::vector<SectionChunk<target_endianness,is64Bits>::AtomInfo>&
> +      SectionChunk<target_endianness,is64Bits>::atoms() const {
> +  return _atoms;
> +}
> +
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +void SectionChunk<target_endianness,is64Bits>::appendAtom(const DefinedAtom
> +                                                          *atom) {
> +  // Figure out offset for atom in this section given alignment constraints.
> +  uint64_t offset = this->_size;
> +  DefinedAtom::Alignment atomAlign = atom->alignment();
> +  uint64_t align2 = 1 << atomAlign.powerOf2;
> +  uint64_t requiredModulus = atomAlign.modulus;
> +  uint64_t currentModulus = (offset % align2);
> +  if ( currentModulus != requiredModulus ) {

no space after or before ().

> +    if ( requiredModulus > currentModulus )
> +      offset += requiredModulus-currentModulus;
> +    else
> +      offset += align2+requiredModulus-currentModulus;

Spaces around -.

> +  }
> +  // Record max alignment of any atom in this section.
> +  if ( align2 > this->_align2 )
> +    this->_align2 = align2;
> +  // Assign atom to this section with this offset.
> +  AtomInfo<target_endianness,is64Bits> ai = {atom, offset};

Space after ,

> +  _atoms.push_back(ai);
> +  // Update section size to include this atom.
> +  this->_size = offset + atom->size();
> +  // Update permissions
> +  DefinedAtom::ContentPermissions perms = atom->permissions();
> +
> +  // TODO: Check content permissions and figure out what to do with .bss
> +  if ( (perms & DefinedAtom::permR__) == DefinedAtom::permR__ )
> +    _permissions |= ELF::SHF_ALLOC;
> +  if ( (perms & DefinedAtom::permRW_) == DefinedAtom::permRW_ )
> +    _permissions |= (ELF::SHF_ALLOC | ELF::SHF_WRITE);
> +  if ( (perms & DefinedAtom::permR_X) == DefinedAtom::permR_X )
> +    _permissions |= (ELF::SHF_ALLOC | ELF::SHF_EXECINSTR);
> +}
> +
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +void SectionChunk<target_endianness,is64Bits>::write(uint8_t *chunkBuffer) {
> +  // Each section's content is just its atoms' content.
> +  for (const AtomInfo<target_endianness,is64Bits> &atomInfo : _atoms ) {

const auto &atomInfo

> +    // Copy raw content of atom to file buffer.
> +    ArrayRef<uint8_t> content = atomInfo.atom->rawContent();
> +    uint64_t contentSize = content.size();
> +    if ( contentSize == 0 )
> +      continue;
> +    uint8_t* atomContent = chunkBuffer + atomInfo.offsetInSection;
> +    ::memcpy(atomContent, content.data(), contentSize);
> +    // Apply fixups to file buffer
> +// XXX_SM : TODO key element here
> +#if 0
> +    for (const Reference *ref : *atomInfo.atom) {
> +      uint32_t offset = ref->offsetInAtom();
> +      uint64_t targetAddress = 0;
> +
> +      if ( ref->target() != nullptr )
> +
> +        targetAddress = _writer.addressOfAtom(ref->target());
> +      uint64_t fixupAddress = _writer.addressOfAtom(atomInfo.atom) + offset;
> +      _writer.kindHandler()->applyFixup(ref->kind(), ref->addend(),
> +                            &atomContent[offset], fixupAddress, targetAddress);
> +    }
> +#endif
> +  }
> +}
> +
> +//===----------------------------------------------------------------------===//
> +//  ELFStringSectionChunk
> +//===----------------------------------------------------------------------===//
> +template <support::endianness target_endianness, bool is64Bits>
> +ELFStringSectionChunk<target_endianness,is64Bits>::ELFStringSectionChunk
> +                      (const WriterOptionsELF &options,
> +                       class ELFWriter<target_endianness, is64Bits> &writer,
> +                       StringRef SecName)
> +  : _segName("PT_NULL")
> +  , _sectionName(SecName)
> +  , _writer(writer)
> +  , _options(options){
> +
> +// First Add a null character. It also occupies 1 byte
> +  StringRef empty("");
> +  _StringSection.push_back(empty);
> +  this->_size = 1;
> +// Assuming we need two string sections: one for sections one for symbols
> +// following code is useless
> +#if 0
> +// Name of the string section is also stored inside the section
> +  _StringSection.push_back(SecName);
> +//take into account the size of this string + null character that succeeds
> +  _size += SecName.size() + 1;
> +#endif
> +
> +  }
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +uint64_t  ELFStringSectionChunk<target_endianness,is64Bits>::addString
> +                                (StringRef SymName) {
> +  uint64_t  offset;
> +  offset = this->_size;
> +  _StringSection.push_back(SymName);
> +  this->_size += SymName.size() + 1;
> +
> +  return offset;
> +}
> +
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +const char* ELFStringSectionChunk<target_endianness,is64Bits>::info () {
> +  return _sectionName.data();
> +}

What does info actually mean?

> +
> +template <support::endianness target_endianness, bool is64Bits>
> +StringRef ELFStringSectionChunk<target_endianness,is64Bits>::sectionName() {
> +  return _sectionName ;
> +}
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +StringRef ELFStringSectionChunk<target_endianness,is64Bits>::segmentName()
> +                                                             const {
> +  return _segName;
> +}
> +
> +// We need to unwrap the _StringSection and then make one large memory
> +// chunk of null terminated strings
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +void ELFStringSectionChunk<target_endianness,is64Bits>::write
> +                          (uint8_t *chunkbuffer) {
> +  uint64_t chunkoffset = 0;
> +  std::vector<StringRef>::iterator it,ie;
> +
> +
> +  for (it=_StringSection.begin(),ie=_StringSection.end(); it!=ie;++it) {
> +    ::memcpy(chunkbuffer+chunkoffset,it->data(),it->size());
> +    chunkoffset += it->size();
> +    ::memcpy(chunkbuffer+chunkoffset,"",1);
> +    chunkoffset += 1;
> +  }
> +}
> +
> +
> +#if 0
> +//===----------------------------------------------------------------------===//
> +//  ELFSymbolTableChunk
> +//===----------------------------------------------------------------------===//
> +template< support::endianness target_endianness, bool is64Bits>
> +ELFSymbolTableChunk::ELFSymbolTableChunk
> +                    (class WriterOptionsELF &options,
> +                     class ELFWriter<target_endianess, is64bits> &writer,
> +                     StringRef SecName,
> +                     class ELFStringSectionChunk<target_endianess, is64bits>
> +                     &Shstr)
> +  : _segName("PT_NULL")
> +  , _sectionName(SecName)
> +  , _writer(writer)
> +  , _options(options)
> +  , _stringSection(Shstr) {
> +  Elf_Sym *symbol = new Elf_Sym;
> +  memset ((void *)symbol,0,sizeof(elf_sym));
> +  _SymbolTable.push_back(symbol);
> +  _offsetInStringTable = _stringSection.addString(SecName)
> +}
> +
> +
> +//We examine each property of atom to infer the various st_* fields in Elf*_Sym
> +
> +template< support::endianness target_endianness, bool is64Bits>
> +ELFSymbolTableChunk<target_endianness,is64Bits>::addSymbol(const Atom *a) {
> + Elf_Sym *symbol = new Elf_Sym;
> + unsigned char b,t;
> + symbol->st_name = _stringSection.addString(a->name());
> +// TODO: Account for common symbols
> + if (a->definition == lld::Atom::definitionRegular) {
> +     symbol->st_value = a->rawContent();
> +     symbol->st_size = a->size();
> +     lld::DefinedAtom::ContentType ct;
> +     switch (ct = a->getType()){
> +        case  DefinedAtom::typeCode:
> +                                         t = ELF::STT_FUNC;
> +                                         break;
> +        case  DefinedAtom::typeData:
> +                                         t = ELF::STT_OBJECT;
> +                                         break;
> +        case  DefinedAtom::typeZeroFill:
> +                                         t = ELF::STT_COMMON;
> +                                         break;
> +        case default:
> +                                         t = ELF::STT_NOTYPE;
> +     }
> +
> +// TODO: Find out how to incorporate STB_HIOS STB_LOOS
> +// STB_HIPROC and STB_LOPROC
> +
> +     if (a->scope() != DefinedAtom::scopeGlobal &&
> +         a->merge() != DefinedAtom::mergeAsWeak)
> +             b = ELF::STB_LOCAL;
> +     else if (a->merge == DefinedAtom::mergeAsWeak)
> +             b= ELF::STB_WEAK;
> +     else
> +             b = ELF::STB_GLOBAL;
> + }
> + else if (a->definition == lld::Atom::definitionAbsolute) {
> +     symbol->st_value = a->value();
> +     symbol->st_size = 0;
> + }
> + else {
> +     symbol->st_value = 0;
> +     symbol->st_size=0;
> +     t = ELF::STT_NOTYPE;
> +     b = ELF::STB_LOCAL;
> + }
> +
> +// This will set the st_info field in Elf_Sym
> +     symbol->setBindingAndType(b,t);
> +}
> +
> +#endif

Can you just remove all the #if 0 stuff for now?

> +
> +
> +
> +//===----------------------------------------------------------------------===//
> +//  ELFHeaderChunk
> +//===----------------------------------------------------------------------===//
> +template<support::endianness target_endianness, bool is64Bits>
> +ELFHeaderChunk <target_endianness,is64Bits>
> +              ::ELFHeaderChunk(const WriterOptionsELF &options,
> +                               const File &file) {

Why is File used sometimes and ELFFile used others?

> +  this->_size = this->size();

Don't use this-> unless it's required.

> +
> +  this->e_ident(ELF::EI_MAG0, 0x7f);
> +  this->e_ident(ELF::EI_MAG1, 'E');
> +  this->e_ident(ELF::EI_MAG2, 'L');
> +  this->e_ident(ELF::EI_MAG3, 'F');
> +  if (options.is64Bit())
> +    this->e_ident(ELF::EI_CLASS, 2);
> +  else
> +    this->e_ident(ELF::EI_CLASS, 1);
> +
> +  if (options.endianness() == support::little)
> +    this->e_ident(ELF::EI_DATA, ELF::ELFDATA2LSB);
> +  else
> +    this->e_ident(ELF::EI_DATA, ELF::ELFDATA2MSB);
> +
> +  this->e_ident(ELF::EI_VERSION, 1);
> +  this->e_ident(ELF::EI_OSABI, ELF::ELFOSABI_NONE);
> +
> +  this->e_type(this->filetype(options.outputKind()));
> +  this->e_machine(options.cpuType());
> +  this->e_version(1);
> +
> +  this->e_entry((uint64_t)0x0);
> +  this->e_phoff((uint64_t)this->_size);
> +  this->e_shoff((uint64_t)0x0);
> +
> +
> +  this->e_flags(0);
> +  this->e_ehsize(this->_size);
> +  this->e_phentsize(0);
> +  this->e_phnum(0);
> +  this->e_shentsize(0);
> +  this->e_shnum(0);
> +  this->e_shstrndx(0);
> +}
> +
> +template< support::endianness target_endianness,
> +         bool is64Bits>
> +StringRef ELFHeaderChunk<target_endianness,is64Bits>
> +                        ::segmentName() const {
> +  return StringRef("ELF");
> +}
> +
> +template< support::endianness target_endianness,
> +         bool is64Bits>
> +void ELFHeaderChunk<target_endianness,is64Bits>
> +                   ::write(uint8_t *chunkBuffer) {
> +  ::memcpy (chunkBuffer, &this->_eh, this->size());

No space before (

> +}
> +
> +template< support::endianness target_endianness,
> +         bool is64Bits>
> +const char* ELFHeaderChunk<target_endianness,is64Bits>
> +                          ::info() {
> +  return "elf_header";
> +}
> +
> +template< support::endianness target_endianness,
> +          bool is64Bits>
> +uint32_t ELFHeaderChunk<target_endianness,is64Bits>
> +                       ::filetype(WriterOptionsELF::OutputKind kind) {
> +  switch ( kind ) {
> +    case WriterOptionsELF::outputDynamicExecutable:
> +    case WriterOptionsELF::outputStaticExecutable:
> +      return ELF::ET_EXEC;
> +    case WriterOptionsELF::outputDylib:
> +      return ELF::ET_DYN;
> +    case WriterOptionsELF::outputObjectFile:
> +      return ELF::ET_REL;
> +  }
> +  assert(0 && "file outputkind not supported");

Use llvm_unreachable.

> +}
> +
> +//===----------------------------------------------------------------------===//
> +//  ELFSectionHeaderChunk
> +//  List of Section Headers:
> +//[Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> +//[ 0]                   NULL            00000000 000000 000000 00      0   0  0
> +//[ 1] .text             PROGBITS        00000000 000034 000040 00  AX  0   0  4
> +//===----------------------------------------------------------------------===//
> +template<support::endianness target_endianness, bool is64Bits>
> +ELFSectionHeaderChunk<target_endianness,is64Bits>
> +              ::ELFSectionHeaderChunk(const WriterOptionsELF& options,
> +                                      class ELFWriter<target_endianness, is64Bits>&
> +                                      writer)
> +    : _options(options)
> +    , _writer(writer) {
> +
> +  this->_size = 0;
> +  this->_align2 = 0;
> +//
> +// The first element in the list is always NULL
> +//
> +    Elf_Shdr *nullShdr = new Elf_Shdr;

Who deletes this? Can we use unique_ptr?

> +    ::memset((void *)nullShdr, 0, sizeof (Elf_Shdr));
> +    _sectionInfo.push_back (nullShdr);
> +
> +    this->_size += sizeof (Elf_Shdr);
> +//TODO : implement strtab and shstrtab
> +//khemant
> +  Elf_Shdr *shdr ;
> +  ELFStringSectionChunk<target_endianness,is64Bits> *str = _writer.shstrtab();
> +  std::vector<SectionChunk<target_endianness,is64Bits>*>
> +              sections = _writer.sectionChunks();

You can use auto here.

> +
> +  typename  std::vector<SectionChunk<target_endianness,is64Bits>*>::iterator
> +                        si = sections.begin();
> +  typename  std::vector<SectionChunk<target_endianness,is64Bits>*>::iterator
> +                        se = sections.end();
> +
> +
> +
> +  for ( ; si != se ; ++si) {

For range.

> +     shdr = new Elf_Shdr;

Ownership?

> +     StringRef name = (*si)->sectionName();
> +     uint64_t offset = str->addString(name);
> +     shdr->sh_name = offset;
> +     shdr->sh_type = (*si)->type();
> +     shdr->sh_flags = (*si)->flags();
> + // TODO: At the time of creation of this section header, we will not have any
> + // address and offset info. We  revisit this after assigning the file
> + // offsets
> +     shdr->sh_offset = (*si)->fileOffset();
> +     shdr->sh_addr = (*si)->address();
> +     shdr->sh_size = (*si)->size();
> +// The next two fields have special meanings:
> +// sh_type           sh_link                             sh_info
> +// SHT_DYNAMIC  The section header index of the string
> +//                table used by entries in the section.   0
> +// SHT_HASH     The section header index of the symbol
> +//                table to which the hash table applies.  0
> +// SHT_REL
> +// SHT_RELA     The section header index of the
> +//                associated symbol table.                The section header
> +//                                                        index of the section
> +//                                                        to which the relocation
> +//                                                        applies.
> +// SHT_SYMTAB
> +// SHT_DYNSYM   The section header index of the
> +//                associated string table.                One greater than the
> +//                                                        symbol table index of
> +//                                                        the last local symbol
> +//                                                        (binding STB_LOCAL).
> +// SHT_GROUP    The section header index of the
> +//              associated symbol table.                  The symbol table
> +//                                                        index of an entry in
> +//                                                        the associated symbol
> +//                                                        table. The name of
> +//                                                        the specified symbol
> +//                                                        table entry provides
> +//                                                        a signature for the
> +//                                                        section group.
> +// SHT_SYMTAB_SHNDX The section header index of
> +//                  the associated symbol table section.  0
> +// None of these chunks are of the above mentioned type, so we short them.
> +     shdr->sh_link = 0;
> +     shdr->sh_info = 0;
> +     shdr->sh_addralign = (*si)->align2();
> +// Not a special section with fixed entries
> +     shdr->sh_entsize = 0;
> +
> +     _sectionInfo.push_back(shdr);
> +    this->_size += sizeof (Elf_Shdr);
> +  }
> +
> +// Now I add in the section string table. For some reason This seems to be
> +// preferred location of string sectiosn in contemporary
> +// (ones that must not be named) linker(s).
> +  shdr = new Elf_Shdr;

Ownership?

> +// I push the name of the string table into the string table as soon as
> +// it is created
> +  shdr->sh_name = 1;
> +  shdr->sh_type = ELF::SHT_STRTAB;
> +  shdr->sh_flags = 0;
> +// NOTE: Refer to above note when assigning st_addr for other sections
> +  shdr->sh_addr = str->address();
> +  shdr->sh_offset = str->fileOffset();
> +  shdr->sh_size = str->size();
> +  shdr->sh_link = 0;
> +  shdr->sh_info = 0;
> +// THis section is not a loadable section, hence we do not care about alignment
> +  shdr->sh_addralign = 1;
> +  _sectionInfo.push_back(shdr);
> +  this->_size += sizeof (Elf_Shdr);
> +
> +
> +

Useless whitespace.

> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> +StringRef ELFSectionHeaderChunk<target_endianness,is64Bits>::segmentName()
> +          const {
> +  return StringRef("SHDR");
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> +void ELFSectionHeaderChunk<target_endianness,is64Bits>::write(
> +                                                    uint8_t *chunkBuffer) {
> +    for (auto si =this->_sectionInfo.begin(), se = this->_sectionInfo.end();
> +         si != se; ++si) {

For range.

> +
> +      Elf_Shdr *shdr = (*si);
> +      ::memcpy(chunkBuffer,
> +               shdr,
> +               sizeof (Elf_Shdr));
> +      chunkBuffer += sizeof (Elf_Shdr);
> +    }

Indent 2 spaces.

> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> +uint16_t ELFSectionHeaderChunk<target_endianness,is64Bits>::count() {
> +    return _sectionInfo.size();
> +}
> +template<support::endianness target_endianness, bool is64Bits>
> +uint16_t ELFSectionHeaderChunk<target_endianness,is64Bits>::size() {
> +    return sizeof (Elf_Shdr);
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> +const char* ELFSectionHeaderChunk<target_endianness,is64Bits>::info() {
> +  return "elf_section_header";
> +}
> +
> +
> +//===----------------------------------------------------------------------===//
> +//  ELFProgramHeaderChunk
> +//===----------------------------------------------------------------------===//
> +// TODO: Implement the methods
> +
> +//===----------------------------------------------------------------------===//
> +//  ELFWriter Class
> +//===----------------------------------------------------------------------===//
> +template <support::endianness target_endianness, bool is64Bits>
> +class ELFWriter : public Writer {
> +public:
> +  LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> +  typedef object::Elf_Shdr_Impl<target_endianness, is64Bits> Elf_Shdr;
> +  ELFWriter(const WriterOptionsELF &options);
> +  virtual error_code  writeFile(const lld::File &file, StringRef path);
> +  const std::vector<Chunk<target_endianness,
> +                          is64Bits> * > chunks() { return _chunks; }
> +  KindHandler *kindHandler() { return _referenceKindHandler; }
> +  std::vector<SectionChunk<target_endianness,
> +              is64Bits> * > sectionChunks() { return _sectionChunks ; }
> +   ELFStringSectionChunk<target_endianness,is64Bits> *shstrtab()
> +                                                   {return shstrtable;}
> +
> +private:
> +  void build (const lld::File &file);
> +  void createChunks (const lld::File &file);

Why File instead of ELFFile?

> +  void assignFileOffsets();
> +  const WriterOptionsELF &_options;
> +  ELFStringSectionChunk<target_endianness,is64Bits> *shstrtable ;
> +  KindHandler *_referenceKindHandler;
> +  class ELFSectionHeaderChunk<target_endianness,is64Bits> *_sectionHeaderChunk;
> +  std::vector<Chunk<target_endianness,is64Bits> * > _chunks;
> +  const DefinedAtom *_entryAtom;
> +  std::vector<SectionChunk<target_endianness,is64Bits> * > _sectionChunks;
> +};
> +
> +
> +//===----------------------------------------------------------------------===//
> +//  ELFWriter
> +//===----------------------------------------------------------------------===//
> +template <support::endianness target_endianness, bool is64Bits>
> +ELFWriter<target_endianness,is64Bits>::ELFWriter
> +                                    (const WriterOptionsELF &options)
> +  : _options(options),
> +    _referenceKindHandler(KindHandler::makeHandler(_options.architecture()))
> +{}
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +void ELFWriter<target_endianness,is64Bits>::build(const lld::File &file)
> +{
> +  // Create objects for each chunk.
> +  this->createChunks(file);
> +
> +  this->assignFileOffsets();
> +}
> +
> +
> +// Creating chunks is a delicate dance in ELF.
> +// 1. ELF Header
> +// 2. Section Header table
> +// 3. String Section (we need this for putting in sh_name member
> +// 4. All the stock sections
> +// 5. Section String section
> +// 6. String section
> +// 7. Symbol section
> +// The above sequence is usually what an ELF section layout looks like.
> +// Since the _chunks vector has all these chunks, we need to be careful with
> +// this sequence when we actually do file offset assignment
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +void ELFWriter<target_endianness,is64Bits>::createChunks(const lld::File &file){
> +  std::map<DefinedAtom::ContentType,
> +            SectionChunk<target_endianness,is64Bits>* > map;

What is this for (comment)?

> +  ELFHeaderChunk<target_endianness,is64Bits>*ehc;

Space after , and before *.

> +
> +// We need to create hand crafted sections such as
> +// shstrtab strtab hash and symtab to put relevant information in
> +// ELF structures and then process the atoms
> +
> +  shstrtable = new  ELFStringSectionChunk<target_endianness,is64Bits>
> +                             (_options,*this,StringRef("shstrtab"));

Ownership?

> +   shstrtable->addString(StringRef("shstrtab"));
> +  //uint64_t ShstrtabOffset = shstrtable->addString(StringRef("shstrtab"));
> +
> +
> +//we also need to process undefined atoms
> +  for (const DefinedAtom* atom : file.defined() ) {
> +    // TODO: Add sectionChoice.
> +    // assert( atom->sectionChoice() == DefinedAtom::sectionBasedOnContent );
> +    DefinedAtom::ContentType type = atom->contentType();
> +    auto pos = map.find(type);
> +    if (pos == map.end()) {
> +      StringRef sectionName = atom->customSectionName();
> +      SectionChunk<target_endianness,is64Bits>
> +                  *chunk = new SectionChunk<target_endianness,is64Bits>
> +                                   (type, sectionName, _options, *this);
> +      if (chunk->type() != ELF::SHT_NULL ) {
> +        map[type] = chunk;
> +        chunk->appendAtom(atom);
> +        _sectionChunks.push_back(chunk);
> +      }
> +    }
> +    else {
> +      pos->second->appendAtom(atom);
> +    }

What is this code doing?

> +    //printf ("Atom Name: %s\n", atom->name().data());
> +  }
> +
> +  //put in the Undefined atoms as well
> +
> +  // Make header chunk
> +    ehc =
> +      new ELFHeaderChunk<target_endianness,is64Bits>
> +        (_options, file);
> +
> +    _sectionHeaderChunk =
> +    new ELFSectionHeaderChunk<target_endianness,is64Bits>( _options, *this);

Ownership?

> +
> +    ehc->e_shoff(ehc->size());
> +    ehc->e_shentsize(_sectionHeaderChunk->size());
> +    ehc->e_shnum(_sectionHeaderChunk->count());
> +
> + // I am pushign string section after all sections are in.

pushing

> + // Hence the index will be total number of non-custom sections we have
> +
> +    ehc->e_shstrndx(_sectionChunks.size() + 1);
> +    _chunks.push_back(ehc);
> +    _chunks.push_back(_sectionHeaderChunk);
> +// We have ELF header, section header. Now push rest of sections
> +    for (auto chnk : _sectionChunks)
> +        _chunks.push_back(chnk);
> +    _chunks.push_back(shstrtable);
> +
> +}
> +
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +void ELFWriter<target_endianness,is64Bits>::assignFileOffsets() {
> +  DEBUG_WITH_TYPE("WriterELF-layout", dbgs()
> +                    << "assign file offsets:\n");
> +  uint64_t offset = 0;
> +  uint64_t address = 0;
> +  for ( Chunk<target_endianness,is64Bits> *chunk : _chunks ) {
> +// FIXME: A __LINKEDIT segment is similar to PT_DYNAMIC segment in ELF?
> +//        I don't see any special alignment requirements however.
> +#if 0
> +    if ( chunk->segmentName().equals("__LINKEDIT") ) {
> +      _linkEditStartOffset  = Chunk::alignTo(offset, 12);
> +      _linkEditStartAddress = Chunk::alignTo(address, 12);
> +      break;
> +    }
> +#endif
> +    chunk->assignFileOffset(offset, address);
> +  }
> +//TODO: We need to fix all file offsets inside various ELF section headers
> +  std::vector<Elf_Shdr*>SecInfo = _sectionHeaderChunk->sectionInfo();
> +  typename std::vector<Elf_Shdr*>::iterator it = SecInfo.begin();
> +// First section is a NULL section with no sh_offset fix
> +   (*it)->sh_offset = 0;
> +   (*it)->sh_addr = 0;
> +   ++it;
> +  for (auto si=_sectionChunks.begin(), se=_sectionChunks.end();
> +       si!=se ; ++si){
> +       (*it)->sh_offset = (*si)->fileOffset();
> +       (*it)->sh_addr = (*si)->address();
> +       ++it;
> +  }
> +// We  have taken care of  all the stock sections. We need to deal with
> +// custom sections
> +// They are section string table, string table and symbol table
> +  (*it)->sh_offset = shstrtable->fileOffset();
> +  (*it)->sh_addr = shstrtable->address();
> +
> +}
> +
> +
> +template <support::endianness target_endianness, bool is64Bits>
> +error_code ELFWriter<target_endianness,is64Bits>::writeFile
> +                                (const lld::File &file, StringRef path) {
> +  this->build(file);
> +
> +  uint64_t totalSize = _chunks.back()->fileOffset() + _chunks.back()->size();
> +//
> +// Pulled in from mailing list:
> +// http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-May/049841.html
> +// Updated to compile with most recent llvm.
> +//
> +  OwningPtr<FileOutputBuffer> buffer;
> +  error_code ec = FileOutputBuffer::create(path,
> +                                          totalSize, buffer,
> +                                          FileOutputBuffer::F_executable);
> +  if ( ec )
> +    return ec;
> +
> +  for ( Chunk<target_endianness,is64Bits> *chunk : _chunks ) {
> +    chunk->write(buffer->getBufferStart()+chunk->fileOffset());
> +  }
> +  return buffer->commit();
> +
> +}
> +
> +} // namespace elf
> +
> +
> +Writer* createWriterELF(const WriterOptionsELF &options) {
> +
> +
> +   std::pair<support::endianness,bool> archType = options.getArchTypeClass();
> +   if (archType.first == support::little && archType.second == false)
> +    return new lld::elf::ELFWriter<support::little,false>(options);
> +   else if (archType.first == support::little && archType.second == true)
> +    return new lld::elf::ELFWriter<support::little,true>(options);
> +   if (archType.first == support::big && archType.second == false)
> +    return new lld::elf::ELFWriter<support::big,false>(options);
> +   else if (archType.first == support::big && archType.second == true)
> +    return new lld::elf::ELFWriter<support::big,true>(options);
> +
> +    return nullptr;
> +}

Needs proper whitespace and ownership semantics.

>  } // namespace lld
> -
> Index: lib/ReaderWriter/ELF/ReferenceKinds.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/ReferenceKinds.cpp	(revision 0)
> +++ lib/ReaderWriter/ELF/ReferenceKinds.cpp	(revision 0)
> @@ -0,0 +1,172 @@
> +//===- lib/ReaderWriter/ELF/ReferenceKinds.cpp ----------------------------===//
> +//
> +//                             The LLVM Linker
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +
> +#include "ReferenceKinds.h"
> +
> +#include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/StringSwitch.h"
> +
> +#include "llvm/Support/ErrorHandling.h"
> +
> +namespace lld {
> +namespace elf {
> +
> +//===----------------------------------------------------------------------===//
> +//  KindHandler
> +//===----------------------------------------------------------------------===//
> +
> +KindHandler::KindHandler() {
> +}
> +
> +KindHandler::~KindHandler() {
> +}
> +
> +KindHandler *KindHandler::makeHandler(WriterOptionsELF::Architecture arch) {

Who owns the return value?

> +  switch( arch ) {
> +    case WriterOptionsELF::arch_hexagon:
> +      return new KindHandler_hexagon();
> +    case WriterOptionsELF::arch_x86:
> +      return new KindHandler_x86();
> +    default:
> +      assert(0 && "arch not supported");

llvm_unreachable. Although should be a proper diagnostic when we have them.

> +  }
> +}
> +
> +//===----------------------------------------------------------------------===//
> +//  KindHandler_x86
> +//  TODO: more to do here
> +//===----------------------------------------------------------------------===//
> +
> +KindHandler_x86::~KindHandler_x86() {
> +}
> +
> +Reference::Kind KindHandler_x86::stringToKind(StringRef str) {
> +  return llvm::StringSwitch<Reference::Kind>(str)
> +    .Case("none",                  none)
> +    .Default(invalid);
> +
> +  llvm_unreachable("invalid x86 Reference kind");
> +}
> +
> +StringRef KindHandler_x86::kindToString(Reference::Kind kind) {
> +  switch ( (Kinds)kind ) {
> +    case invalid:
> +      return StringRef("invalid");
> +    case none:
> +      return StringRef("none");
> +  }
> +  llvm_unreachable("invalid x86 Reference kind");
> +}
> +
> +bool KindHandler_x86::isCallSite(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_x86::isCallSite");
> +  return false;
> +}
> +
> +bool KindHandler_x86::isPointer(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_x86::isPointer");
> +  return false;
> +}
> +
> +bool KindHandler_x86::isLazyImmediate(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_x86::isLazyImmediate");
> +  return false;
> +}
> +
> +bool KindHandler_x86::isLazyTarget(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_x86::isLazyTarget");
> +  return false;
> +}
> +
> +
> +void KindHandler_x86::applyFixup(Kind kind, uint64_t addend,
> +                                    uint8_t *location, uint64_t fixupAddress,
> +                                    uint64_t targetAddress) {
> +  int32_t *loc32 = reinterpret_cast<int32_t*>(location);
> +  uint64_t *loc64 = reinterpret_cast<uint64_t*>(location);
> +  switch ( (Kinds)kind ) {
> +    case none:
> +      // do nothing
> +      break;
> +    case invalid:
> +      assert(0 && "invalid Reference Kind");
> +      break;
> +  }
> +}
> +
> +//===----------------------------------------------------------------------===//
> +//  KindHandler_hexagon
> +//  TODO: more to do here
> +//===----------------------------------------------------------------------===//
> +
> +KindHandler_hexagon::~KindHandler_hexagon() {
> +}
> +
> +Reference::Kind KindHandler_hexagon::stringToKind(StringRef str) {
> +  return llvm::StringSwitch<Reference::Kind>(str)
> +    .Case("none",                  none)
> +    .Default(invalid);
> +
> +  llvm_unreachable("invalid hexagon Reference kind");
> +}
> +
> +StringRef KindHandler_hexagon::kindToString(Reference::Kind kind) {
> +  switch ( (Kinds)kind ) {
> +    case invalid:
> +      return StringRef("invalid");
> +    case none:
> +      return StringRef("none");
> +  }
> +  llvm_unreachable("invalid hexagon Reference kind");
> +}
> +
> +bool KindHandler_hexagon::isCallSite(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isCallSite");
> +  return false;
> +}
> +
> +bool KindHandler_hexagon::isPointer(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isPointer");
> +  return false;
> +}
> +
> +bool KindHandler_hexagon::isLazyImmediate(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isLazyImmediate");
> +  return false;
> +}
> +
> +bool KindHandler_hexagon::isLazyTarget(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isLazyTarget");
> +  return false;
> +}
> +
> +
> +void KindHandler_hexagon::applyFixup(Kind kind, uint64_t addend,
> +                                    uint8_t *location, uint64_t fixupAddress,
> +                                    uint64_t targetAddress) {
> +  int32_t *loc32 = reinterpret_cast<int32_t*>(location);
> +  uint64_t *loc64 = reinterpret_cast<uint64_t*>(location);
> +  switch ( (Kinds)kind ) {
> +    case none:
> +      // do nothing
> +      break;
> +    case invalid:
> +      assert(0 && "invalid Reference Kind");
> +      break;
> +  }
> +}
> +
> +
> +
> +} // namespace elf
> +} // namespace lld
> +
> +
> +
> Index: lib/ReaderWriter/ELF/ReaderELF.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/ReaderELF.cpp	(revision 163142)
> +++ lib/ReaderWriter/ELF/ReaderELF.cpp	(working copy)
> @@ -214,7 +214,11 @@
>        return (Alignment(Symbol->st_value));
>      }
>
> -    return Alignment(1);
> +    return Alignment(llvm::Log2_64(Section->sh_addralign));
> +//      return (Alignment(llvm::Log2_32(Symbol->st_value)));
> +//    }
> +//
> +//    return Alignment(llvm::Log2_32(Section->sh_addralign));

What is this change doing?

>    }
>
>    // Do we have a choice for ELF?  All symbols
> Index: lib/ReaderWriter/ELF/CMakeLists.txt
> ===================================================================
> --- lib/ReaderWriter/ELF/CMakeLists.txt	(revision 163142)
> +++ lib/ReaderWriter/ELF/CMakeLists.txt	(working copy)
> @@ -1,4 +1,6 @@
>  add_lld_library(lldELF
>    ReaderELF.cpp
>    WriterELF.cpp
> +  ReferenceKinds.cpp
> +  WriterOptionsELF.cpp
>    )
> Index: lib/ReaderWriter/ELF/ReferenceKinds.h
> ===================================================================
> --- lib/ReaderWriter/ELF/ReferenceKinds.h	(revision 0)
> +++ lib/ReaderWriter/ELF/ReferenceKinds.h	(revision 0)
> @@ -0,0 +1,93 @@
> +//===- lib/ReaderWriter/ELF/ReferenceKinds.h ------------------------------===//
> +//
> +//                             The LLVM Linker
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +
> +#include "lld/Core/LLVM.h"
> +#include "lld/Core/Reference.h"
> +#include "lld/ReaderWriter/WriterELF.h"
> +
> +#ifndef LLD_READER_WRITER_ELF_REFERENCE_KINDS_H_
> +#define LLD_READER_WRITER_ELF_REFERENCE_KINDS_H_
> +
> +namespace lld {
> +namespace elf {
> +
> +
> +///
> +/// The KindHandler class is the abstract interface to Reference::Kind
> +/// values for ELF files.  Particular Kind values (e.g. 3) has a different
> +/// meaning for each architecture.
> +/// TODO: Needs to be updated for ELF, stubs for now.
> +///
> +class KindHandler {
> +public:
> +  typedef Reference::Kind Kind;
> +
> +  static KindHandler *makeHandler(WriterOptionsELF::Architecture arch);
> +  virtual             ~KindHandler();
> +  virtual Kind        stringToKind(StringRef str) = 0;
> +  virtual StringRef   kindToString(Kind) = 0;
> +  virtual bool        isCallSite(Kind) = 0;
> +  virtual bool        isPointer(Kind) = 0;
> +  virtual bool        isLazyImmediate(Kind) = 0;
> +  virtual bool        isLazyTarget(Kind) = 0;
> +  virtual void        applyFixup(Kind kind, uint64_t addend, uint8_t *location,
> +                           uint64_t fixupAddress, uint64_t targetAddress) = 0;
> +
> +protected:
> +  KindHandler();
> +};
> +
> +
> +class KindHandler_hexagon : public KindHandler {
> +public:
> +  enum Kinds {
> +    invalid,         // used to denote an error creating a Reference
> +    none,
> +  };
> +
> +  virtual ~KindHandler_hexagon();
> +  virtual Kind stringToKind(StringRef str);
> +  virtual StringRef kindToString(Kind);
> +  virtual bool isCallSite(Kind);
> +  virtual bool isPointer(Kind);
> +  virtual bool isLazyImmediate(Kind);
> +  virtual bool isLazyTarget(Kind);
> +  virtual void applyFixup(Kind kind, uint64_t addend, uint8_t *location,
> +                  uint64_t fixupAddress, uint64_t targetAddress);
> +
> +};
> +
> +
> +class KindHandler_x86 : public KindHandler {
> +public:
> +  enum Kinds {
> +    invalid,         // used to denote an error creating a Reference
> +    none,
> +  };
> +
> +  virtual ~KindHandler_x86();
> +  virtual Kind stringToKind(StringRef str);
> +  virtual StringRef kindToString(Kind);
> +  virtual bool isCallSite(Kind);
> +  virtual bool isPointer(Kind);
> +  virtual bool isLazyImmediate(Kind);
> +  virtual bool isLazyTarget(Kind);
> +  virtual void applyFixup(Kind kind, uint64_t addend, uint8_t *location,
> +                  uint64_t fixupAddress, uint64_t targetAddress);
> +
> +};
> +
> +} // namespace elf
> +} // namespace lld
> +
> +
> +
> +#endif // LLD_READER_WRITER_ELF_REFERENCE_KINDS_H_
> +
>

- Michael Spencer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ReaderWriter3.patch
Type: application/octet-stream
Size: 50287 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120910/509510a7/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ELF.patch
Type: application/octet-stream
Size: 4268 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120910/509510a7/attachment-0001.obj>


More information about the llvm-commits mailing list