[lld] r249955 - ELF2: Remove ProgramHeader class and use Elf_Phdr directly. NFC.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 11 07:24:54 PDT 2015
Sorry about the style change. I'll separate them in future.
I think the use of unordered_set is reasonable because the number of
segments are very small.
On Sun, Oct 11, 2015 at 6:58 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> The change to use a std::unordered_set is hilly questionable. You
> could have kept just a pair of Elf_Phdr and bool, no?
>
> On 11 October 2015 at 09:48, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
> > Functions names normally start with a verb. So it should probably be
> > copyPhdr and setPhdr.
> >
> > On 10 October 2015 at 18:34, Rui Ueyama via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >> Author: ruiu
> >> Date: Sat Oct 10 17:34:30 2015
> >> New Revision: 249955
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=249955&view=rev
> >> Log:
> >> ELF2: Remove ProgramHeader class and use Elf_Phdr directly. NFC.
> >>
> >> Modified:
> >> lld/trunk/ELF/Writer.cpp
> >>
> >> Modified: lld/trunk/ELF/Writer.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=249955&r1=249954&r2=249955&view=diff
> >>
> ==============================================================================
> >> --- lld/trunk/ELF/Writer.cpp (original)
> >> +++ lld/trunk/ELF/Writer.cpp Sat Oct 10 17:34:30 2015
> >> @@ -14,6 +14,7 @@
> >> #include "Target.h"
> >>
> >> #include "llvm/Support/FileOutputBuffer.h"
> >> +#include <unordered_set>
> >>
> >> using namespace llvm;
> >> using namespace llvm::ELF;
> >> @@ -24,7 +25,7 @@ using namespace lld::elf2;
> >>
> >> namespace {
> >>
> >> -static uint32_t toPHDRFlags(uint64_t Flags) {
> >> +static uint32_t toPhdrFlags(uint64_t Flags) {
> >> uint32_t Ret = PF_R;
> >> if (Flags & SHF_WRITE)
> >> Ret |= PF_W;
> >> @@ -33,34 +34,6 @@ static uint32_t toPHDRFlags(uint64_t Fla
> >> return Ret;
> >> }
> >>
> >> -template <class ELFT> struct ProgramHeader {
> >> - typedef typename ELFFile<ELFT>::uintX_t uintX_t;
> >> - typedef typename ELFFile<ELFT>::Elf_Phdr Elf_Phdr;
> >> -
> >> - ProgramHeader(uintX_t Type, uintX_t Flags, uintX_t FileOff, uintX_t
> VA) {
> >> - std::memset(&Header, 0, sizeof(Elf_Phdr));
> >> - Header.p_type = Type;
> >> - Header.p_flags = Flags;
> >> - Header.p_align = Target->getPageSize();
> >> - Header.p_offset = FileOff;
> >> - Header.p_vaddr = VA;
> >> - Header.p_paddr = VA;
> >> - }
> >> -
> >> - void setValuesFromSection(OutputSectionBase<ELFT::Is64Bits> *Sec) {
> >> - Header.p_flags = toPHDRFlags(Sec->getFlags());
> >> - Header.p_offset = Sec->getFileOff();
> >> - Header.p_vaddr = Sec->getVA();
> >> - Header.p_paddr = Header.p_vaddr;
> >> - Header.p_filesz = Sec->getSize();
> >> - Header.p_memsz = Header.p_filesz;
> >> - Header.p_align = Sec->getAlign();
> >> - }
> >> -
> >> - Elf_Phdr Header;
> >> - bool Closed = false;
> >> -};
> >> -
> >> // The writer writes a SymbolTable result to a file.
> >> template <class ELFT> class Writer {
> >> public:
> >> @@ -100,13 +73,17 @@ private:
> >> std::vector<OutputSectionBase<ELFT::Is64Bits> *> OutputSections;
> >> unsigned getNumSections() const { return OutputSections.size() + 1; }
> >>
> >> + void phdrSet(Elf_Phdr *PH, uint32_t Type, uint32_t Flags, uintX_t
> FileOff,
> >> + uintX_t VA, uintX_t Align);
> >> + void phdrCopy(Elf_Phdr *PH, OutputSectionBase<ELFT::Is64Bits> *From);
> >> +
> >> llvm::BumpPtrAllocator PAlloc;
> >> SymbolTable<ELFT> &Symtab;
> >> - std::vector<ProgramHeader<ELFT> *> PHDRs;
> >> - ProgramHeader<ELFT> PhdrPHDR{PT_PHDR, PF_R, 0, 0};
> >> - ProgramHeader<ELFT> FileHeaderPHDR{PT_LOAD, PF_R, 0, 0};
> >> - ProgramHeader<ELFT> InterpPHDR{PT_INTERP, 0, 0, 0};
> >> - ProgramHeader<ELFT> DynamicPHDR{PT_DYNAMIC, 0, 0, 0};
> >> + std::vector<Elf_Phdr *> Phdrs;
> >> + Elf_Phdr PhdrPhdr;
> >> + Elf_Phdr FileHeaderPhdr;
> >> + Elf_Phdr InterpPhdr;
> >> + Elf_Phdr DynamicPhdr;
> >>
> >> uintX_t FileSize;
> >> uintX_t ProgramHeaderOff;
> >> @@ -473,7 +450,7 @@ template <class ELFT> void Writer<ELFT>:
> >> }
> >>
> >> template <class ELFT>
> >> -static bool needsPHDR(OutputSectionBase<ELFT::Is64Bits> *Sec) {
> >> +static bool needsPhdr(OutputSectionBase<ELFT::Is64Bits> *Sec) {
> >> return Sec->getFlags() & SHF_ALLOC;
> >> }
> >>
> >> @@ -481,50 +458,44 @@ static bool needsPHDR(OutputSectionBase<
> >> // file offsets.
> >> template <class ELFT> void Writer<ELFT>::assignAddresses() {
> >> assert(!OutputSections.empty() && "No output sections to layout!");
> >> - uintX_t VA = getVAStart();
> >> - uintX_t FileOff = 0;
> >> -
> >> - FileOff += sizeof(Elf_Ehdr);
> >> - VA += sizeof(Elf_Ehdr);
> >> + uintX_t VA = getVAStart() + sizeof(Elf_Ehdr);
> >> + uintX_t FileOff = sizeof(Elf_Ehdr);
> >>
> >> - // The first PHDR entry is PT_PHDR which describes the program
> header itself.
> >> - PHDRs.push_back(&PhdrPHDR);
> >> - PhdrPHDR.Header.p_align = 8;
> >> - PhdrPHDR.Header.p_offset = FileOff;
> >> - PhdrPHDR.Header.p_vaddr = VA;
> >> - PhdrPHDR.Header.p_paddr = VA;
> >> + // The first Phdr entry is PT_PHDR which describes the program
> header itself.
> >> + Phdrs.push_back(&PhdrPhdr);
> >> + phdrSet(&PhdrPhdr, PT_PHDR, PF_R, FileOff, VA, /*Align=*/8);
> >>
> >> - // Reserve space for PHDRs.
> >> + // Reserve space for Phdrs.
> >> ProgramHeaderOff = FileOff;
> >> FileOff = RoundUpToAlignment(FileOff, Target->getPageSize());
> >> VA = RoundUpToAlignment(VA, Target->getPageSize());
> >>
> >> if (needsInterpSection())
> >> - PHDRs.push_back(&InterpPHDR);
> >> + Phdrs.push_back(&InterpPhdr);
> >>
> >> - // Create a PHDR for the file header.
> >> - PHDRs.push_back(&FileHeaderPHDR);
> >> - FileHeaderPHDR.Header.p_vaddr = getVAStart();
> >> - FileHeaderPHDR.Header.p_paddr = getVAStart();
> >> - FileHeaderPHDR.Header.p_align = Target->getPageSize();
> >> + // Create a Phdr for the file header.
> >> + Phdrs.push_back(&FileHeaderPhdr);
> >> + phdrSet(&FileHeaderPhdr, PT_LOAD, PF_R, 0, getVAStart(),
> >> + Target->getPageSize());
> >>
> >> + std::unordered_set<Elf_Phdr *> Closed;
> >> for (OutputSectionBase<ELFT::Is64Bits> *Sec : OutputSections) {
> >> if (Sec->getSize()) {
> >> - uintX_t Flags = toPHDRFlags(Sec->getFlags());
> >> - ProgramHeader<ELFT> *Last = PHDRs.back();
> >> - if (Last->Header.p_flags != Flags || !needsPHDR<ELFT>(Sec)) {
> >> - // Flags changed. End current PHDR and potentially create a
> new one.
> >> - if (!Last->Closed) {
> >> - Last->Header.p_filesz = FileOff - Last->Header.p_offset;
> >> - Last->Header.p_memsz = VA - Last->Header.p_vaddr;
> >> - Last->Closed = true;
> >> + uintX_t Flags = toPhdrFlags(Sec->getFlags());
> >> + Elf_Phdr *Last = Phdrs.back();
> >> + if (Last->p_flags != Flags || !needsPhdr<ELFT>(Sec)) {
> >> + // Flags changed. End current Phdr and potentially create a
> new one.
> >> + if (Closed.insert(Last).second) {
> >> + Last->p_filesz = FileOff - Last->p_offset;
> >> + Last->p_memsz = VA - Last->p_vaddr;
> >> }
> >>
> >> - if (needsPHDR<ELFT>(Sec)) {
> >> + if (needsPhdr<ELFT>(Sec)) {
> >> VA = RoundUpToAlignment(VA, Target->getPageSize());
> >> FileOff = RoundUpToAlignment(FileOff, Target->getPageSize());
> >> - PHDRs.push_back(new (PAlloc)
> >> - ProgramHeader<ELFT>(PT_LOAD, Flags,
> FileOff, VA));
> >> + auto *PH = new (PAlloc) Elf_Phdr;
> >> + phdrSet(PH, PT_LOAD, Flags, FileOff, VA,
> Target->getPageSize());
> >> + Phdrs.push_back(PH);
> >> }
> >> }
> >> }
> >> @@ -542,17 +513,22 @@ template <class ELFT> void Writer<ELFT>:
> >> FileOff += Size;
> >> }
> >>
> >> - // Add a PHDR for the dynamic table.
> >> - if (needsDynamicSections())
> >> - PHDRs.push_back(&DynamicPHDR);
> >> -
> >> - FileOff += OffsetToAlignment(FileOff, ELFT::Is64Bits ? 8 : 4);
> >> + if (needsInterpSection()) {
> >> + InterpPhdr.p_type = PT_INTERP;
> >> + phdrCopy(&InterpPhdr, Out<ELFT>::Interp);
> >> + }
> >> + if (needsDynamicSections()) {
> >> + Phdrs.push_back(&DynamicPhdr);
> >> + DynamicPhdr.p_type = PT_DYNAMIC;
> >> + phdrCopy(&DynamicPhdr, Out<ELFT>::Dynamic);
> >> + }
> >>
> >> // Fix up the first entry's size.
> >> - PhdrPHDR.Header.p_filesz = sizeof(Elf_Phdr) * PHDRs.size();
> >> - PhdrPHDR.Header.p_memsz = sizeof(Elf_Phdr) * PHDRs.size();
> >> + PhdrPhdr.p_filesz = sizeof(Elf_Phdr) * Phdrs.size();
> >> + PhdrPhdr.p_memsz = sizeof(Elf_Phdr) * Phdrs.size();
> >>
> >> // Add space for section headers.
> >> + FileOff = RoundUpToAlignment(FileOff, ELFT::Is64Bits ? 8 : 4);
> >> SectionHeaderOff = FileOff;
> >> FileOff += getNumSections() * sizeof(Elf_Shdr);
> >> FileSize = FileOff;
> >> @@ -587,26 +563,21 @@ template <class ELFT> void Writer<ELFT>:
> >> EHdr->e_shoff = SectionHeaderOff;
> >> EHdr->e_ehsize = sizeof(Elf_Ehdr);
> >> EHdr->e_phentsize = sizeof(Elf_Phdr);
> >> - EHdr->e_phnum = PHDRs.size();
> >> + EHdr->e_phnum = Phdrs.size();
> >> EHdr->e_shentsize = sizeof(Elf_Shdr);
> >> EHdr->e_shnum = getNumSections();
> >> EHdr->e_shstrndx = Out<ELFT>::StrTab->getSectionIndex();
> >>
> >> // If nothing was merged into the file header PT_LOAD, set the size
> correctly.
> >> - if (FileHeaderPHDR.Header.p_filesz == Target->getPageSize()) {
> >> - uint64_t Size = sizeof(Elf_Ehdr) + sizeof(Elf_Phdr) * PHDRs.size();
> >> - FileHeaderPHDR.Header.p_filesz = Size;
> >> - FileHeaderPHDR.Header.p_memsz = Size;
> >> + if (FileHeaderPhdr.p_filesz == Target->getPageSize()) {
> >> + uint64_t Size = sizeof(Elf_Ehdr) + sizeof(Elf_Phdr) * Phdrs.size();
> >> + FileHeaderPhdr.p_filesz = Size;
> >> + FileHeaderPhdr.p_memsz = Size;
> >> }
> >>
> >> - if (needsInterpSection())
> >> - InterpPHDR.setValuesFromSection(Out<ELFT>::Interp);
> >> - if (needsDynamicSections())
> >> - DynamicPHDR.setValuesFromSection(Out<ELFT>::Dynamic);
> >> -
> >> auto PHdrs = reinterpret_cast<Elf_Phdr *>(Buf + EHdr->e_phoff);
> >> - for (ProgramHeader<ELFT> *PHDR : PHDRs)
> >> - *PHdrs++ = PHDR->Header;
> >> + for (Elf_Phdr *PH : Phdrs)
> >> + *PHdrs++ = *PH;
> >>
> >> auto SHdrs = reinterpret_cast<Elf_Shdr *>(Buf + EHdr->e_shoff);
> >> // First entry is null.
> >> @@ -631,6 +602,29 @@ template <class ELFT> void Writer<ELFT>:
> >> Sec->writeTo(Buf + Sec->getFileOff());
> >> }
> >>
> >> +template <class ELFT>
> >> +void Writer<ELFT>::phdrSet(Elf_Phdr *PH, uint32_t Type, uint32_t Flags,
> >> + uintX_t FileOff, uintX_t VA, uintX_t Align)
> {
> >> + PH->p_type = Type;
> >> + PH->p_flags = Flags;
> >> + PH->p_offset = FileOff;
> >> + PH->p_vaddr = VA;
> >> + PH->p_paddr = VA;
> >> + PH->p_align = Align;
> >> +}
> >> +
> >> +template <class ELFT>
> >> +void Writer<ELFT>::phdrCopy(Elf_Phdr *PH,
> >> + OutputSectionBase<ELFT::Is64Bits> *From) {
> >> + PH->p_flags = toPhdrFlags(From->getFlags());
> >> + PH->p_offset = From->getFileOff();
> >> + PH->p_vaddr = From->getVA();
> >> + PH->p_paddr = From->getVA();
> >> + PH->p_filesz = From->getSize();
> >> + PH->p_memsz = From->getSize();
> >> + PH->p_align = From->getAlign();
> >> +}
> >> +
> >> template void lld::elf2::writeResult<ELF32LE>(SymbolTable<ELF32LE>
> *Symtab);
> >> template void lld::elf2::writeResult<ELF32BE>(SymbolTable<ELF32BE>
> *Symtab);
> >> template void lld::elf2::writeResult<ELF64LE>(SymbolTable<ELF64LE>
> *Symtab);
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151011/5727cca3/attachment.html>
More information about the llvm-commits
mailing list