[llvm-commits] LLD patch - symbol table and reorganization of sections
Hemant Kulkarni
khemant at codeaurora.org
Mon Oct 1 10:28:56 PDT 2012
Please ignore my previous mail, I posted old patch again. Here is the new patch. Sorry.
--
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: Hemant Kulkarni [mailto:khemant at codeaurora.org]
Sent: Monday, October 01, 2012 12:26 PM
To: 'Michael Spencer'
Cc: 'llvm-commits at cs.uiuc.edu'; 'kledzik at apple.com'; 'sidneym at codeaurora.org'
Subject: RE: LLD patch - symbol table and reorganization of sections
Here is a revised patch as per comments.
--
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: Friday, September 28, 2012 4:41 PM
To: Hemant Kulkarni
Cc: llvm-commits at cs.uiuc.edu; kledzik at apple.com; sidneym at codeaurora.org
Subject: Re: LLD patch - symbol table and reorganization of sections
On Fri, Sep 28, 2012 at 12:00 PM, Hemant Kulkarni <khemant at codeaurora.org> wrote:
> Purpose of patch:
>
> Add symbol table to writer to emit symbols.
>
> Make all sections – ones made due to atoms and ones made by linker at
> output time to be derived from one class. Have this class all the
> attributes needed to make a section header. Use this class to build
> section table header and set all offsets/addresses.
>
>
>
> Added test for the same. The test needed to be changed to reflect
> correct order sections are now emitted; hence you might see change in
> existing test too.
>
> Please review the patch and send in comments.
>
>
>
> --
>
> Hemant Kulkarni
>
> khemant at codeaurora.org
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by the Linux Foundation
>
>
> Index: lib/ReaderWriter/ELF/WriterELF.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/WriterELF.cpp (revision 164787)
> +++ lib/ReaderWriter/ELF/WriterELF.cpp (working copy)
> @@ -35,6 +35,7 @@
> #include "llvm/Support/raw_ostream.h"
> #include "llvm/Support/system_error.h"
>
> +
> #include <map>
> #include <tuple>
> #include <vector>
> @@ -42,11 +43,14 @@
> using namespace llvm;
> using namespace llvm::object;
> namespace lld {
> +
> namespace elf {
>
> template<support::endianness target_endianness, bool is64Bits> class
> ELFWriter;
>
> +
> +
> /// \brief A Chunk is a contiguous range of space.
> template<support::endianness target_endianness, bool is64Bits> class
> Chunk { @@ -63,6 +67,8 @@
> uint64_t fileOffset() const;
> uint64_t align2() const;
> static uint64_t alignTo(uint64_t value, uint8_t align2);
> + uint64_t ordinal() const { return _ordinal;}
> + void setOrdinal(uint64_t newVal) { _ordinal = newVal;}
>
> protected:
> Chunk();
> @@ -71,43 +77,80 @@
> uint64_t _address;
> uint64_t _fileOffset;
> uint64_t _align2;
> + uint64_t _ordinal;
> };
>
> +
> +template<support::endianness target_endianness, bool is64Bits> static
> +void swapChunks(uint64_t one, uint64_t two,
> + std::vector<Chunk<target_endianness, is64Bits>*> &vec){
> + uint64_t tempOrdinal;
> + if (one == two) return;
> + tempOrdinal = vec[one]->ordinal();
> + vec[one]->setOrdinal(vec[two]->ordinal());
> + vec[two]->setOrdinal(tempOrdinal);
> +}
This doesn't need the vector. It should just take Chunk<target_endianness, is64Bits> &a, Chunk<target_endianness, is64Bits> &b.
Also, this doesn't exactly swap the chunks. A more accurate name would be swapChunkPosition.
> +
> /// Pair of atom and offset in section.
> typedef std::tuple<const DefinedAtom*, uint64_t> AtomInfo;
>
> -/// \brief A Section represents a set of Atoms assigned to a specific ELF
> -/// Section.
> +/// \brief A SectionChunk represents ELF sections
> template<support::endianness target_endianness, bool is64Bits> class
> SectionChunk : public Chunk<target_endianness, is64Bits> {
> public:
> - SectionChunk(DefinedAtom::ContentType,
> - StringRef sectionName,
> - const WriterOptionsELF &options,
> - 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 ArrayRef<AtomInfo> atoms() const;
> + virtual StringRef segmentName() const { return _segmentName; }
> + virtual bool occupiesNoDiskSpace();
> + virtual const char *info();
> + StringRef sectionName() { return _sectionName; }
> + uint64_t shStrtableOffset(){ return _offsetInStringTable; }
> + void setShStrtableOffset (uint64_t val)
> + { _offsetInStringTable = val; }
> + uint32_t flags() { return _flags; }
> + uint32_t type() { 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);
>
> -private:
> +protected:
> + bool _isLoadable;
> + uint64_t _link;
> + uint64_t _shinfo;
> + uint16_t _entsize;
> StringRef _segmentName;
> StringRef _sectionName;
> const WriterOptionsELF &_options;
> ELFWriter<target_endianness, is64Bits> &_writer;
> - uint32_t _flags;
> - uint32_t _type;
> - uint32_t _permissions;
> + uint64_t _flags;
> + uint64_t _type;
> + uint64_t _offsetInStringTable;
> +};
> +
> +
> +/// \brief A StockSectionChunk is a section created by linker with all attributes
> +/// concluded from the defined atom contained within.
> +template<support::endianness target_endianness, bool is64Bits> class
> +StockSectionChunk : public SectionChunk<target_endianness, is64Bits>
> +{
> +public:
> + virtual StringRef segmentName() { return this->_segmentName; }
> + void appendAtom(const DefinedAtom*);
> + virtual void write(uint8_t *filebuffer);
> + const ArrayRef<AtomInfo> atoms() const;
> + StockSectionChunk(StringRef sectionName, bool loadable,
> + DefinedAtom::ContentType type,
> + const WriterOptionsELF &options,
> + ELFWriter<target_endianness, is64Bits> &writer);
> +private:
> std::vector<AtomInfo> _atoms;
> };
> -
> +
> /// \brief An ELFHeaderChunk represents the Elf[32/64]_Ehdr structure at the
> /// start of an ELF executable file.
> template<support::endianness target_endianness, bool is64Bits> @@
> -134,7 +177,7 @@
> 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); }
> + uint64_t size() { return sizeof (Elf_Ehdr); }
>
> virtual StringRef segmentName() const;
> virtual void write(uint8_t *fileBuffer);
> @@ -158,6 +201,8 @@
> ELFSectionHeaderChunk(const WriterOptionsELF &Options,
> ELFWriter<target_endianness, is64Bits>&);
>
> + void createHeaders();
> +
> virtual StringRef segmentName() const;
> virtual void write(uint8_t *filebuffer);
> virtual const char *info();
> @@ -169,7 +214,6 @@
> return _sectionInfo;
> }
>
> - bool is64Bit() { return _options.is64Bit(); }
>
> private:
> const WriterOptionsELF &_options;
> @@ -184,32 +228,50 @@
> /// 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> {
> +class ELFStringSectionChunk : public SectionChunk<target_endianness,
> +is64Bits> {
> public:
> - LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
> ELFStringSectionChunk(const WriterOptionsELF &Options,
> ELFWriter<target_endianness, is64Bits> &writer,
> StringRef secName);
> + virtual StringRef segmentName() const { return this->_segmentName; }
> + uint64_t addString(StringRef symName);
> + const char *info();
> + virtual void write(uint8_t *filebuffer);
>
> - uint64_t addString(StringRef symName);
> +private:
> + std::vector<StringRef> _stringSection; };
>
> - virtual StringRef segmentName() const;
> - virtual void write(uint8_t *filebuffer);
> - virtual const char *info();
> - StringRef sectionName();
> +//
> +/// \brief Represents the symtab section /// /// ELFSymbolTableChunk
> +represents the Symbol table as per ELF ABI // This is a table with
> +Elf[32/64]_Sym entries in it.
> +//
Get rid of the extra // and "This is a table..." is missing a /.
> +template<support::endianness target_endianness, bool is64Bits> class
> +ELFSymbolTableChunk : public SectionChunk<target_endianness,
> +is64Bits> {
> +public:
> + typedef object::Elf_Sym_Impl<target_endianness, is64Bits> Elf_Sym;
> + ELFSymbolTableChunk(const WriterOptionsELF &options,
> + ELFWriter<target_endianness, is64Bits> &writer,
> + StringRef secName);
> + virtual StringRef segmentName() const { return this->_segmentName; }
> + void addSymbol(const Atom *a, uint16_t shndx);
> + void addSymbol(Elf_Sym* x)
> + { _symbolTable.push_back(x);
> + this->_size+= sizeof(Elf_Sym) ;}
Fix the indentation here.
> + const char *info();
> + void setAttributes();
>
> + virtual void write(uint8_t *fileBuffer);
>
>
> private:
> - StringRef _segName;
> - std::vector<StringRef> _StringSection;
> - StringRef _sectionName;
> - ELFWriter<target_endianness, is64Bits> &_writer;
> - const WriterOptionsELF &_options;
> -
> + std::vector<Elf_Sym*> _symbolTable; class
> + ELFStringSectionChunk<target_endianness, is64Bits> *_stringSection;
Drop class.
> + 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.
> @@ -239,6 +301,9 @@
> 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.
Comments should be indented to the same level as the code it describes.
> + static uint64_t orderNumber = 0;
> + _ordinal = orderNumber++;
> }
>
> template<support::endianness target_endianness, bool is64Bits> @@
> -308,86 +373,79 @@
>
> 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 = "PT_LOAD";
> - _sectionName = sectionName;
> - _flags = ELF::SHF_ALLOC | ELF::SHF_EXECINSTR;
> - _type = ELF::SHT_PROGBITS;
> - break;
> - case DefinedAtom::typeData:
> - _segmentName = "PT_LOAD";
> - _sectionName = sectionName;
> - _flags = ELF::SHF_ALLOC | ELF::SHF_WRITE;
> - _type = ELF::SHT_PROGBITS;
> - break;
> - case DefinedAtom::typeZeroFill:
> - _segmentName = "PT_LOAD";
> - _sectionName = sectionName;
> - _flags = ELF::SHF_ALLOC | ELF::SHF_WRITE;
> - _type = ELF::SHT_NOBITS;
> - break;
> - case DefinedAtom::typeConstant:
> - _segmentName = "PT_LOAD";
> - _sectionName = sectionName;
> - _flags = ELF::SHF_ALLOC;
> - _type = ELF::SHT_PROGBITS;
> - break;
> - default:
> - llvm_unreachable("Unhandled content type for section!");
> - }
> -}
> + 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)
> + : _isLoadable(loadable)
> + , _link(link)
> + , _shinfo(info)
> + , _entsize(entsz)
> + , _segmentName(segName)
> + , _sectionName(secName)
> + , _options(op)
> + , _writer(writer)
> + , _flags(flags)
> + , _type(type)
> + , _offsetInStringTable(0) {}
>
> template<support::endianness target_endianness, bool is64Bits> bool
> SectionChunk<target_endianness, is64Bits>::occupiesNoDiskSpace() {
> 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;
> +const char *SectionChunk<target_endianness, is64Bits>::info() {
> + return _sectionName.data();
> }
>
> -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;
> -}
> +//
> +//===----------------------------------------------------------------
> +------===//
> +// StockSectionChunk
> +//===----------------------------------------------------------------
> +------===//
> +//
>
> template<support::endianness target_endianness, bool is64Bits>
> -uint32_t SectionChunk<target_endianness, is64Bits>::permissions() {
> - return _permissions;
> +StockSectionChunk<target_endianness, is64Bits>::
> + StockSectionChunk(StringRef secName, bool loadable,
> + DefinedAtom::ContentType type,
> + const WriterOptionsELF &options,
> + ELFWriter<target_endianness, is64Bits> &writer)
> + : SectionChunk<target_endianness, is64Bits>(secName, StringRef("PT_NULL"),
> + loadable, 0lu, 0lu, 0u, 0lu, 0lu,
> + options, writer)
> + {
> +
> + this->_segmentName = this->_isLoadable ? "PT_LOAD" : "PT_NULL" ;
> + switch(type) {
> + case DefinedAtom::typeCode:
> + this->_type = ELF::SHT_PROGBITS;
> + break;
> + case DefinedAtom::typeData:
> + this->_type = ELF::SHT_PROGBITS;
> + break;
> + case DefinedAtom::typeZeroFill:
> + this->_type = ELF::SHT_NOBITS;
> + case DefinedAtom::typeConstant:
> + this->_type = ELF::SHT_PROGBITS;
One space after this->_type.
> + break;
> + default:
> + llvm_unreachable("Unhandled content type for section!"); }
> }
> +
>
> 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 ArrayRef<AtomInfo> SectionChunk<target_endianness, is64Bits>::
> +const ArrayRef<AtomInfo> StockSectionChunk<target_endianness, is64Bits>::
> atoms() const {
> return _atoms;
> }
>
>
> template<support::endianness target_endianness, bool is64Bits>
> -void SectionChunk<target_endianness, is64Bits>::
> +void StockSectionChunk<target_endianness, is64Bits>::
> appendAtom(const DefinedAtom *atom) {
> // Figure out offset for atom in this section given alignment constraints.
> uint64_t offset = this->_size;
> @@ -413,16 +471,16 @@
>
> // TODO: Check content permissions and figure out what to do with .bss
> if ((perms & DefinedAtom::permR__) == DefinedAtom::permR__)
> - this->_permissions |= ELF::SHF_ALLOC;
> + this->_flags |= ELF::SHF_ALLOC;
> if ((perms & DefinedAtom::permRW_) == DefinedAtom::permRW_)
> - this->_permissions |= (ELF::SHF_ALLOC | ELF::SHF_WRITE);
> + this->_flags |= (ELF::SHF_ALLOC | ELF::SHF_WRITE);
> if ((perms & DefinedAtom::permR_X) == DefinedAtom::permR_X)
> - this->_permissions |= (ELF::SHF_ALLOC | ELF::SHF_EXECINSTR);
> + this->_flags |= (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.
> +void StockSectionChunk<target_endianness, is64Bits>::write(uint8_t *chunkBuffer)
> +{ // Each section's content is just its atoms' content.
> for (const auto &ai : _atoms ) {
> // Copy raw content of atom to file buffer.
> ArrayRef<uint8_t> content = std::get<0>(ai)->rawContent();
> @@ -437,17 +495,18 @@
> uint64_t targetAddress = 0;
>
> if ( ref->target() != nullptr )
> - targetAddress = _writer.addressOfAtom(ref->target());
> + targetAddress = this->_writer.addressOfAtom(ref->target());
>
> - uint64_t fixupAddress = _writer.addressOfAtom(std::get<0>(ai)) + offset;
> - _writer.kindHandler()->applyFixup(ref->kind(), ref->addend(),
> + uint64_t fixupAddress = this->_writer.addressOfAtom(std::get<0>(ai)) +
> + offset;
> + this->_writer.kindHandler()->applyFixup(ref->kind(), ref->addend(),
> &atomContent[offset],
> fixupAddress,
> targetAddress);
Arguments should be lined up with the new call.
> }
> }
> }
> -//
> +
> //===----------------------------------------------------------------------===//
> // ELFStringSectionChunk
> //===----------------------------------------------------------------------===//
> @@ -456,54 +515,179 @@
> ELFStringSectionChunk(const WriterOptionsELF &options,
> ELFWriter<target_endianness, is64Bits> &writer,
> StringRef secName)
> - : _segName("PT_NULL")
> - , _sectionName(secName)
> - , _writer(writer)
> - , _options(options) {
> + : SectionChunk<target_endianness, is64Bits>(secName, StringRef("PT_NULL"),
const char * implicitly converts to StringRef.
> + false, 0lu, 0lu, 0lu,
> + ELF::SHT_STRTAB, 0lu, options,
> + writer) {
> // First Add a null character. It also occupies 1 byte
> - _StringSection.emplace_back("");
> + _stringSection.emplace_back("");
> this->_size = 1;
> }
>
> template<support::endianness target_endianness, bool is64Bits>
> uint64_t ELFStringSectionChunk<target_endianness, is64Bits>::
> addString(StringRef symName) {
> - _StringSection.emplace_back(symName);
> -
> +
No empty lines at the beginning of a function.
> + _stringSection.emplace_back(symName);
> uint64_t offset = this->_size;
> this->_size += symName.size() + 1;
>
> return offset;
> }
>
> +
> +// 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;
> +
> + for (auto it : _stringSection) {
> + ::memcpy(chunkBuffer + chunkOffset, it.data(), it.size());
> + chunkOffset += it.size();
> + ::memcpy(chunkBuffer + chunkOffset, "", 1);
> + chunkOffset += 1;
> + }
> +}
> +
> +template<support::endianness target_endianness, bool is64Bits>
> const char *ELFStringSectionChunk<target_endianness, is64Bits>::info() {
> - return _sectionName.data();
> + return "String Table";
> }
>
> +//===----------------------------------------------------------------------===//
> +// ELFSymbolTableChunk
> +//===----------------------------------------------------------------------===//
> +template< support::endianness target_endianness, bool is64Bits>
> +ELFSymbolTableChunk<target_endianness, is64Bits>::ELFSymbolTableChunk
> + (const WriterOptionsELF &options,
> + ELFWriter<target_endianness, is64Bits> &writer,
> + StringRef secName)
> + : SectionChunk<target_endianness, is64Bits>(secName, StringRef("PT_NULL"),
> + false, 0, 0, 0, ELF::SHT_SYMTAB,
> + sizeof(Elf_Sym), options, writer)
> + {
> + _stringSection = this->_writer.strtab();
> + Elf_Sym *symbol = new Elf_Sym;
Who owns this memory?
> + memset ((void *)symbol,0, sizeof(Elf_Sym));
Space after ,. No space after memset.
> + _symbolTable.push_back(symbol);
> + this->_link = 0;
> + this->_entsize = sizeof(Elf_Sym);
> + this->_size = (sizeof(Elf_Sym));
Extra ().
> +}
Indentation.
> +
> +/// \brief Add symbols to symbol table
> +// We examine each property of atom to infer the various st_* fields in Elf*_Sym
Missing /.
> +
No newline between comment and function.
> +template< support::endianness target_endianness, bool is64Bits>
> +void ELFSymbolTableChunk<target_endianness, is64Bits>::addSymbol(const Atom *a,
> + uint16_t shndx) {
A good way to reflow this would be:
void ELFSymbolTableChunk<target_endianness, is64Bits>
::addSymbol(const Atom *a, uint16_t shndx) {
> + Elf_Sym *symbol = new(_symbolAllocate.Allocate<Elf_Sym>()) Elf_Sym;
> + unsigned char b = 0 , t = 0;
No space before ,.
> + const DefinedAtom *da;
> + const AbsoluteAtom *aa;
> +
> + symbol->st_name = _stringSection->addString(a->name());
> +// In relocatable files, st_value holds a section offset for a defined symbol.
> +// st_value is an offset from the beginning of the section that st_shndx
> +// identifies. After we assign file offsets we can set this value correctly.
> + symbol->st_size = 0;
> + symbol->st_shndx = shndx;
> + symbol->st_value = 0;
> +// FIXME: Need to change and account all STV* when visibilities are supported
> + symbol->st_other = ELF::STV_DEFAULT;
> + if (a->definition() == lld::Atom::definitionRegular){
This should be replaced with:
if (const DefinedAtom *da = llvm::dyn_cast<const DefinedAtom>(a))
> + //da = static_cast<const DefinedAtom *>(a);
Don't leave in commented out code.
> + da = llvm::dyn_cast<const DefinedAtom>(a);
> + symbol->st_size = da->size();
> + lld::DefinedAtom::ContentType ct;
> + switch (ct = da->contentType()){
> + case DefinedAtom::typeCode:
> + t = ELF::STT_FUNC;
> + break;
> + case DefinedAtom::typeData:
Indentation.
> + t = ELF::STT_OBJECT;
> + break;
> + case DefinedAtom::typeZeroFill:
> +// In relocatable files, st_value holds alignment constraints for a symbol whose
> +// section index is SHN_COMMON
> + if (this->_options.type() == ELF::ET_REL){
> + t = ELF::STT_COMMON;
> + symbol->st_value = 0x1 << (da->alignment()).powerOf2;
Why 0x? Also uneeded ().
> + symbol->st_shndx = ELF::SHN_COMMON;
> + }
> + break;
> + case DefinedAtom::typeFirstInSection:
> + t = ELF::STT_SECTION;
> + break;
> +// TODO:: How to find STT_FILE symbols?
> + default:
> + t = ELF::STT_NOTYPE;
> + }
> +// TODO: Find out how to incorporate STB_HIOS STB_LOOS
> +// STB_HIPROC and STB_LOPROC
> +
> + if (da->scope() == DefinedAtom::scopeTranslationUnit)
> + b = ELF::STB_LOCAL;
> + else if (da->merge() == DefinedAtom::mergeAsWeak)
> + b= ELF::STB_WEAK;
> + else
> + b = ELF::STB_GLOBAL;
> + }
> +//FIXME: Absolute atoms need more properties to differentiate each other
> +// based on binding and type of symbol
> + else if (a->definition() == lld::Atom::definitionAbsolute){
> + aa = llvm::dyn_cast<const AbsoluteAtom>(a);
Same alternate if as above.
> + symbol->st_value = aa->value();
> + }
> +// We have an undefined atom
> + else {
No new lines between } and else.
> + symbol->st_value = 0;
> + t = ELF::STT_NOTYPE;
> + b = ELF::STB_LOCAL;
> + }
> +
> +// This will set the st_info field in Elf_Sym
> + symbol->setBindingAndType(b, t);
> + _symbolTable.push_back(symbol);
> + this->_size += sizeof(Elf_Sym);
> +
> +}
The indentation for the above function is very messed up.
> +
> +
> template<support::endianness target_endianness, bool is64Bits>
> -StringRef ELFStringSectionChunk<target_endianness, is64Bits>::sectionName() {
> - return _sectionName ;
> +void ELFSymbolTableChunk<target_endianness, is64Bits>::setAttributes() {
> +// sh_info should be one greater than last symbol with STB_LOCAL binding
> +// we sort the symbol table to keep all local symbols at the beginning
> + std::stable_sort(_symbolTable.begin(), _symbolTable.end(),
> + static_cast<bool(*)(const Elf_Sym*, const Elf_Sym*)>([]
Undeeded static_cast. This was used elsewhere due to a bug with VS2010, which we
no longer support.
> + (const Elf_Sym *A, const Elf_Sym *B) -> bool {
> + return (A->getBinding() < B->getBinding());}));
> + uint16_t shInfo = 0;
> + for (auto i : _symbolTable) {
> + if (i->getBinding() != ELF::STB_LOCAL)
Indentation.
> + break;
> + shInfo++;
> + }
> + this->_shinfo = shInfo;
> +// we set the associated string table index in th sh_link member
> + this->_link = (this->_writer.strtab())->ordinal() - 1;
Undeeded ().
> + this->_align2 = sizeof(void*);
This seems wrong to me. The output of the linker should not depend on the
platform it is running on.
> }
>
> template<support::endianness target_endianness, bool is64Bits>
> -StringRef ELFStringSectionChunk<target_endianness, is64Bits>::
> - segmentName() const {
> - return _segName;
> +const char *ELFSymbolTableChunk<target_endianness, is64Bits>::info() {
> + return "Symbol Table";
> }
>
> -// 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>::
> +void ELFSymbolTableChunk<target_endianness, is64Bits>::
> write(uint8_t *chunkBuffer) {
> uint64_t chunkOffset = 0;
> -
> - for (auto it : _StringSection) {
> - ::memcpy(chunkBuffer + chunkOffset, it.data(), it.size());
> - chunkOffset += it.size();
> - ::memcpy(chunkBuffer + chunkOffset, "", 1);
> - chunkOffset += 1;
> + for (auto it : _symbolTable) {
> + ::memcpy(chunkBuffer + chunkOffset, it, this->_entsize);
> + chunkOffset += this->_entsize;
> }
> }
>
> @@ -573,7 +757,6 @@
> is64Bits> &writer)
> : _options(options)
> , _writer(writer) {
> -
> this->_size = 0;
> this->_align2 = 0;
> // The first element in the list is always NULL
> @@ -582,14 +765,20 @@
> _sectionInfo.push_back(nullshdr);
>
> this->_size += sizeof (Elf_Shdr);
> + }
>
> +template<support::endianness target_endianness, bool is64Bits>
> +void ELFSectionHeaderChunk<target_endianness, is64Bits>::createHeaders(){
> ELFStringSectionChunk<target_endianness, is64Bits> *str = _writer.shstrtab();
>
> - for (const auto &chunk : _writer.sectionChunks()) {
> + for (const auto &chunk : _writer.sectionChunks()) {
This shouldn't have been changed.
> Elf_Shdr *shdr = new (_sectionAllocate.Allocate<Elf_Shdr>()) Elf_Shdr;
> StringRef Name = chunk->sectionName();
> - uint64_t offset = str->addString(Name);
> - shdr->sh_name = offset;
> + if (chunk->shStrtableOffset() == 0){
> + chunk->setShStrtableOffset(str->addString(Name));
Indentation.
> + }
> + shdr->sh_name = chunk->shStrtableOffset();
> +
> shdr->sh_type = chunk->type();
> shdr->sh_flags = chunk->flags();
> // TODO: At the time of creation of this section header, we will not have
> @@ -627,43 +816,24 @@
> // 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_link = chunk->link() ;
> + shdr->sh_info = chunk->shinfo();
> shdr->sh_addralign = chunk->align2();
> - // Not a special section with fixed entries
> - shdr->sh_entsize = 0;
> + shdr->sh_entsize = chunk->entsize();
>
> _sectionInfo.push_back(shdr);
> this->_size += sizeof (Elf_Shdr);
> +// We set the attributes of symbol table as mentioned in above comment
> + (_writer.symtab())->setAttributes();
Undeeded ().
> }
> -
> - // Now I add in the section string table. For some reason This seems to be
> - // preferred location of string sections in contemporary
> - // (ones that must not be named) linker(s).
> - Elf_Shdr *shdr = new (_sectionAllocate.Allocate<Elf_Shdr>()) Elf_Shdr;
> - // 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);
> }
>
> +
> +
> template<support::endianness target_endianness, bool is64Bits>
> StringRef ELFSectionHeaderChunk<target_endianness, is64Bits>
> ::segmentName() const {
> - return "SHDR";
> + return StringRef("PT_NULL");
Uneeded StringRef.
> }
>
> template<support::endianness target_endianness, bool is64Bits>
> @@ -702,6 +872,7 @@
> public:
> 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;
> ELFWriter(const WriterOptionsELF &options);
> virtual error_code writeFile(const lld::File &File, StringRef path);
> uint64_t addressOfAtom(const Atom *atom);
> @@ -715,7 +886,15 @@
> ELFStringSectionChunk<target_endianness, is64Bits> *shstrtab() {
> return _shstrtable;
> }
> +
> + ELFStringSectionChunk<target_endianness, is64Bits> *strtab() {
> + return _strtable;
> + }
> + ELFSymbolTableChunk<target_endianness, is64Bits> *symtab() {
> + return _symtable;
> + }
>
> +
> private:
> void build(const lld::File &file);
> void createChunks(const lld::File &file);
> @@ -728,12 +907,16 @@
> typedef llvm::DenseMap<const Atom*, uint64_t> AtomToAddress;
>
> ELFStringSectionChunk<target_endianness, is64Bits> *_shstrtable ;
> + ELFStringSectionChunk<target_endianness, is64Bits> *_strtable ;
> + ELFSymbolTableChunk<target_endianness, is64Bits> *_symtable;
> std::unique_ptr<KindHandler> _referenceKindHandler;
> ELFSectionHeaderChunk<target_endianness, is64Bits> *_sectionHeaderChunk;
> AtomToAddress _atomToAddress;
> std::vector<Chunk<target_endianness, is64Bits>*> _chunks;
> const DefinedAtom *_entryAtom;
> std::vector<SectionChunk<target_endianness, is64Bits>*> _sectionChunks;
> + std::vector<StockSectionChunk<target_endianness, is64Bits>*>
> + _stockSectionChunks;
> llvm::BumpPtrAllocator _chunkAllocate;
> };
>
> @@ -758,8 +941,21 @@
> template<support::endianness target_endianness, bool is64Bits>
> void ELFWriter<target_endianness, is64Bits>
> ::createChunks (const lld::File &file) {
> - std::map<StringRef, SectionChunk<target_endianness, is64Bits>*> sectionMap;
> + std::map<StringRef, StockSectionChunk<target_endianness, is64Bits>*>
> + sectionMap;
>
> + // Make header chunk
> + ELFHeaderChunk<target_endianness, is64Bits> *ehc =
> + new (_chunkAllocate.Allocate
> + <ELFHeaderChunk<target_endianness, is64Bits>>())
> + ELFHeaderChunk<target_endianness, is64Bits>(_options, file);
> + _chunks.push_back(ehc);
> +
> + _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.
> @@ -768,58 +964,128 @@
> <ELFStringSectionChunk<target_endianness, is64Bits>>())
> ELFStringSectionChunk<target_endianness, is64Bits>
> (_options, *this, ".shstrtab");
> - _shstrtable->addString(".shstrtab");
> + _shstrtable->setShStrtableOffset(_shstrtable->addString(".shstrtab"));
> + _sectionChunks.push_back(_shstrtable);
> +
> + _strtable = new (_chunkAllocate.Allocate
> + <ELFStringSectionChunk<target_endianness, is64Bits>>())
> + ELFStringSectionChunk<target_endianness, is64Bits>
> + (_options, *this, ".strtab");
> + _strtable->setShStrtableOffset( _shstrtable->addString(".strtab"));
> + _sectionChunks.push_back(_strtable);
> +
> + _symtable = new (_chunkAllocate.Allocate
> + <ELFSymbolTableChunk<target_endianness, is64Bits>>())
> + ELFSymbolTableChunk<target_endianness, is64Bits>
> + (_options, *this, ".symtab");
> + _symtable->setShStrtableOffset( _shstrtable->addString(".symtab"));
> + _sectionChunks.push_back(_symtable);
>
> - //we also need to process undefined atoms
> +//TODO: implement .hash section
> +
> for (const DefinedAtom *a : file.defined() ) {
> // TODO: Add sectionChoice.
> // assert( atom->sectionChoice() == DefinedAtom::sectionBasedOnContent );
> StringRef sectionName = a->customSectionName();
> + if (a->sectionChoice() ==
> + DefinedAtom::SectionChoice::sectionBasedOnContent) {
> + if (a->size() <8)
> + sectionName = ".sbss";
> + else
> + sectionName = ".bss";
> + }
> auto pos = sectionMap.find(sectionName);
> DefinedAtom::ContentType type = a->contentType();
> - if (pos == sectionMap.end()) {
> - if (type != DefinedAtom::typeUnknown){
> - SectionChunk<target_endianness, is64Bits>
> - *chunk = new (_chunkAllocate.Allocate
> - <SectionChunk<target_endianness, is64Bits>>())
> - SectionChunk<target_endianness, is64Bits>
> - (type, sectionName, _options, *this);
> + if (type != DefinedAtom::typeUnknown){
> + if (pos == sectionMap.end()) {
> + StockSectionChunk<target_endianness, is64Bits>
> + *chunk = new(_chunkAllocate.Allocate
> + <StockSectionChunk<target_endianness, is64Bits>>
> + ())StockSectionChunk<target_endianness, is64Bits>
> + (sectionName, true, type, _options, *this);
>
> sectionMap[sectionName] = chunk;
> chunk->appendAtom(a);
> _sectionChunks.push_back(chunk);
> + _stockSectionChunks.push_back(chunk);
> +
> + } else {
> + pos->second->appendAtom(a);
> }
> - } else {
> - pos->second->appendAtom(a);
> }
> }
>
> - //put in the Undefined atoms as well
> - // Make header chunk
> - ELFHeaderChunk<target_endianness, is64Bits> *ehc =
> - new (_chunkAllocate.Allocate
> - <ELFHeaderChunk<target_endianness, is64Bits>>())
> - ELFHeaderChunk<target_endianness, is64Bits>(_options, file);
>
> - _sectionHeaderChunk = new (_chunkAllocate.Allocate<ELFSectionHeaderChunk
> - <target_endianness, is64Bits>>())
> - ELFSectionHeaderChunk
> - <target_endianness, is64Bits>(_options, *this);
> + 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.
> +
> + swapChunks(reinterpret_cast<uint64_t>(_chunks.size()) - 1,
> + _shstrtable->ordinal(), _chunks);
> + swapChunks(reinterpret_cast<uint64_t>(_chunks.size()) - 2,
> + _strtable->ordinal(), _chunks);
> + swapChunks(reinterpret_cast<uint64_t>(_chunks.size()) - 3,
> + _symtable->ordinal(), _chunks);
> +
> +// 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
> +
> + std::stable_sort(_chunks.begin(), _chunks.end(),
> + static_cast<bool(*)(const Chunk<target_endianness, is64Bits>*,
> + const Chunk<target_endianness, is64Bits>*)>([]
> + (const Chunk<target_endianness, is64Bits> *A,
> + const Chunk<target_endianness, is64Bits> *B) -> bool {
> + return (A->ordinal() < B->ordinal());}));
> +
> + std::stable_sort(_sectionChunks.begin(), _sectionChunks.end(),
> + static_cast<bool(*)(const SectionChunk<target_endianness, is64Bits>*,
> + const SectionChunk<target_endianness, is64Bits>*)>([]
> + (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);
> + }
> +
> +for (const auto ssc : _stockSectionChunks){
> + for (const auto da : ssc->atoms()) {
Indentation.
> + _symtable->addSymbol(std::get<0>(da),ssc->ordinal() -1);
Space after ,.
> + }
> +}
> + for (const UndefinedAtom *a : file.undefined()) {
> + _symtable->addSymbol(a, ELF::SHN_UNDEF);
> + }
> +
> + 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());
> -
> - // I am pushing string section after all sections are in.
> - // 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);
> +// 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>
> @@ -828,13 +1094,11 @@
>
> // _atomToAddress is a DenseMap that maps an atom its file address.
> // std::get<1>(ai) is the offset from the start of the section to the atom.
> - for (auto &chunk : _sectionChunks){
> + for (auto chunk : _stockSectionChunks){
> for (auto &ai : chunk->atoms() ) {
> _atomToAddress[std::get<0>(ai)] = chunk->address() + std::get<1>(ai);
> }
> }
> -
> -
> }
>
> template<support::endianness target_endianness, bool is64Bits>
> @@ -859,11 +1123,6 @@
> (*it)->sh_addr = chunk->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>
> @@ -895,7 +1154,7 @@
>
> Writer *createWriterELF(const WriterOptionsELF &options) {
> if (!options.is64Bit() && options.endianness() == llvm::support::little)
> - return new lld::elf::ELFWriter<support::little, false>(options);
> + return new lld::elf::ELFWriter<support::little, false>(options);
> else if (options.is64Bit() && options.endianness() == llvm::support::little)
> return new lld::elf::ELFWriter<support::little, true>(options);
> else if (!options.is64Bit() && options.endianness() == llvm::support::big)
> Index: test/elf/sections.objtxt
> ===================================================================
> --- test/elf/sections.objtxt (revision 164035)
> +++ test/elf/sections.objtxt (working copy)
> @@ -3,20 +3,27 @@
> RUN: elf-dump --dump-section %t1 | FileCheck -check-prefix=ED %s
>
> OBJDUMP: 0 000000000 00000000000000000
> -OBJDUMP: 1 .text 00000000a 0000000000000014c TEXT DATA
> -OBJDUMP: 2 .data 000000004 00000000000000158 DATA
> -OBJDUMP: 3 .special 000000004 0000000000000015c DATA
> -OBJDUMP: 4 .anotherspecial 000000004 00000000000000160 DATA
> -OBJDUMP: 5 .bss 000000000 00000000000000164 BSS
> -OBJDUMP: 6 .shstrtab 000000035 00000000000000164
> +OBJDUMP: 1 .sbss 000000000 0000000000000019c
> +OBJDUMP: 2 .anotherspecial 000000004 000000000000001a0 DATA
> +OBJDUMP: 3 .special 000000004 000000000000001a4 DATA
> +OBJDUMP: 4 .text 00000000a 000000000000001a8 TEXT DATA
> +OBJDUMP: 5 .data 000000004 000000000000001b4 DATA
> +OBJDUMP: 6 .symtab 0000000f0 000000000000001b8
> +OBJDUMP: 7 .strtab 000000011 000000000000002a8
> +OBJDUMP: 8 .shstrtab 000000046 000000000000002b9
>
> READOBJ: File Format : ELF32-i386
> READOBJ: Arch : i386
> READOBJ: Address Size: 32 bits
> +READOBJ: Symbols
> +READOBJ: .anotherspecial DBG 1a0 0 1a0 formatspecific
> +READOBJ: .symtab DBG 1b8 0 1b8 formatspecific
> +READOBJ: baz FUNC 1a8 a 1a8 global
> +READOBJ: z DATA 1a0 4 1a0 global
>
> ED: 'e_indent[EI_DATA]', 0x01
> ED: 'e_machine', 0x0003
> ED: Section 1
> -ED: 'sh_addralign', 0x00000004
> +ED: 'sh_addralign', 0x00000002
> ED: Section 2
> -ED: 'sh_addralign', 0x00000004
> +ED: 'sh_addralign', 0x00000008
>
- Michael Spencer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: symtable-rev1.diff
Type: application/octet-stream
Size: 42045 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121001/0aa500fc/attachment.obj>
More information about the llvm-commits
mailing list