[PATCH] D29021: [ELF] - Stop handling local symbols in a special way.
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 23 06:13:01 PST 2017
LGTM.
We can try to speed up -r afterwards if it is a problem.
Cheers,
Rafael
George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
> grimar created this revision.
>
> Previously we stored kept locals in a KeptLocalSyms arrays,
> belonged to files.
>
> Patch makes SymbolTableSection to store locals in Symbols member,
> that already present and was used for globals.
> SymbolTableSection already had NumLocals counter member, so change
> itself is trivial.
>
> That allows to simplify handling of -r,
> Body::DynsymIndex is no more used as "symbol table index" for relocatable
> output.
>
> Change was suggested during review of https://reviews.llvm.org/D28773 and opens road for https://reviews.llvm.org/D28612.
>
>
> https://reviews.llvm.org/D29021
>
> Files:
> ELF/InputFiles.h
> ELF/InputSection.cpp
> ELF/SyntheticSections.cpp
> ELF/SyntheticSections.h
> ELF/Writer.cpp
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -466,11 +466,7 @@
> InputSectionBase<ELFT> *Sec = DR->Section;
> if (!shouldKeepInSymtab<ELFT>(Sec, B->getName(), *B))
> continue;
> - ++In<ELFT>::SymTab->NumLocals;
> - if (Config->Relocatable)
> - B->DynsymIndex = In<ELFT>::SymTab->NumLocals;
> - F->KeptLocalSyms.push_back(std::make_pair(
> - DR, In<ELFT>::SymTab->StrTabSec.addString(B->getName())));
> + In<ELFT>::SymTab->addLocal(B);
> }
> }
> }
> @@ -1040,10 +1036,10 @@
> if (!includeInSymtab<ELFT>(*Body))
> continue;
> if (In<ELFT>::SymTab)
> - In<ELFT>::SymTab->addSymbol(Body);
> + In<ELFT>::SymTab->addGlobal(Body);
>
> if (In<ELFT>::DynSymTab && S->includeInDynsym()) {
> - In<ELFT>::DynSymTab->addSymbol(Body);
> + In<ELFT>::DynSymTab->addGlobal(Body);
> if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(Body))
> if (SS->file()->isNeeded())
> In<ELFT>::VerNeed->addSymbol(SS);
> Index: ELF/SyntheticSections.h
> ===================================================================
> --- ELF/SyntheticSections.h
> +++ ELF/SyntheticSections.h
> @@ -366,23 +366,26 @@
> void finalize() override;
> void writeTo(uint8_t *Buf) override;
> size_t getSize() const override { return getNumSymbols() * sizeof(Elf_Sym); }
> - void addSymbol(SymbolBody *Body);
> + void addGlobal(SymbolBody *Body);
> + void addLocal(SymbolBody *Body);
> StringTableSection<ELFT> &getStrTabSec() const { return StrTabSec; }
> - unsigned getNumSymbols() const { return NumLocals + Symbols.size() + 1; }
> + unsigned getNumSymbols() const { return Symbols.size() + 1; }
> + size_t getSymbolIndex(SymbolBody *Body);
>
> ArrayRef<SymbolTableEntry> getSymbols() const { return Symbols; }
>
> static const OutputSectionBase *getOutputSection(SymbolBody *Sym);
>
> - unsigned NumLocals = 0;
> - StringTableSection<ELFT> &StrTabSec;
> -
> private:
> void writeLocalSymbols(uint8_t *&Buf);
> void writeGlobalSymbols(uint8_t *Buf);
>
> // A vector of symbols and their string table offsets.
> std::vector<SymbolTableEntry> Symbols;
> +
> + StringTableSection<ELFT> &StrTabSec;
> +
> + unsigned NumLocals = 0;
> };
>
> // Outputs GNU Hash section. For detailed explanation see:
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1065,22 +1065,19 @@
> this->OutSec->Info = this->Info = NumLocals + 1;
> this->OutSec->Entsize = this->Entsize;
>
> - if (Config->Relocatable) {
> - size_t I = NumLocals;
> - for (const SymbolTableEntry &S : Symbols)
> - S.Symbol->DynsymIndex = ++I;
> + if (Config->Relocatable)
> return;
> - }
>
> if (!StrTabSec.isDynamic()) {
> - std::stable_sort(
> - Symbols.begin(), Symbols.end(),
> - [](const SymbolTableEntry &L, const SymbolTableEntry &R) {
> - return L.Symbol->symbol()->computeBinding() == STB_LOCAL &&
> - R.Symbol->symbol()->computeBinding() != STB_LOCAL;
> - });
> + auto GlobBegin = Symbols.begin() + NumLocals;
> + std::stable_sort(GlobBegin, Symbols.end(), [](const SymbolTableEntry &L,
> + const SymbolTableEntry &R) {
> + return L.Symbol->symbol()->computeBinding() == STB_LOCAL &&
> + R.Symbol->symbol()->computeBinding() != STB_LOCAL;
> + });
> return;
> }
> +
> if (In<ELFT>::GnuHashTab)
> // NB: It also sorts Symbols to meet the GNU hash table requirements.
> In<ELFT>::GnuHashTab->addSymbols(Symbols);
> @@ -1094,10 +1091,23 @@
> S.Symbol->DynsymIndex = ++I;
> }
>
> -template <class ELFT> void SymbolTableSection<ELFT>::addSymbol(SymbolBody *B) {
> +template <class ELFT> void SymbolTableSection<ELFT>::addGlobal(SymbolBody *B) {
> Symbols.push_back({B, StrTabSec.addString(B->getName(), false)});
> }
>
> +template <class ELFT> void SymbolTableSection<ELFT>::addLocal(SymbolBody *B) {
> + assert(!StrTabSec.isDynamic());
> + ++NumLocals;
> + Symbols.push_back({B, StrTabSec.addString(B->getName())});
> +}
> +
> +template <class ELFT>
> +size_t SymbolTableSection<ELFT>::getSymbolIndex(SymbolBody *Body) {
> + auto I = llvm::find_if(
> + Symbols, [&](const SymbolTableEntry &E) { return E.Symbol == Body; });
> + return I - Symbols.begin() + 1;
> +}
> +
> template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *Buf) {
> Buf += sizeof(Elf_Sym);
>
> @@ -1113,35 +1123,35 @@
> void SymbolTableSection<ELFT>::writeLocalSymbols(uint8_t *&Buf) {
> // Iterate over all input object files to copy their local symbols
> // to the output symbol table pointed by Buf.
> - for (ObjectFile<ELFT> *File : Symtab<ELFT>::X->getObjectFiles()) {
> - for (const std::pair<const DefinedRegular<ELFT> *, size_t> &P :
> - File->KeptLocalSyms) {
> - const DefinedRegular<ELFT> &Body = *P.first;
> - InputSectionBase<ELFT> *Section = Body.Section;
> - auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
> -
> - if (!Section) {
> - ESym->st_shndx = SHN_ABS;
> - ESym->st_value = Body.Value;
> - } else {
> - const OutputSectionBase *OutSec = Section->OutSec;
> - ESym->st_shndx = OutSec->SectionIndex;
> - ESym->st_value = OutSec->Addr + Section->getOffset(Body);
> - }
> - ESym->st_name = P.second;
> - ESym->st_size = Body.template getSize<ELFT>();
> - ESym->setBindingAndType(STB_LOCAL, Body.Type);
> - Buf += sizeof(*ESym);
> +
> + for (auto I = Symbols.begin(); I != Symbols.begin() + NumLocals; ++I) {
> + const DefinedRegular<ELFT> &Body = *cast<DefinedRegular<ELFT>>(I->Symbol);
> + InputSectionBase<ELFT> *Section = Body.Section;
> + auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
> +
> + if (!Section) {
> + ESym->st_shndx = SHN_ABS;
> + ESym->st_value = Body.Value;
> + } else {
> + const OutputSectionBase *OutSec = Section->OutSec;
> + ESym->st_shndx = OutSec->SectionIndex;
> + ESym->st_value = OutSec->Addr + Section->getOffset(Body);
> }
> + ESym->st_name = I->StrTabOffset;
> + ESym->st_size = Body.template getSize<ELFT>();
> + ESym->setBindingAndType(STB_LOCAL, Body.Type);
> + Buf += sizeof(*ESym);
> }
> }
>
> template <class ELFT>
> void SymbolTableSection<ELFT>::writeGlobalSymbols(uint8_t *Buf) {
> // Write the internal symbol table contents to the output symbol table
> // pointed by Buf.
> auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
> - for (const SymbolTableEntry &S : Symbols) {
> +
> + for (auto I = Symbols.begin() + NumLocals; I != Symbols.end(); ++I) {
> + const SymbolTableEntry &S = *I;
> SymbolBody *Body = S.Symbol;
> size_t StrOff = S.StrTabOffset;
>
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -247,7 +247,8 @@
> if (Config->Rela)
> P->r_addend = getAddend<ELFT>(Rel);
> P->r_offset = RelocatedSection->getOffset(Rel.r_offset);
> - P->setSymbolAndType(Body.DynsymIndex, Type, Config->Mips64EL);
> + P->setSymbolAndType(In<ELFT>::SymTab->getSymbolIndex(&Body), Type,
> + Config->Mips64EL);
> }
> }
>
> Index: ELF/InputFiles.h
> ===================================================================
> --- ELF/InputFiles.h
> +++ ELF/InputFiles.h
> @@ -180,10 +180,6 @@
> // R_MIPS_GPREL16 / R_MIPS_GPREL32 relocations.
> uint32_t MipsGp0 = 0;
>
> - // The number is the offset in the string table. It will be used as the
> - // st_name of the symbol.
> - std::vector<std::pair<const DefinedRegular<ELFT> *, unsigned>> KeptLocalSyms;
> -
> // Name of source file obtained from STT_FILE symbol value,
> // or empty string if there is no such symbol in object file
> // symbol table.
More information about the llvm-commits
mailing list