[llvm-commits] [PATCH] [lld] patch for ELF Writer to get it to 'Hello world'
Shankar Easwaran
shankare at codeaurora.org
Fri Dec 21 21:11:29 PST 2012
Done the changes as specified by your review comments.
OK to commit ?
Thanks
Shankar Easwaran
On 12/21/2012 9:54 PM, Michael Spencer wrote:
> On Fri, Dec 21, 2012 at 2:23 PM, Shankar Easwaran
> <shankare at codeaurora.org> wrote:
>> Hi Michael,
>>
>> Attached is the new diff after the review with the test case changes.
>>
>> You would need to apply the other changes that were in the previous patches
>> to build.
>>
>> a) ELFAtoms.h
>> b) ReaderELF changes
>> c) HexagonRelocation changes
>>
>> OK to commit ?
>>
>> Thanks
>>
>> Shankar Easwaran
>>
>>
>> On 12/21/2012 11:11 AM, Michael Spencer wrote:
>>>
>>> ================
>>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:80-86
>>> @@ +79,9 @@
>>> + /// \brief Describes the type of Chunk
>>> + enum Kind {
>>> + ELFHeader, // ELF Header
>>> + ProgramHeader, // Program Header
>>> + Segment, // Segment
>>> + Section, // Section
>>> + SectionHeader // Section header
>>> + };
>>> + Chunk(StringRef name, Kind kind) : _name(name),
>>> ----------------
>>> Shankar Kalpathi Easwaran wrote:
>>>> Michael Spencer wrote:
>>>>> This either needs to be an enum class, or the members need to follow the
>>>>> naming convention. Currently this breaks MSVC because it doesn't do two
>>>>> phase name lookup and therefore sees Section and Segment in the derived
>>>>> class (Chunk) instead of the templates Section and Segment.
>>>> Changed the names to add ELF in the front
>>>
>>> http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
>>>
>>> They should start with K_
>>>
>>> ================
>>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:121-123
>>> @@ +120,5 @@
>>> + int64_t fileOffset() const { return _fileoffset; }
>>> + void setFileOffset(int64_t offset) { _fileoffset =
>>> offset; }
>>> + // Output start address of the chunk
>>> + void setStart(int64_t start) { _start = start; }
>>> + int64_t startAddr() const { return _start; }
>>> ----------------
>>> Shankar Kalpathi Easwaran wrote:
>>>> Michael Spencer wrote:
>>>>> What is the difference between these two function? Is one the virtual
>>>>> address?
>>>> yes.
>>> Then the names could be improved and a \brief added.
>>>
>>> ================
>>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:549
>>> @@ -235,1 +548,3 @@
>>> + void setOrdinal(int64_t ordinal) {
>>> + ordinal = ordinal;
>>> }
>>> ----------------
>>> Shankar Kalpathi Easwaran wrote:
>>>> Michael Spencer wrote:
>>>>> _ordinal = ordinal;
>>>> Nice catch.
>>> Clang -Wall caught it for me :P
>>>
>>> ================
>>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:1008
>>> @@ +1007,3 @@
>>> + static inline bool classof(const Chunk<target_endianness, is64Bits> *c)
>>> {
>>> + return c->kind() == Section<target_endianness,
>>> is64Bits>::StringTable;
>>> + }
>>> ----------------
>>> Shankar Kalpathi Easwaran wrote:
>>>> Michael Spencer wrote:
>>>>> This is broken. Chunk already has a Kind with the same value.
>>>> Chunk doesnot have such a type called StringTable.
>>> Not named StringTable. But StringTable shares the same underlying value as
>>> Chunk::Segment
>>>
>>> ================
>>> Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:1491-1502
>>> @@ -826,1 +1490,14 @@
>>> + case DefinedAtom::typeCode:
>>> + if (name.startswith(".eh_frame_hdr"))
>>> + return ORDER_EH_FRAMEHDR;
>>> + if (name.startswith(".eh_frame"))
>>> + return ORDER_EH_FRAME;
>>> + else if (name.startswith(".init"))
>>> + return ORDER_INIT;
>>> + else if (name.startswith(".fini"))
>>> + return ORDER_FINI;
>>> + else if (name.startswith(".hash"))
>>> + return ORDER_HASH;
>>> + else
>>> + return ORDER_TEXT;
>>> ----------------
>>> Shankar Kalpathi Easwaran wrote:
>>>> Michael Spencer wrote:
>>>>> llvm::StringSwitch in llvm/ADT/StringSwitch.h
>>>> How do you do startswith with StringSwitch ?
>>> .StartsWith("") instead of .Case("").
>>>
>>>
>>> https://github.com/Bigcheese/llvm/blob/master/lib/Object/COFFObjectFile.cpp#L214
>>>
>>>
>>> http://llvm-reviews.chandlerc.com/D222
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
>> the Linux Foundation
>>
>> Index: WriterELF.cpp
>> ===================================================================
>> --- WriterELF.cpp (revision 170923)
>> +++ WriterELF.cpp (working copy)
>> @@ -11,20 +11,20 @@
>> #include "ReferenceKinds.h"
>>
>> #include "lld/Core/DefinedAtom.h"
>> +#include "llvm/Object/ELF.h"
> This needs to be sorted. It should come right before the Support headers.
>
>> #include "lld/Core/File.h"
>> #include "lld/Core/InputFiles.h"
>> #include "lld/Core/Reference.h"
>> #include "lld/Core/SharedLibraryAtom.h"
>> -
>> #include "llvm/ADT/ArrayRef.h"
>> #include "llvm/ADT/DenseMap.h"
>> +#include "llvm/ADT/Hashing.h"
>> #include "llvm/ADT/OwningPtr.h"
>> #include "llvm/ADT/SmallVector.h"
>> +#include "llvm/ADT/StringExtras.h"
>> #include "llvm/ADT/StringMap.h"
>> #include "llvm/ADT/StringRef.h"
>> -
>> -#include "llvm/Object/ELF.h"
>> -
>> +#include "llvm/ADT/StringSwitch.h"
>> #include "llvm/Support/Allocator.h"
>> #include "llvm/Support/Debug.h"
>> #include "llvm/Support/ELF.h"
>> @@ -34,1382 +34,2020 @@
>> #include "llvm/Support/MathExtras.h"
>> #include "llvm/Support/raw_ostream.h"
>> #include "llvm/Support/system_error.h"
>> +#include "ELFAtoms.h"
>>
>> #include <map>
>> +#include <unordered_map>
>> #include <tuple>
>> #include <vector>
>> -#include <algorithm>
>>
>> using namespace llvm;
>> using namespace llvm::object;
>> namespace lld {
>> namespace elf {
>>
>> -// The group decides where sections of similar permissions reside
>> -// inside a file. This is also used to create program headers. Each group
>> -// will be in a new segment.
>> +template<support::endianness target_endianness, bool is64Bits>
>> +class ELFExecutableWriter;
>>
>> -/// \name Chunk Groups
>> -/// Each "Chunk" should be arranged in a weak strict order
>> -/// This is done to minimize memory utilization when creating program segments.
>> -/// This will remain in place till a more felxible mechanism of moving chunks
>> -/// around and packing for memory efficiency is put in place.
>> -/// The Chunks are arranged by group numbers. Every switch from group A to B
>> -/// triggers a formation of new segment. Additionally segments are also made
>> -/// within a group if there are vast regions (multiple pages) of 0 for
>> -/// alignment constraints.
>> -enum {
>> -/// @{
>> -///Invalid group (default on creation)
>> - CG_INVALID = 0x0,
>> -///This is a "header" kind of chunk that goes at beginning of ELF file
>> - CG_HEADER = 0x1,
>> -///This is a section chunk with read write and execute permissions
>> - CG_RWX = 0x3,
>> -///This is a section chunk with read and execute permissions
>> - CG_RX = 0x4,
>> -///This is a section chunk with read permission
>> - CG_R = 0x5,
>> -///This is a section chunk with read and write permissions
>> - CG_RW = 0x6,
>> -///This is a section chunk with write and execute permissions
>> - CG_WX = 0x7,
>> -///This is a section chunk with write permission
>> - CG_W = 0x8,
>> -///This is a section chunk with execute permission
>> - CG_X = 0x9,
>> -///This is a section which is no loaded by program loader
>> - CG_NO_LOAD = 0xFE,
>> -///This is ELF file metadata that goes at end of file
>> - CG_FILE_END = 0xFF
>> -/// @}
>> -};
>> +/// \brief The ELFWriter class is a base class for the linker to write
>> +/// various kinds of ELF files.
>> +class ELFWriter : public Writer {
>> + public:
> This shouldn't be indented.
>
>> + ELFWriter() { }
>>
>> -template<support::endianness target_endianness, bool is64Bits>
>> -class ELFWriter;
>> + public:
>> + /// \brief builds the chunks that needs to be written to the output
>> + /// ELF file
>> + virtual void buildChunks(const lld::File &file) = 0;
>>
>> -template<support::endianness target_endianness, bool is64Bits>
>> -class StockSectionChunk;
>> + /// \brief Writes the chunks into the output file specified by path
>> + virtual error_code writeFile(const lld::File &File, StringRef path) = 0;
>>
>> -/// \brief A Chunk is a contiguous range of space.
>> + /// \brief Writes the chunks into the output file specified by path
>> + virtual int64_t addressOfAtom(const Atom *atom) = 0;
>> +
>> + /// \brief Return the processing function to apply Relocations
>> + virtual KindHandler *kindHandler() = 0;
>> +};
>> +
>> +/// \brief A chunk is a contiguous region of space
>> template<support::endianness target_endianness, bool is64Bits>
>> class Chunk {
>> public:
>> - LLVM_ELF_IMPORT_TYPES(target_endianness, is64Bits)
>> +
>> + /// \brief Describes the type of Chunk
>> + enum Kind {
>> + K_ELFHeader, // ELF Header
>> + K_ELFProgramHeader, // Program Header
>> + K_ELFSegment, // Segment
>> + K_ELFSection, // Section
>> + K_ELFSectionHeader // Section header
>> + };
>> + Chunk(StringRef name, Kind kind)
>> + : _name(name)
>> + , _kind(kind)
>> + , _fsize(0)
>> + , _msize(0)
>> + , _align2(0)
>> + , _order(0)
>> + , _ordinal(1)
>> + , _start(0)
>> + , _fileoffset(0) {}
>> virtual ~Chunk() {}
>> - virtual StringRef segmentName() const = 0;
>> - virtual bool occupiesNoDiskSpace();
>> - virtual void write(uint8_t *fileBuffer) = 0;
>> - void assignFileOffset(uint64_t &curOff, uint64_t &curAddr);
>> - virtual const char *info() = 0;
>> - uint64_t size() const;
>> - uint64_t address() const;
>> - uint64_t fileOffset() const;
>> - uint64_t align2() const;
>> - static uint64_t alignTo(uint64_t value, uint8_t align2);
>> - uint64_t ordinal() const { return _ordinal;}
>> - void setOrdinal(uint64_t newVal) { _ordinal = newVal;}
>> - void setGroup(uint16_t val) { _group = val;}
>> - uint16_t group() const { return _group;}
>> - void init();
>> - bool isLoadable() { return _isLoadable; }
>> - void isLoadable(uint64_t val) { _isLoadable = val; }
>> - enum class Kind {
>> - Header, // This is a header chunk
>> - Section // chunk represents a section
>> - };
>> - Kind getChunkKind() const { return _cKind; }
>> -private:
>> - const Kind _cKind;
>> + // Does the chunk occupy disk space
>> + virtual bool occupiesNoDiskSpace() const {
>> + return false;
>> + }
>> + // The name of the chunk
>> + StringRef name() const { return _name; }
>> + // Kind of chunk
>> + Kind kind() const { return _kind; }
>> + int64_t fileSize() const { return _fsize; }
>> + int64_t align2() const { return _align2; }
>> + void appendAtom() const;
>> +
>> + // The ordinal value of the chunk
>> + int64_t ordinal() const { return _ordinal;}
>> + void setOrdinal(int64_t newVal) { _ordinal = newVal;}
>> + // The order in which the chunk would appear in the output file
>> + int64_t order() const { return _order; }
>> + void setOrder(int32_t order) { _order = order; }
>> + // Output file offset of the chunk
>> + int64_t fileOffset() const { return _fileoffset; }
>> + void setFileOffset(int64_t offset) { _fileoffset = offset; }
>> + // Output start address of the chunk
>> + void setVAddr(int64_t start) { _start = start; }
>> + int64_t virtualAddr() const { return _start; }
>> + // Does the chunk occupy memory during execution ?
>> + int64_t memSize() const { return _msize; }
>> + void setMemSize(int64_t msize) { _msize = msize; }
>> + // Writer the chunk
>> + virtual void write(ELFWriter *writer,
>> + OwningPtr<FileOutputBuffer> &buffer) = 0;
>> + // Finalize the chunk before writing
>> + virtual void finalize() = 0;
>> +
>> protected:
>> - Chunk();
>> - Chunk(Kind K): _cKind(K) { this->init();}
>> - uint64_t _size;
>> - uint64_t _address;
>> - uint64_t _fileOffset;
>> - uint64_t _align2;
>> - uint64_t _ordinal;
>> - uint16_t _group;
>> - bool _isLoadable;
>> + StringRef _name;
>> + Kind _kind;
>> + int64_t _fsize;
>> + int64_t _msize;
>> + int64_t _align2;
>> + int32_t _order;
>> + int64_t _ordinal;
>> + int64_t _start;
>> + int64_t _fileoffset;
> These should all be unsigned.
>
>> };
>>
>> -template<support::endianness target_endianness, bool is64Bits>
>> -static void swapChunkPositions(Chunk<target_endianness, is64Bits>&a,
>> - Chunk<target_endianness, is64Bits>&b) {
>> - uint64_t tempOrdinal;
>> - if (a.ordinal() == b.ordinal()) return;
>> - tempOrdinal = a.ordinal();
>> - a.setOrdinal(b.ordinal());
>> - b.setOrdinal(tempOrdinal);
>> -}
>> +/// \brief The ELFLayoutOptions encapsulates the options used by all Layouts
>> +/// Examples of the ELFLayoutOptions would be a script that would be used
>> +/// to drive the layout
>> +class ELFLayoutOptions {
>> +public:
>> + ELFLayoutOptions() { }
>>
>> -template<support::endianness target_endianness, bool is64Bits>
>> -bool chunkGroupSort(Chunk<target_endianness, is64Bits> *A,
>> - Chunk<target_endianness, is64Bits> *B) {
>> - if (A->group() == CG_INVALID || B->group() == CG_INVALID)
>> - llvm_unreachable("Invalid group number");
>> - return A->group() < B->group();
>> -}
>> + ELFLayoutOptions(StringRef &linker_script) : _script(linker_script)
>> + {}
>>
>> + /// parse the linker script
>> + error_code parseLinkerScript();
>> +
>> + /// Is the current section present in the linker script
>> + bool isSectionPresent();
>> +
>> +private:
>> + StringRef _script;
>> +};
>> +
>> +/// \brief The ELFLayout is an abstract class for managing the final layout for
>> +/// the kind of binaries(Shared Libraries / Relocatables / Executables 0
>> +/// Each architecture (Hexagon, PowerPC, MIPS) would have a concrete
>> +/// subclass derived from ELFLayout for generating each binary thats
>> +// needed by the lld linker
>> +class ELFLayout {
>> +public:
>> + typedef int32_t SectionOrder;
>> + typedef int32_t SegmentType;
>> + typedef int32_t Flags;
> Unsigned.
>
>> +
>> +public:
>> + /// Return the order the section would appear in the output file
>> + virtual SectionOrder getSectionOrder
>> + (const StringRef name,
>> + int32_t contentType,
>> + int32_t contentPerm) = 0;
>> + /// append the Atom to the layout and create appropriate sections
>> + virtual error_code addAtom(const Atom *atom) = 0;
>> + /// find the Atom Address in the current layout
>> + virtual bool findAtomAddrByName(const StringRef name, int64_t &addr) = 0;
>> + /// associates a section to a segment
>> + virtual void assignSectionsToSegments() = 0;
>> + /// associates a virtual address to the segment, section, and the atom
>> + virtual void assignVirtualAddress() = 0;
>> + /// associates a file offset to the segment, section and the atom
>> + virtual void assignFileOffsets() = 0;
>> +
>> +public:
>> + ELFLayout() {}
>> + ELFLayout(WriterOptionsELF &writerOptions,
>> + ELFLayoutOptions &layoutOptions)
>> + : _writerOptions(writerOptions)
>> + , _layoutOptions(layoutOptions) {}
>> + virtual ~ELFLayout() { }
>> +
>> +private:
>> + WriterOptionsELF _writerOptions;
>> + ELFLayoutOptions _layoutOptions;
>> +};
>> +
>> +
>> +/// \brief A section contains a set of atoms that have similiar properties
>> +/// The atoms that have similiar properties are merged to form a section
>> template<support::endianness target_endianness, bool is64Bits>
>> -struct ChunkComparator {
>> - bool operator()(uint16_t A, Chunk<target_endianness, is64Bits> *B) {
>> - return A < B->group();
>> +class Section : public Chunk<target_endianness, is64Bits> {
>> +public:
>> + // The Kind of section that the object represents
>> + enum Kind {
>> + K_Default = 0x100,
>> + K_SymbolTable,
>> + K_StringTable,
>> + };
> I see now that this isn't the using the same kind field as Chunk is.
> So no need for the 0x100. Although it does hide Chunk::Kind, so it
> needs a different name.
>
>> + // Create a section object, the section is set to the default type if the
>> + // caller doesnot set it
>> + Section(const StringRef sectionName,
>> + const int32_t contentType,
>> + const int32_t contentPermissions,
>> + const int32_t order,
>> + const Kind kind = K_Default)
>> + : Chunk<target_endianness, is64Bits>
>> + (sectionName, Chunk<target_endianness, is64Bits>::K_ELFSection)
>> + , _contentType(contentType)
>> + , _contentPermissions(contentPermissions)
>> + , _sectionKind(kind)
>> + , _entSize(0)
>> + , _shInfo(0)
>> + , _link(0) {
>> + this->setOrder(order);
>> }
>> - bool operator()(Chunk<target_endianness, is64Bits> *A, uint16_t B) {
>> - return A->group() < B;
>> +
>> + /// return the section kind
>> + Kind sectionKind() const {
>> + return _sectionKind;
>> }
>> -#if defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL == 2
>> - bool operator()(Chunk<target_endianness, is64Bits> *A,
>> - Chunk<target_endianness, is64Bits> *B) {
>> - return A->group() < B->group();
>> - }
>> -#endif
>> -};
>>
>> -/// Pair of atom and offset in section.
>> -typedef std::tuple<const DefinedAtom*, uint64_t> AtomInfo;
>> + /// Align the offset to the required modulus defined by the atom alignment
>> + uint64_t alignOffset(uint64_t offset, DefinedAtom::Alignment &atomAlign) {
>> + uint64_t requiredModulus = atomAlign.modulus;
>> + int64_t align2 = 1 << atomAlign.powerOf2;
> Unsigned. Won't warn if you do 1u <<.
>
>> + uint64_t currentModulus = (offset % align2);
>> + uint64_t retOffset = offset;
>> + if (currentModulus != requiredModulus) {
>> + if (requiredModulus > currentModulus)
>> + retOffset += requiredModulus - currentModulus;
>> + else
>> + retOffset += align2 + requiredModulus - currentModulus;
>> + }
>> + return retOffset;
>> + }
>>
>> +
>> + void finalize() {
>> + // sh_info should be one greater than last symbol with STB_LOCAL binding
>> + // we sort the symbol table to keep all local symbols at the beginning
>> + std::stable_sort(_symbolTable.begin(), _symbolTable.end(),
>> + [](const Elf_Sym *A, const Elf_Sym *B) {
>> + return A->getBinding() < B->getBinding();
>> + });
> This needs to be reformatted.
>
>> + uint16_t shInfo = 0;
>> + for (auto i : _symbolTable) {
>> + if (i->getBinding() != ELF::STB_LOCAL)
>> + break;
>> + shInfo++;
>> + }
>> + this->_shInfo = shInfo;
>> + this->setLink(_stringSection->ordinal());
>> + }
>> +
>> + // HashKey for the Segment
>> + class SegmentHashKey {
>> + public:
>> + int64_t operator() (const SegmentKey &k) const {
>> + // k.first = SegmentName
>> + // k.second = SegmentFlags
>> + return llvm::hash_combine(k.first, k.second);
>> }
>> - symbol->st_value = aa->value();
>> - } else {
>> - symbol->st_value = 0;
>> - t = ELF::STT_NOTYPE;
>> - b = ELF::STB_LOCAL;
>> - }
>> - symbol->setBindingAndType(b, t);
>> + };
>>
>> - _symbolTable.push_back(symbol);
>> - this->_size += sizeof(Elf_Sym);
>> -}
>> + typedef std::unordered_map<Key,
>> + Section<target_endianness, is64Bits>*, HashKey> SectionMapT;
>> + typedef std::unordered_map<SegmentKey,
>> + Segment<target_endianness, is64Bits>*,
>> + SegmentHashKey> SegmentMapT;
>>
> - Michael Spencer
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation
More information about the llvm-commits
mailing list