[PATCH] D30085: [LLD][ELF] Alloc local symbols to be added to the SymTab after global symbols

[Rafael Avila de Espindola via llvm-commits]

LGTM

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

> peter.smith created this revision.
>
> This change moves the SymbolBodies with isLocal() == true before the global symbols then calculating NumLocals rather than assuming all locals are added before globals and the first NumLocals have isLocal() == true.
>
> This permits the thunk creation code, that generates local symbols, to be moved after the pass that adds global symbols from synthetics to the symbol table.
>
>
> https://reviews.llvm.org/D30085
>
> Files:
>   ELF/SyntheticSections.cpp
>   ELF/Writer.cpp
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1112,13 +1112,7 @@
>    if (In<ELFT>::Iplt && !In<ELFT>::Iplt->empty())
>      In<ELFT>::Iplt->addSymbols();
>  
> -  // Some architectures use small displacements for jump instructions.
> -  // It is linker's responsibility to create thunks containing long
> -  // jump instructions if jump targets are too far. Create thunks.
> -  if (Target->NeedsThunks)
> -    createThunks<ELFT>(OutputSections);
> -
> -  // Now that we have defined all possible symbols including linker-
> +  // Now that we have defined all possible global symbols including linker-
>    // synthesized ones. Visit all symbols to give the finishing touches.
>    for (Symbol *S : Symtab<ELFT>::X->getSymbols()) {
>      SymbolBody *Body = S->body();
> @@ -1168,6 +1162,12 @@
>      fixHeaders();
>    }
>  
> +  // Some architectures use small displacements for jump instructions.
> +  // It is linker's responsibility to create thunks containing long
> +  // jump instructions if jump targets are too far. Create thunks.
> +  if (Target->NeedsThunks)
> +    createThunks<ELFT>(OutputSections);
> +
>    // 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/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1085,9 +1085,17 @@
>  
>  template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
>    this->OutSec->Link = this->Link = StrTabSec.OutSec->SectionIndex;
> -  this->OutSec->Info = this->Info = NumLocals + 1;
>    this->OutSec->Entsize = this->Entsize;
>  
> +  if (!StrTabSec.isDynamic()) {
> +    // All explictly added STB_LOCAL symbols without a Symbol are first
> +    auto It = std::stable_partition(
> +        Symbols.begin(), Symbols.end(),
> +        [](const SymbolTableEntry &S) { return S.Symbol->isLocal(); });
> +    NumLocals = It - Symbols.begin();
> +  }
> +  this->OutSec->Info = this->Info = 1 + NumLocals;
> +
>    if (Config->Relocatable)
>      return;
>  
> @@ -1122,7 +1130,6 @@
>  
>  template <class ELFT> void SymbolTableSection<ELFT>::addLocal(SymbolBody *B) {
>    assert(!StrTabSec.isDynamic());
> -  ++NumLocals;
>    Symbols.push_back({B, StrTabSec.addString(B->getName())});
>  }
>  
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1112,13 +1112,7 @@
>    if (In<ELFT>::Iplt && !In<ELFT>::Iplt->empty())
>      In<ELFT>::Iplt->addSymbols();
>  
> -  // Some architectures use small displacements for jump instructions.
> -  // It is linker's responsibility to create thunks containing long
> -  // jump instructions if jump targets are too far. Create thunks.
> -  if (Target->NeedsThunks)
> -    createThunks<ELFT>(OutputSections);
> -
> -  // Now that we have defined all possible symbols including linker-
> +  // Now that we have defined all possible global symbols including linker-
>    // synthesized ones. Visit all symbols to give the finishing touches.
>    for (Symbol *S : Symtab<ELFT>::X->getSymbols()) {
>      SymbolBody *Body = S->body();
> @@ -1168,6 +1162,12 @@
>      fixHeaders();
>    }
>  
> +  // Some architectures use small displacements for jump instructions.
> +  // It is linker's responsibility to create thunks containing long
> +  // jump instructions if jump targets are too far. Create thunks.
> +  if (Target->NeedsThunks)
> +    createThunks<ELFT>(OutputSections);
> +
>    // 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/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1085,9 +1085,17 @@
>  
>  template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
>    this->OutSec->Link = this->Link = StrTabSec.OutSec->SectionIndex;
> -  this->OutSec->Info = this->Info = NumLocals + 1;
>    this->OutSec->Entsize = this->Entsize;
>  
> +  if (!StrTabSec.isDynamic()) {
> +    // All explictly added STB_LOCAL symbols without a Symbol are first
> +    auto It = std::stable_partition(
> +        Symbols.begin(), Symbols.end(),
> +        [](const SymbolTableEntry &S) { return S.Symbol->isLocal(); });
> +    NumLocals = It - Symbols.begin();
> +  }
> +  this->OutSec->Info = this->Info = 1 + NumLocals;
> +
>    if (Config->Relocatable)
>      return;
>  
> @@ -1122,7 +1130,6 @@
>  
>  template <class ELFT> void SymbolTableSection<ELFT>::addLocal(SymbolBody *B) {
>    assert(!StrTabSec.isDynamic());
> -  ++NumLocals;
>    Symbols.push_back({B, StrTabSec.addString(B->getName())});
>  }
>