[PATCH] D33772: [LLD][ELF] SHF_LINK_ORDER should sort based on InputSectionDescriptions

[Rafael Avila de Espindola via llvm-commits]

LGTM


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

> peter.smith updated this revision to Diff 101177.
> peter.smith added a comment.
>
>> On trunk we now have
>
>
>
>   if (!Script->Opt.HasSections) {
>     if (!Config->Relocatable)
>       fixSectionAlignments();
>     Script->fabricateDefaultCommands();
>   } else {
>     Script->synchronize();
>   }
>
>> Would it be possible to move this block together? That would avoid the need to change synchronize as it would now be earlier too.
>
> I've updated the diff to move the fabricatedDefaultCommands() after ThunkCreation, this permits the block to stay together. Later Today I'm intending to post a set of patches to convert Thunks to insert into InputSectionDescriptions which will hopefully permit synchronize() to be removed.
>
>
> https://reviews.llvm.org/D33772
>
> Files:
>   ELF/OutputSections.cpp
>   ELF/Writer.cpp
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -257,14 +257,6 @@
>    if (ErrorCount)
>      return;
>  
> -  if (!Script->Opt.HasSections) {
> -    Script->fabricateDefaultCommands();
> -    if (!Config->Relocatable)
> -      fixSectionAlignments();
> -  } else {
> -    Script->synchronize();
> -  }
> -
>    for (BaseCommand *Base : Script->Opt.Commands)
>      if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base))
>        OutputSectionCommands.push_back(Cmd);
> @@ -1260,6 +1252,15 @@
>        applySynthetic({InX::MipsGot},
>                       [](SyntheticSection *SS) { SS->updateAllocSize(); });
>    }
> +
> +  if (!Script->Opt.HasSections) {
> +    Script->fabricateDefaultCommands();
> +    if (!Config->Relocatable)
> +      fixSectionAlignments();
> +  } else {
> +    Script->synchronize();
> +  }
> +
>    // Fill other section headers. The dynamic table is finalized
>    // at the end because some tags like RELSZ depend on result
>    // of finalizing other sections.
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -99,8 +99,20 @@
>  
>  template <class ELFT> void OutputSection::finalize() {
>    if ((this->Flags & SHF_LINK_ORDER) && !this->Sections.empty()) {
> +    OutputSectionCommand *Cmd = Script->getCmd(this);
> +    // Link order may be distributed across several InputSectionDescriptions
> +    // but sort must consider them all at once.
> +    std::vector<InputSection **> ScriptSections;
> +    std::vector<InputSection *> Sections;
> +    for (BaseCommand *Base : Cmd->Commands)
> +      if (auto *ISD = dyn_cast<InputSectionDescription>(Base))
> +        for (InputSection *&IS : ISD->Sections) {
> +          ScriptSections.push_back(&IS);
> +          Sections.push_back(IS);
> +        }
>      std::sort(Sections.begin(), Sections.end(), compareByFilePosition);
> -    assignOffsets();
> +    for (int I = 0, N = Sections.size(); I < N; ++I)
> +      *ScriptSections[I] = Sections[I];
>  
>      // We must preserve the link order dependency of sections with the
>      // SHF_LINK_ORDER flag. The dependency is indicated by the sh_link field. We
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -257,14 +257,6 @@
>    if (ErrorCount)
>      return;
>  
> -  if (!Script->Opt.HasSections) {
> -    Script->fabricateDefaultCommands();
> -    if (!Config->Relocatable)
> -      fixSectionAlignments();
> -  } else {
> -    Script->synchronize();
> -  }
> -
>    for (BaseCommand *Base : Script->Opt.Commands)
>      if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base))
>        OutputSectionCommands.push_back(Cmd);
> @@ -1260,6 +1252,15 @@
>        applySynthetic({InX::MipsGot},
>                       [](SyntheticSection *SS) { SS->updateAllocSize(); });
>    }
> +
> +  if (!Script->Opt.HasSections) {
> +    Script->fabricateDefaultCommands();
> +    if (!Config->Relocatable)
> +      fixSectionAlignments();
> +  } else {
> +    Script->synchronize();
> +  }
> +
>    // Fill other section headers. The dynamic table is finalized
>    // at the end because some tags like RELSZ depend on result
>    // of finalizing other sections.
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -99,8 +99,20 @@
>  
>  template <class ELFT> void OutputSection::finalize() {
>    if ((this->Flags & SHF_LINK_ORDER) && !this->Sections.empty()) {
> +    OutputSectionCommand *Cmd = Script->getCmd(this);
> +    // Link order may be distributed across several InputSectionDescriptions
> +    // but sort must consider them all at once.
> +    std::vector<InputSection **> ScriptSections;
> +    std::vector<InputSection *> Sections;
> +    for (BaseCommand *Base : Cmd->Commands)
> +      if (auto *ISD = dyn_cast<InputSectionDescription>(Base))
> +        for (InputSection *&IS : ISD->Sections) {
> +          ScriptSections.push_back(&IS);
> +          Sections.push_back(IS);
> +        }
>      std::sort(Sections.begin(), Sections.end(), compareByFilePosition);
> -    assignOffsets();
> +    for (int I = 0, N = Sections.size(); I < N; ++I)
> +      *ScriptSections[I] = Sections[I];
>  
>      // We must preserve the link order dependency of sections with the
>      // SHF_LINK_ORDER flag. The dependency is indicated by the sh_link field. We