[llvm-commits] LLD: patch Program Headers
Hemant Kulkarni
khemant at codeaurora.org
Fri Nov 2 15:28:31 PDT 2012
MSVC doesn't like this in debug mode. The fix is to add:
#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
If I put a preprocessor to include a certain version of the overloaded operator, I will also need to add a preprocessor and make sure I pass in an object of that type in the equal_range function. Is adding such #ifdef a good idea?
We could alternatively just keep the both operands chunks version so all compilers build it at all levels. What do you say?
-Hemant
--
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: Thursday, November 01, 2012 2:20 PM
To: Hemant Kulkarni
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] LLD: patch Program Headers
On Mon, Oct 29, 2012 at 4:16 PM, Hemant Kulkarni <khemant at codeaurora.org> wrote:
> Hi,
>
>
>
> The attached patch adds machinery to assign offsets and addresses to
> chunks such that program headers can be created using them. The patch
> also puts in the mechanism to emit these headers.
>
> Please review the patch and comment on it. The patch rearranges the
> sections based on where they usually fall in an ELF file for other
> linkers. This triggered change in the tests.
>
>
>
> More test cases will follow to check corners.
>
>
>
> --
>
> Hemant Kulkarni
>
> khemant at codeaurora.org
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by the Linux Foundation
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> Index: include/lld/ReaderWriter/WriterELF.h
> ===================================================================
> --- include/lld/ReaderWriter/WriterELF.h (revision 166506)
> +++ include/lld/ReaderWriter/WriterELF.h (working copy)
> @@ -33,6 +33,8 @@
> , _type(llvm::ELF::ET_EXEC)
> , _pointerWidth(4)
> , _machine(llvm::ELF::EM_386)
> + , _baseAddress(0x400000)
> + , _pageSize(0x1000)
> {}
>
> /// \brief Create a specific instance of an architecture.
> @@ -45,18 +47,24 @@
> const llvm::support::endianness endian,
> const uint16_t Type,
> const uint16_t Machine,
> - uint64_t pointerWidth = 4)
> + uint64_t pointerWidth = 4,
> + uint64_t baseAddress = 0x400000,
> + uint64_t pageSize = 0x1000)
> : _is64Bit(Is64Bit)
> , _endianness(endian)
> , _type(Type)
> , _pointerWidth(pointerWidth)
> - , _machine(Machine) {}
> + , _machine(Machine)
> + , _baseAddress(baseAddress)
> + , _pageSize(pageSize) {}
>
> bool is64Bit() const { return _is64Bit; }
> llvm::support::endianness endianness() const { return _endianness; }
> uint16_t type() const { return _type; }
> uint16_t machine() const { return _machine; }
> uint16_t pointerWidth() const { return _pointerWidth; }
> + uint64_t baseAddress() const { return _baseAddress; } uint64_t
> + pageSize() const { return _pageSize; }
>
> /// \brief Get the entry point if type() is ET_EXEC. Empty otherwise.
> StringRef entryPoint() const;
> @@ -67,6 +75,8 @@
> uint16_t _type;
> uint16_t _pointerWidth;
> uint16_t _machine;
> + uint64_t _baseAddress;
> + uint64_t _pageSize;
> };
>
> /// \brief Create a WriterELF using the given options.
> Index: lib/ReaderWriter/ELF/WriterELF.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/WriterELF.cpp (revision 166506)
> +++ lib/ReaderWriter/ELF/WriterELF.cpp (working copy)
> @@ -38,15 +38,38 @@
> #include <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.
> +enum {
> + CG_INVALID = 0x0,
> + CG_HEADER = 0x1,
> + CG_RWX = 0x3,
> + CG_RX = 0x4,
> + CG_R = 0x5,
> + CG_RW = 0x6,
> + CG_WX = 0x7,
> + CG_W = 0x8,
> + CG_X = 0x9,
> + CG_NO_LOAD = 0xFE,
> + CG_FILE_END = 0xFF
> +};
> +
> template<support::endianness target_endianness, bool is64Bits> class
> ELFWriter;
>
> +// Each "Chunk" (explained below) should be arranged in a weak strict
> +order // This is done to minimize memory utilization when creating program headers.
> +// This will remain in place till a more felxible mechanism of moving
> +chunks // around and packing for memory efficiency is put in place.
> +
This should use doxygen member grouping to group all the chunks.
http://www.stack.nl/~dimitri/doxygen/grouping.html#memgroup
For example:
/// \name Chunks
/// Each "Chunk" [...]
/// around and packing for memory efficiency is put in place.
/// \{
All the chunks go in here.
/// \}
> /// \brief A Chunk is a contiguous range of space.
> template<support::endianness target_endianness, bool is64Bits> class
> Chunk { @@ -65,14 +88,30 @@
> 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 chunkKind {
> + headerKind, // This is a header chunk
> + sectionKind // chunk represents a section
> + };
The enum needs to follow the coding standard.
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
Also, we should be using enum class. In this case I would do:
enum class Kind {
Header,
Section
};
> + chunkKind getChunkKind() const { return _cKind; } static inline
> + bool classof(const Chunk<target_endianness, is64Bits>*)
> + { return true;}
This is a weird place to indent the body to.
> +private:
> + const chunkKind _cKind;
> protected:
> Chunk();
> + Chunk(chunkKind 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;
> };
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -85,6 +124,16 @@
> b.setOrdinal(tempOrdinal);
> }
>
> +template<support::endianness target_endianness, bool is64Bits> struct
> +ChunkComparator {
> + bool operator()(uint16_t A, Chunk<target_endianness, is64Bits> *B) {
> + return (A < B->group());
Don't put parens around this.
> + }
> + bool operator()(Chunk<target_endianness, is64Bits> *A, uint16_t B) {
> + return (A->group() < B);
> + }
> +};
> +
MSVC doesn't like this in debug mode. The fix is to add:
#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;
>
> @@ -100,20 +149,22 @@
> void setShStrtableOffset (uint64_t val) {
> _offsetInStringTable = val; }
> uint32_t flags() { return _flags; }
> - uint32_t type() { return _type; }
> + uint32_t type() const { return _type; }
> uint64_t link() { return _link; }
> void link(uint64_t val) { _link = val; }
> uint16_t shinfo() { return _shinfo; }
> - bool isLoadable() { return _isLoadable; }
> - void isLoadable(uint64_t val) { _isLoadable = val; }
> uint64_t entsize() { return _entsize; }
> SectionChunk(StringRef secName, StringRef segName, bool loadable,
> uint64_t flags , uint64_t link, uint64_t info ,
> uint64_t type, uint64_t entsz, const WriterOptionsELF &op,
> ELFWriter<target_endianness, is64Bits> &writer);
> + static inline bool
> + classof(const SectionChunk<target_endianness, is64Bits> *s) {
> + return true;}
>
> + static inline bool classof(const Chunk<target_endianness, is64Bits> *c) {
> + return c->getChunkKind() == Chunk<target_endianness,
> + is64Bits>::sectionKind; }
> protected:
> - bool _isLoadable;
> uint64_t _link;
> uint64_t _shinfo;
> uint16_t _entsize;
> @@ -173,6 +224,13 @@
> virtual StringRef segmentName() const;
> virtual void write(uint8_t *fileBuffer);
> virtual const char *info();
> + static inline bool
> + classof(const ELFHeaderChunk<target_endianness, is64Bits> *s) {
> + return true;
> + }
Reflow this. It's indented to the wrong level.
> + static inline bool classof(const Chunk<target_endianness, is64Bits> *c) {
> + return c->getChunkKind() == Chunk<target_endianness,
> + is64Bits>::headerKind; }
>
> private:
> Elf_Ehdr _eh;
> @@ -192,12 +250,20 @@
> virtual StringRef segmentName() const;
> virtual void write(uint8_t *filebuffer);
> virtual const char *info();
> - void computeSize(const lld::File &file);
> + void computeSize();
> uint16_t count();
> uint16_t size();
> + void fixOffsets();
> const ArrayRef<Elf_Shdr*> sectionInfo() {
> return _sectionInfo;
> }
> + static inline bool
> + classof(const ELFSectionHeaderChunk<target_endianness, is64Bits> *s) {
> + return true;
> + }
Same.
> + static inline bool classof(const Chunk<target_endianness, is64Bits> *c) {
> + return c->getChunkKind() == Chunk<target_endianness,
> + is64Bits>::headerKind; }
>
> private:
> const WriterOptionsELF &_options;
> @@ -250,23 +316,41 @@
> llvm::BumpPtrAllocator _symbolAllocate; };
>
> -/// 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.
> +/// \brief ELFProgramHeaderChunk represents the Elf[32/64]_Phdr
> +structure near /// the start of an ELF executable file. ELFHeader's
> +e_phentsize and e_phnum /// show the number of entries in table and size of each one.
> 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);
> + typedef object::Elf_Phdr<target_endianness, is64Bits> Elf_Phdr;
> + ELFProgramHeaderChunk(const WriterOptionsELF &options,
> + ELFWriter<target_endianness, is64Bits> &);
>
> virtual StringRef segmentName() const;
> virtual void write(uint8_t *filebuffer);
> virtual const char *info();
> + void createPHeaders();
> + uint64_t computeNumber();
> + static inline bool
> + classof(const ELFHeaderChunk<target_endianness, is64Bits> *s) {
> + return true;
> + }
Same.
> + static inline bool classof(const Chunk<target_endianness, is64Bits> *c) {
> + return c->getChunkKind() == Chunk<target_endianness,
> + is64Bits>::headerKind; }
>
> private:
> -// TODO: Replace this with correct ELF::* type method
> + typedef typename std::vector<SectionChunk<target_endianness, is64Bits>*>
> + ::iterator secIterator;
> + const WriterOptionsELF &_options;
> + ELFWriter<target_endianness, is64Bits> &_writer;
> + std::vector<Elf_Phdr*> _programHeaders;
> + llvm::BumpPtrAllocator _headerAllocate;
> + typedef std::pair<uint64_t, uint64_t> sectionRange;
> + std::vector<sectionRange> _segments;
> + std::map<uint16_t, uint16_t> _groupToPF;
> +
> };
>
>
> //===-----------------------------------------------------------------
> -----===//
> @@ -274,9 +358,19 @@
>
> //===-----------------------------------------------------------------
> -----===//
>
> template<support::endianness target_endianness, bool is64Bits>
> -Chunk<target_endianness, is64Bits>::Chunk()
> - : _size(0), _address(0), _fileOffset(0), _align2(0) {
> - // 0 and 1 are reserved. 0 for ELF header and 1 for Sectiontable header.
> +Chunk<target_endianness, is64Bits>::Chunk(){
> + this->init();
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits> void
> +Chunk<target_endianness, is64Bits>::init(){
> + this->_size = 0;
> + this->_address = 0;
> + this->_fileOffset = 0;
> + this->_align2 = 0;
> + this->_group = CG_INVALID;
> + this->_isLoadable = false;
> + // 0 and 1 are reserved. 0 for ELF header and 1 for program header.
> static uint64_t orderNumber = 0;
> _ordinal = orderNumber++;
> }
> @@ -352,7 +446,8 @@
> uint64_t flags , uint64_t link, uint64_t info , uint64_t type,
> uint64_t entsz, const WriterOptionsELF &op,
> ELFWriter<target_endianness, is64Bits> &writer)
> - : _isLoadable(loadable)
> + : Chunk<target_endianness, is64Bits>(Chunk<target_endianness, is64Bits>
> + ::sectionKind)
> , _link(link)
> , _shinfo(info)
> , _entsize(entsz)
> @@ -362,8 +457,10 @@
> , _writer(writer)
> , _flags(flags)
> , _type(type)
> - , _offsetInStringTable(0) {}
> -
> + , _offsetInStringTable(0) {
> + this->isLoadable(loadable);
> + }
This seems indented wrong.
> +//FIXME: We need to make decision here for every section created
> template<support::endianness target_endianness, bool is64Bits> bool
> SectionChunk<target_endianness, is64Bits>::occupiesNoDiskSpace() {
> return false;
> @@ -388,6 +485,13 @@
> loadable, 0lu, 0lu, 0u, 0lu, 0lu,
> options, writer) {
> this->_segmentName = this->_isLoadable ? "PT_LOAD" : "PT_NULL" ;
> + // If the section is custom loadable section, group should be set explicitly.
> + // Stock non loadable section go as NO_LOAD and others will get
> + their // group determined by the atoms contained within. Many
> + constant // sections will have no symbols but the constants are
> + referred as // offset from start symbol, hence there may not be any
> + defined atoms being // appended to them, we force them at time of
> + creation to CG_R
> + this->setGroup(this->isLoadable() ? CG_INVALID : CG_NO_LOAD);
> switch(type) {
> case DefinedAtom::typeCode:
> this->_type = ELF::SHT_PROGBITS;
> @@ -400,6 +504,8 @@
> break;
> case DefinedAtom::typeConstant:
> this->_type = ELF::SHT_PROGBITS;
> + this->_flags = ELF::SHF_ALLOC;
> + this->setGroup(CG_R);
> break;
> default:
> llvm_unreachable("Unhandled content type for section!"); @@
> -416,6 +522,8 @@ template<support::endianness target_endianness, bool
> is64Bits> void StockSectionChunk<target_endianness, is64Bits>::
> appendAtom(const DefinedAtom *atom) {
> + static uint16_t groupArray[] = {CG_INVALID, CG_W, CG_R, CG_RW, CG_X,
> + CG_WX, CG_RX, CG_RWX};
> // Figure out offset for atom in this section given alignment constraints.
> uint64_t offset = this->_size;
> DefinedAtom::Alignment atomAlign = atom->alignment(); @@ -437,14
> +545,16 @@
> 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__)
> this->_flags |= ELF::SHF_ALLOC;
> if ((perms & DefinedAtom::permRW_) == DefinedAtom::permRW_)
> this->_flags |= (ELF::SHF_ALLOC | ELF::SHF_WRITE);
> if ((perms & DefinedAtom::permR_X) == DefinedAtom::permR_X)
> this->_flags |= (ELF::SHF_ALLOC | ELF::SHF_EXECINSTR);
> + if (atom->contentType() == DefinedAtom::typeZeroFill)
> + this->_flags |= (ELF::SHF_ALLOC | ELF::SHF_WRITE);
> + this->setGroup(groupArray[this->_flags]);
> }
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -492,6 +602,7 @@
> // First Add a null character. It also occupies 1 byte
> _stringSection.emplace_back("");
> this->_size = 1;
> + this->setGroup(CG_NO_LOAD);
> }
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -543,6 +654,8 @@
> this->_link = 0;
> this->_entsize = sizeof(Elf_Sym);
> this->_size = sizeof(Elf_Sym);
> + this->_align2 = this->_options.pointerWidth();
> + this->setGroup(CG_NO_LOAD);
> }
>
> template< support::endianness target_endianness, bool is64Bits> @@
> -670,7 +783,9 @@ template<support::endianness target_endianness, bool
> is64Bits> ELFHeaderChunk<target_endianness, is64Bits>
> ::ELFHeaderChunk(const WriterOptionsELF &options,
> - const File &File) {
> + const File &File)
> + : Chunk<target_endianness, is64Bits>(Chunk<target_endianness, is64Bits>
> + ::headerKind){
> this->_size = size();
> e_ident(ELF::EI_MAG0, 0x7f);
> e_ident(ELF::EI_MAG1, 'E');
> @@ -689,7 +804,7 @@
> e_version(1);
>
> e_entry(0ULL);
> - e_phoff(this->_size);
> + e_phoff(0);
> e_shoff(0ULL);
>
> e_flags(0);
> @@ -699,6 +814,7 @@
> e_shentsize(0);
> e_shnum(0);
> e_shstrndx(0);
> + this->setGroup(CG_HEADER);
> }
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -730,7 +846,9 @@
> ::ELFSectionHeaderChunk(const WriterOptionsELF& options,
> ELFWriter<target_endianness,
> is64Bits>
> &writer)
> - : _options(options)
> + : Chunk<target_endianness, is64Bits>(Chunk<target_endianness, is64Bits>
> + ::headerKind) ,
> + _options(options)
> , _writer(writer) {
> this->_size = 0;
> this->_align2 = 0;
> @@ -738,11 +856,32 @@
> Elf_Shdr *nullshdr = new (_sectionAllocate.Allocate<Elf_Shdr>()) Elf_Shdr;
> ::memset(nullshdr, 0, sizeof (Elf_Shdr));
> _sectionInfo.push_back(nullshdr);
> -
> this->_size += sizeof (Elf_Shdr);
> + this->setGroup(CG_FILE_END);
> }
>
> +
> template<support::endianness target_endianness, bool is64Bits>
> +void ELFSectionHeaderChunk<target_endianness,
> +is64Bits>::computeSize(){
> + this->_size = (this->_writer.sectionChunks().size() + 1) *
> +sizeof(Elf_Shdr); }
> +
> +template<support::endianness target_endianness, bool is64Bits> void
> +ELFSectionHeaderChunk<target_endianness, is64Bits>::fixOffsets(){
> + auto it = _sectionInfo.begin();
> + auto sections = _writer.sectionChunks();
> + // First section is a NULL section with no sh_offset fix
> + (*it)->sh_offset = 0;
> + (*it)->sh_addr = 0;
> + ++it;
> + for (auto &chunk : sections){
> + (*it)->sh_offset = chunk->fileOffset();
> + (*it)->sh_addr = chunk->address();
> + ++it;
> + }
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> void ELFSectionHeaderChunk<target_endianness, is64Bits>::createHeaders(){
> ELFStringSectionChunk<target_endianness, is64Bits> *str =
> _writer.shstrtab();
>
> @@ -805,8 +944,113 @@
>
> //===-----------------------------------------------------------------
> -----===//
> // ELFProgramHeaderChunk
>
> //===-----------------------------------------------------------------
> -----===//
> -// TODO: Implement the methods
> +template<support::endianness target_endianness, bool is64Bits>
> +ELFProgramHeaderChunk<target_endianness, is64Bits>
> + ::ELFProgramHeaderChunk(const WriterOptionsELF &options,
> + ELFWriter<target_endianness, is64Bits>
> + &writer)
> + : Chunk<target_endianness, is64Bits>(Chunk<target_endianness, is64Bits>
> + ::headerKind)
> + , _options(options)
> + , _writer(writer) {
> + this->_align2 = 0;
> + this->setGroup(CG_HEADER);
> + _groupToPF[CG_RWX] = ELF::PF_R | ELF::PF_W | ELF::PF_X;
> + _groupToPF[CG_RW] = ELF::PF_R | ELF::PF_W;
> + _groupToPF[CG_RX] = ELF::PF_R | ELF::PF_X;
> + _groupToPF[CG_R] = ELF::PF_R;
> + _groupToPF[CG_W] = ELF::PF_W;
> + _groupToPF[CG_WX] = ELF::PF_W | ELF::PF_X;
> + _groupToPF[CG_X] = ELF::PF_X;
> + }
>
> +template<support::endianness target_endianness, bool is64Bits> void
> +ELFProgramHeaderChunk<target_endianness, is64Bits>::createPHeaders()
> +{
> +
> + //TODO Once dynamic linking is supported, implement PHDR segment
> + // Implement mechanism inside this class to correctly find
> + // attributes for sections such as eh_frame, note etc
> + // when they are supported.
> + const uint16_t seg[] = { CG_RWX, CG_RX, CG_R, CG_RW, CG_WX, CG_W,
> + CG_X }; std::pair<secIterator, secIterator> sr; auto sections =
> + _writer.sectionChunks(); _programHeaders.clear(); this->_size = 0;
> + for (auto i : seg) {
> + uint64_t size = 0, mSize = 0;
> + sr = std::equal_range(sections.begin(), sections.end(),
> + i, ChunkComparator<target_endianness,
> + is64Bits>());
sr and i need more descriptive names.
> + if (static_cast<int>(sr.first - sr.second) != 0) {
Why does this need a static_cast? And can't this just be sr.first != sr.second?
> + // FIXME: fix occupiesNoDiskSpace() function in Chunks class
> + auto j = sr.first, k = j + 1;
These need better names.
> + // Since this group has a section, atleast this is a part of segment
> + size = (*j)->occupiesNoDiskSpace() ? 0 : (*j)->size();
> + mSize = (*j)->size();
> + for (; k != sr.second; k++) {
> + // This means there are more than 1 sections of same permissions
> + if ((*k)->fileOffset() - (*j)->fileOffset() + size >
> + _options.pageSize()) {
> + // we have a case where padding zeros span more than a page
> + // we can skip those pages.
> + Elf_Phdr *phdr = new(_headerAllocate.Allocate<Elf_Phdr>()) Elf_Phdr;
> + phdr->p_type = ELF::PT_LOAD;
> + phdr->p_offset = (*j)->fileOffset();
> + phdr->p_vaddr = (*j)->address();
> + phdr->p_paddr = phdr->p_vaddr;
> + phdr->p_filesz = size;
> + // memory size may be more than file size if there are sections
> + // that do not occupy space on disk such as .bss
> + phdr->p_memsz = mSize;
> + phdr->p_flags = _groupToPF[i];
> + phdr->p_align = _options.pageSize();
> + _programHeaders.push_back(phdr);
> + this->_size += sizeof(Elf_Phdr);
> + j = k;
> + size = (*j)->occupiesNoDiskSpace() ? 0 : (*j)->size();
> + mSize = (*j)->size();
> + } else {
> + size = (*k)->fileOffset() - (*j)->fileOffset() +
> + ((*k)->occupiesNoDiskSpace() ? 0 : (*k)->size()) ;
> + mSize = (*k)->fileOffset() - (*j)->fileOffset() + (*k)->size();
> + }
> + }
Unexpected deindent here.
> + Elf_Phdr *phdr = new(_headerAllocate.Allocate<Elf_Phdr>()) Elf_Phdr;
> + phdr->p_type = ELF::PT_LOAD;
> + phdr->p_offset = (*j)->fileOffset();
> + phdr->p_vaddr = (*j)->address();
> + phdr->p_paddr = phdr->p_vaddr;
> + phdr->p_filesz = size;
> + phdr->p_memsz = mSize;
> + phdr->p_flags = _groupToPF[i];
> + phdr->p_align = _options.pageSize();
> + _programHeaders.push_back(phdr);
> + this->_size += sizeof(Elf_Phdr);
> + }
> + }
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> +uint64_t ELFProgramHeaderChunk<target_endianness,
> +is64Bits>::computeNumber() {
> + return _programHeaders.size();
> +}
> +template<support::endianness target_endianness, bool is64Bits> const
> +char *ELFProgramHeaderChunk<target_endianness, is64Bits>::info() {
> + return "elf_program_header";
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits> void
> +ELFProgramHeaderChunk<target_endianness, is64Bits>
> + ::write(uint8_t *chunkBuffer) {
> + for (const auto si : _programHeaders) {
> + ::memcpy(chunkBuffer, si, sizeof(*si));
> + chunkBuffer += sizeof (*si);
> + }
> +}
> +template<support::endianness target_endianness, bool is64Bits>
> +StringRef ELFProgramHeaderChunk<target_endianness, is64Bits>
> + ::segmentName() const {
> + return "PT_NULL";
> +}
>
> //===-----------------------------------------------------------------
> -----===//
> // ELFWriter Class
>
> //===-----------------------------------------------------------------
> -----===//
> @@ -816,24 +1060,26 @@
> LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> typedef object::Elf_Shdr_Impl<target_endianness, is64Bits> Elf_Shdr;
> typedef object::Elf_Sym_Impl<target_endianness, is64Bits> Elf_Sym;
> + typedef object::Elf_Phdr<target_endianness, is64Bits> Elf_Phdr;
> ELFWriter(const WriterOptionsELF &options);
> virtual error_code writeFile(const lld::File &File, StringRef path);
> uint64_t addressOfAtom(const Atom *atom);
> - ArrayRef<Chunk<target_endianness, is64Bits>*> chunks() { return
> _chunks; }
> + std::vector<Chunk<target_endianness, is64Bits>*> chunks() const {
> + return _chunks; }
properly flow this.
> KindHandler *kindHandler() { return _referenceKindHandler.get(); }
>
> - std::vector<SectionChunk<target_endianness, is64Bits>*>
> sectionChunks() {
> + std::vector<SectionChunk<target_endianness, is64Bits>*>
> + sectionChunks() const {
> return _sectionChunks ;
> }
>
> - ELFStringSectionChunk<target_endianness, is64Bits> *shstrtab() {
> + ELFStringSectionChunk<target_endianness, is64Bits> *shstrtab()
> + const {
> return _shstrtable;
> }
>
> - ELFStringSectionChunk<target_endianness, is64Bits> *strtab() {
> + ELFStringSectionChunk<target_endianness, is64Bits> *strtab() const
> + {
> return _strtable;
> }
> - ELFSymbolTableChunk<target_endianness, is64Bits> *symtab() {
> + ELFSymbolTableChunk<target_endianness, is64Bits> *symtab() const {
> return _symtable;
> }
>
> @@ -847,7 +1093,10 @@
> /// \brief AtomToAddress: Is a mapping from an Atom to the address
> where /// it will live in the output file.
> typedef llvm::DenseMap<const Atom*, uint64_t> AtomToAddress;
> -
> + typedef typename std::vector<Chunk<target_endianness, is64Bits>*>
> + ::iterator chunkIterator;
> + ELFProgramHeaderChunk<target_endianness, is64Bits> *_phdr;
> + ELFHeaderChunk<target_endianness, is64Bits> *ehc;
> ELFStringSectionChunk<target_endianness, is64Bits> *_shstrtable ;
> ELFStringSectionChunk<target_endianness, is64Bits> *_strtable ;
> ELFSymbolTableChunk<target_endianness, is64Bits> *_symtable; @@
> -878,7 +1127,22 @@ // Create objects for each chunk.
> createChunks(file);
> assignFileOffsets();
> + _phdr->createPHeaders();
> + _sectionHeaderChunk->createHeaders();
> + // Creating program headers changed its size. so we need to
> + re-assign offsets assignFileOffsets();
> + _sectionHeaderChunk->fixOffsets();
> + _phdr->createPHeaders();
> buildAtomToAddressMap();
> + ehc->e_shentsize(_sectionHeaderChunk->size());
> + ehc->e_shnum(_sectionHeaderChunk->count());
> + // We need to put the index of section string table in ELF header
> + // first two chunks are not sections so we subtract 2 to start
> + sections // and add 1 since we have a NULL header
> + ehc->e_shstrndx(_shstrtable->ordinal() - 1);
> + ehc->e_phnum(_phdr->computeNumber());
> + ehc->e_phoff(_phdr->fileOffset());
> + ehc->e_phentsize(sizeof(Elf_Phdr));
> }
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -888,17 +1152,17 @@
> sectionMap;
>
> // Make header chunk
> - ELFHeaderChunk<target_endianness, is64Bits> *ehc =
> - new (_chunkAllocate.Allocate
> + ehc = new (_chunkAllocate.Allocate
> <ELFHeaderChunk<target_endianness, is64Bits>>())
> ELFHeaderChunk<target_endianness, is64Bits>(_options, file);
> _chunks.push_back(ehc);
> +
> + _phdr = new (_chunkAllocate.Allocate
> + <ELFProgramHeaderChunk<target_endianness, is64Bits>>())
> + ELFProgramHeaderChunk<target_endianness, is64Bits>(_options,
> +
> + *this); _chunks.push_back(_phdr);
>
> - _sectionHeaderChunk = new (_chunkAllocate.Allocate<ELFSectionHeaderChunk
> - <target_endianness, is64Bits>>())
> - ELFSectionHeaderChunk
> - <target_endianness, is64Bits>(_options, *this);
> - _chunks.push_back(_sectionHeaderChunk);
> // 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.
> @@ -957,48 +1221,63 @@
> for (auto chnk : _sectionChunks)
> _chunks.push_back(chnk);
>
> -// After creating chunks, we might lay them out diffrently.
> -// Lets make sure symbol table, string table and section string table
> -// are at the end. In future we might provide knob -// to the driver
> to decide layout.
> - swapChunkPositions(*_chunks[_chunks.size() - 1],
> - *reinterpret_cast<Chunk<target_endianness,
> - is64Bits>*>(_shstrtable));
> - swapChunkPositions(*_chunks[_chunks.size() - 2],
> - *reinterpret_cast<Chunk<target_endianness,
> - is64Bits>*>(_strtable));
> - swapChunkPositions(*_chunks[_chunks.size() - 3],
> - *reinterpret_cast<Chunk<target_endianness,
> - is64Bits>*>(_symtable));
> -// We sort the _chunks vector to have all chunks as per ordianl
> number -// this will help to write out the chunks in the order we
> decided
> + _sectionHeaderChunk = new (_chunkAllocate.Allocate<ELFSectionHeaderChunk
> + <target_endianness, is64Bits>>())
> + ELFSectionHeaderChunk
> + <target_endianness,
> + is64Bits>(_options, *this); _chunks.push_back(_sectionHeaderChunk);
>
> - std::stable_sort(_chunks.begin(), _chunks.end(),([]
> - (const Chunk<target_endianness, is64Bits> *A,
> - const Chunk<target_endianness, is64Bits> *B) -> bool {
> - return (A->ordinal() < B->ordinal());}));
> +// We sort the chunks based on the group they belong to
> + std::stable_sort(_chunks.begin(), _chunks.end(), ([]
> + (const Chunk<target_endianness, is64Bits> *A,
> + const Chunk<target_endianness, is64Bits> *B) -> bool {
> + if (A->group() == CG_INVALID || B->group() == CG_INVALID)
> + llvm_unreachable("Invalid group number");
> + return (A->group() < B->group());}));
This is really hard to read.
std::stable_sort(_chunks.begin(), _chunks.end(),
[](const Chunk<target_endianness, is64Bits> *A,
const Chunk<target_endianness, is64Bits> *B) -> bool {
if (A->group() == CG_INVALID || B->group() == CG_INVALID)
llvm_unreachable("Invalid group number");
return A->group() < B->group();
});
Also get rid of the exra parens around the lambda and return.
> +// The CG_RW group also has to be arranged such that all //
> +SHT_NOBITS type of sections (.*bss) are at end of this // "partition"
> +of group.
Indent comments to the level of the code they describe.
> + chunkIterator cI;
> + std::pair<chunkIterator, chunkIterator> range; range =
> + std::equal_range(_chunks.begin() + 2, _chunks.end() - 1, CG_RW,
> + ChunkComparator<target_endianness,
> + is64Bits>());
> +
> + cI = std::stable_partition(range.first, range.second, ([]
> + (const Chunk<target_endianness, is64Bits> *A)
> + -> bool {
> + auto X = llvm::dyn_cast
> + <SectionChunk<target_endianness,
> + is64Bits>>(A);
> + return (X->type() !=
> + ELF::SHT_NOBITS);}));
Same issue as above.
cI = std::stable_partition(range.first, range.second,
[](const Chunk<target_endianness, is64Bits> *A) -> bool {
auto X = llvm::dyn_cast<SectionChunk<target_endianness, is64Bits>>(A);
return X->type() != ELF::SHT_NOBITS;
});
>
> +// We reassign all the ordinals since its needed when making headers
> +and // populating symbol table.
> + for (auto chnk : _chunks) {
> + static uint64_t i = 0;
Why is this static? That seems very broken.
> + chnk->setOrdinal(i++);
> + }
> +
> +//We sort the sections as per new ordinal set after group sorting.
> std::stable_sort(_sectionChunks.begin(), _sectionChunks.end(),([]
> (const SectionChunk<target_endianness, is64Bits> *A,
> const SectionChunk<target_endianness, is64Bits> *B) -> bool {
> return (A->ordinal() < B->ordinal());}));
> -
> -// Once the layout is fixed, we can now go and populate symbol table
> -// with correct st_shndx member.
>
> - for (auto chnk : _sectionChunks ){
> - Elf_Sym *sym = new (_chunkAllocate.Allocate<Elf_Sym>())Elf_Sym;
> - sym->st_name = 0;
> - sym->st_value = 0;
> - sym->st_size = 0;
> - sym->st_other = ELF::STV_DEFAULT;
> -// first two chunks are not sections hence we subtract 2 but there is
> a -// NULL section in section table so add 1
> - sym->st_shndx = chnk->ordinal() - 1 ;
> - sym->setBindingAndType(ELF::STB_LOCAL, ELF::STT_SECTION);
> - _symtable->addSymbol(sym);
> - }
> -
> +// Populate symbol table with correct st_shndx member.
> + if (_options.type() == ELF::ET_REL) {
> + for (auto chnk : _sectionChunks ){
Space before {.
> + Elf_Sym *sym = new
> + (_chunkAllocate.Allocate<Elf_Sym>())Elf_Sym;
Space before Elf_Sym.
> + sym->st_name = 0;
> + sym->st_value = 0;
> + sym->st_size = 0;
> + sym->st_other = ELF::STV_DEFAULT;
> + // first two chunks are not sections hence we subtract 2 but there is a
> + // NULL section in section table so add 1
> + sym->st_shndx = chnk->ordinal() - 1 ;
> + sym->setBindingAndType(ELF::STB_LOCAL, ELF::STT_SECTION);
> + _symtable->addSymbol(sym);
> + }
> + }
> for (const auto ssc : _stockSectionChunks){
> for (const auto da : ssc->atoms()) {
> _symtable->addSymbol(std::get<0>(da), ssc->ordinal() -1); @@
> -1011,15 +1290,6 @@
> for (const AbsoluteAtom *a : file.absolute()) {
> _symtable->addSymbol(a, ELF::SHN_ABS);
> }
> -
> - _sectionHeaderChunk->createHeaders();
> - ehc->e_shoff(ehc->size());
> - ehc->e_shentsize(_sectionHeaderChunk->size());
> - ehc->e_shnum(_sectionHeaderChunk->count());
> -// We need to put the index of section string table in ELF header -//
> first two chunks are not sections so we subtract 2 to start sections
> -// and add 1 since we have a NULL header
> - ehc->e_shstrndx(_shstrtable->ordinal() - 1); }
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -1040,23 +1310,38 @@
> DEBUG_WITH_TYPE("WriterELF-layout", dbgs()
> << "assign file offsets:\n");
> uint64_t offset = 0;
> - uint64_t address = 0;
> - for (auto chunk : _chunks) {
> -
> - chunk->assignFileOffset(offset, address);
> + uint64_t address = _options.baseAddress(); uint16_t chunkGroup;
> + static bool secondPass = false;
This shouldn't be static. It makes the entire writer only usable once. And breaks multithreading.
> + auto chunkIt = _chunks.begin();
> + // first (two in case of ET_EXEC or ET_DYN) chunks is (are) not
> + section(s) (*chunkIt)->assignFileOffset(offset, address);
> + chunkIt++; if (_options.type() == ELF::ET_EXEC ||
> + _options.type() == ELF::ET_DYN) {
> + (*chunkIt)->assignFileOffset(offset, address); chunkIt++;
Indendation?
> }
> - //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 &chunk : _sectionChunks){
> - (*it)->sh_offset = chunk->fileOffset();
> - (*it)->sh_addr = chunk->address();
> - ++it;
> + while (chunkIt != (_chunks.end() - 1) ) {
> + (*chunkIt)->assignFileOffset(offset, address);
> + if (_options.type() == ELF::ET_EXEC ||
> + _options.type() == ELF::ET_DYN) {
> + chunkGroup = (*chunkIt)->group();
> + chunkIt++;
> + // If the chunk group changes we start on new page
> + if (chunkGroup != (*chunkIt)->group() && (*chunkIt)->group() != CG_NO_LOAD
> + && (*chunkIt)->group() != CG_FILE_END)
> + address = address + _options.pageSize();
> + } else {
> + chunkIt++;
> + }
> }
> + (*chunkIt)->assignFileOffset(offset, address); if (secondPass) {
> + ehc->e_shoff(_sectionHeaderChunk->fileOffset());
> + } else {
> + secondPass = true;
> + }
> }
>
> template<support::endianness target_endianness, bool is64Bits>
>
- Michael Spencer
More information about the llvm-commits
mailing list