[PATCH] D34344: [LLD][ELF] Extract allocateHeaders() from assignAddresses()

[Rafael Avila de Espindola via llvm-commits]

LGTM

Thanks

Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:

> peter.smith updated this revision to Diff 105146.
> peter.smith added a comment.
>
> Rebased to account for r307039
>
>
> https://reviews.llvm.org/D34344
>
> Files:
>   ELF/LinkerScript.cpp
>   ELF/LinkerScript.h
>   ELF/Writer.cpp
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -218,7 +218,8 @@
>        OutputSectionCommands.begin(), OutputSectionCommands.end(),
>        [](OutputSectionCommand *Cmd) { Cmd->maybeCompress<ELFT>(); });
>  
> -  Script->assignAddresses(Phdrs);
> +  Script->assignAddresses();
> +  Script->allocateHeaders(Phdrs);
>  
>    // Remove empty PT_LOAD to avoid causing the dynamic linker to try to mmap a
>    // 0 sized region. This has to be done late since only after assignAddresses
> Index: ELF/LinkerScript.h
> ===================================================================
> --- ELF/LinkerScript.h
> +++ ELF/LinkerScript.h
> @@ -281,8 +281,8 @@
>    void assignOffsets(OutputSectionCommand *Cmd);
>    void createOrphanCommands();
>    void processNonSectionCommands();
> -  void assignAddresses(std::vector<PhdrEntry> &Phdrs);
> -
> +  void assignAddresses();
> +  void allocateHeaders(std::vector<PhdrEntry> &Phdrs);
>    void addSymbol(SymbolAssignment *Cmd);
>    void processCommands(OutputSectionFactory &Factory);
>  
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -784,10 +784,14 @@
>    }
>  }
>  
> -static void
> -allocateHeaders(std::vector<PhdrEntry> &Phdrs,
> -                ArrayRef<OutputSectionCommand *> OutputSectionCommands,
> -                uint64_t Min) {
> +void LinkerScript::allocateHeaders(std::vector<PhdrEntry> &Phdrs) {
> +  uint64_t Min = std::numeric_limits<uint64_t>::max();
> +  for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> +    OutputSection *Sec = Cmd->Sec;
> +    if (Sec->Flags & SHF_ALLOC)
> +      Min = std::min<uint64_t>(Min, Sec->Addr);
> +  }
> +
>    auto FirstPTLoad = llvm::find_if(
>        Phdrs, [](const PhdrEntry &E) { return E.p_type == PT_LOAD; });
>    if (FirstPTLoad == Phdrs.end())
> @@ -827,7 +831,7 @@
>      Phdrs.erase(PhdrI);
>  }
>  
> -void LinkerScript::assignAddresses(std::vector<PhdrEntry> &Phdrs) {
> +void LinkerScript::assignAddresses() {
>    // Assign addresses as instructed by linker script SECTIONS sub-commands.
>    Dot = 0;
>    ErrorOnMissingSection = true;
> @@ -847,15 +851,6 @@
>      auto *Cmd = cast<OutputSectionCommand>(Base);
>      assignOffsets(Cmd);
>    }
> -
> -  uint64_t MinVA = std::numeric_limits<uint64_t>::max();
> -  for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> -    OutputSection *Sec = Cmd->Sec;
> -    if (Sec->Flags & SHF_ALLOC)
> -      MinVA = std::min<uint64_t>(MinVA, Sec->Addr);
> -  }
> -
> -  allocateHeaders(Phdrs, OutputSectionCommands, MinVA);
>  }
>  
>  // Creates program headers as instructed by PHDRS linker script command.
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -218,7 +218,8 @@
>        OutputSectionCommands.begin(), OutputSectionCommands.end(),
>        [](OutputSectionCommand *Cmd) { Cmd->maybeCompress<ELFT>(); });
>  
> -  Script->assignAddresses(Phdrs);
> +  Script->assignAddresses();
> +  Script->allocateHeaders(Phdrs);
>  
>    // Remove empty PT_LOAD to avoid causing the dynamic linker to try to mmap a
>    // 0 sized region. This has to be done late since only after assignAddresses
> Index: ELF/LinkerScript.h
> ===================================================================
> --- ELF/LinkerScript.h
> +++ ELF/LinkerScript.h
> @@ -281,8 +281,8 @@
>    void assignOffsets(OutputSectionCommand *Cmd);
>    void createOrphanCommands();
>    void processNonSectionCommands();
> -  void assignAddresses(std::vector<PhdrEntry> &Phdrs);
> -
> +  void assignAddresses();
> +  void allocateHeaders(std::vector<PhdrEntry> &Phdrs);
>    void addSymbol(SymbolAssignment *Cmd);
>    void processCommands(OutputSectionFactory &Factory);
>  
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -784,10 +784,14 @@
>    }
>  }
>  
> -static void
> -allocateHeaders(std::vector<PhdrEntry> &Phdrs,
> -                ArrayRef<OutputSectionCommand *> OutputSectionCommands,
> -                uint64_t Min) {
> +void LinkerScript::allocateHeaders(std::vector<PhdrEntry> &Phdrs) {
> +  uint64_t Min = std::numeric_limits<uint64_t>::max();
> +  for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> +    OutputSection *Sec = Cmd->Sec;
> +    if (Sec->Flags & SHF_ALLOC)
> +      Min = std::min<uint64_t>(Min, Sec->Addr);
> +  }
> +
>    auto FirstPTLoad = llvm::find_if(
>        Phdrs, [](const PhdrEntry &E) { return E.p_type == PT_LOAD; });
>    if (FirstPTLoad == Phdrs.end())
> @@ -827,7 +831,7 @@
>      Phdrs.erase(PhdrI);
>  }
>  
> -void LinkerScript::assignAddresses(std::vector<PhdrEntry> &Phdrs) {
> +void LinkerScript::assignAddresses() {
>    // Assign addresses as instructed by linker script SECTIONS sub-commands.
>    Dot = 0;
>    ErrorOnMissingSection = true;
> @@ -847,15 +851,6 @@
>      auto *Cmd = cast<OutputSectionCommand>(Base);
>      assignOffsets(Cmd);
>    }
> -
> -  uint64_t MinVA = std::numeric_limits<uint64_t>::max();
> -  for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> -    OutputSection *Sec = Cmd->Sec;
> -    if (Sec->Flags & SHF_ALLOC)
> -      MinVA = std::min<uint64_t>(MinVA, Sec->Addr);
> -  }
> -
> -  allocateHeaders(Phdrs, OutputSectionCommands, MinVA);
>  }
>  
>  // Creates program headers as instructed by PHDRS linker script command.