[PATCH] D35843: [ELF] - Fix "--symbol-ordering-file doesn't work with linker scripts"
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 2 16:40:19 PDT 2017
This LGTM.
I think there are two potential improvements, but they can be done as
followups:
* Removing the switch over Config->EKind. This depends on D35936.
* Moving the sort out of LinkerScript::computeInputSections.
The second one should allow sharing the code path for the script and non
script cases.
Cheers,
Rafael
George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
> grimar updated this revision to Diff 109332.
> grimar added a comment.
>
> - Addressed review comments.
>
>
> https://reviews.llvm.org/D35843
>
> Files:
> ELF/InputSection.cpp
> ELF/InputSection.h
> ELF/LinkerScript.cpp
> ELF/OutputSections.cpp
> ELF/OutputSections.h
> ELF/Writer.cpp
> test/ELF/linkerscript/symbol-ordering-file.s
>
> Index: test/ELF/linkerscript/symbol-ordering-file.s
> ===================================================================
> --- test/ELF/linkerscript/symbol-ordering-file.s
> +++ test/ELF/linkerscript/symbol-ordering-file.s
> @@ -0,0 +1,23 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: echo "SECTIONS { .foo : { *(.foo) } }" > %t.script
> +
> +# RUN: ld.lld %t.o --script %t.script -o %t.out
> +# RUN: llvm-objdump -s %t.out| FileCheck %s --check-prefix=BEFORE
> +# BEFORE: Contents of section .foo:
> +# BEFORE-NEXT: 1122
> +
> +# RUN: echo "_foo2" > %t.ord
> +# RUN: echo "_foo1" >> %t.ord
> +# RUN: ld.lld --symbol-ordering-file %t.ord %t.o --script %t.script -o %t2.out
> +# RUN: llvm-objdump -s %t2.out| FileCheck %s --check-prefix=AFTER
> +# AFTER: Contents of section .foo:
> +# AFTER-NEXT: 2211
> +
> +.section .foo,"ax", at progbits,unique,1
> +_foo1:
> + .byte 0x11
> +
> +.section .foo,"ax", at progbits,unique,2
> +_foo2:
> + .byte 0x22
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -870,27 +870,8 @@
> if (Config->SymbolOrderingFile.empty())
> return;
>
> - // Build a map from symbols to their priorities. Symbols that didn't
> - // appear in the symbol ordering file have the lowest priority 0.
> - // All explicitly mentioned symbols have negative (higher) priorities.
> - DenseMap<StringRef, int> SymbolOrder;
> - int Priority = -Config->SymbolOrderingFile.size();
> - for (StringRef S : Config->SymbolOrderingFile)
> - SymbolOrder.insert({S, Priority++});
> -
> - // Build a map from sections to their priorities.
> - DenseMap<SectionBase *, int> SectionOrder;
> - for (ObjFile<ELFT> *File : ObjFile<ELFT>::Instances) {
> - for (SymbolBody *Body : File->getSymbols()) {
> - auto *D = dyn_cast<DefinedRegular>(Body);
> - if (!D || !D->Section)
> - continue;
> - int &Priority = SectionOrder[D->Section];
> - Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
> - }
> - }
> -
> // Sort sections by priority.
> + DenseMap<SectionBase *, int> SectionOrder = buildSectionOrder<ELFT>();
> for (BaseCommand *Base : Script->Opt.Commands)
> if (auto *Sec = dyn_cast<OutputSection>(Base))
> Sec->sort([&](InputSectionBase *S) { return SectionOrder.lookup(S); });
> Index: ELF/OutputSections.h
> ===================================================================
> --- ELF/OutputSections.h
> +++ ELF/OutputSections.h
> @@ -170,6 +170,8 @@
>
> uint64_t getHeaderSize();
> void reportDiscarded(InputSectionBase *IS);
> +void sortByOrder(llvm::MutableArrayRef<InputSection *> In,
> + std::function<int(InputSectionBase *S)> Order);
>
> extern std::vector<OutputSection *> OutputSections;
> } // namespace elf
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -184,6 +184,20 @@
> Type == SHT_NOTE;
> }
>
> +void elf::sortByOrder(MutableArrayRef<InputSection *> In,
> + std::function<int(InputSectionBase *S)> Order) {
> + typedef std::pair<int, InputSection *> Pair;
> + auto Comp = [](const Pair &A, const Pair &B) { return A.first < B.first; };
> +
> + std::vector<Pair> V;
> + for (InputSection *S : In)
> + V.push_back({Order(S), S});
> + std::stable_sort(V.begin(), V.end(), Comp);
> +
> + for (size_t I = 0; I < V.size(); ++I)
> + In[I] = V[I].second;
> +}
> +
> void elf::reportDiscarded(InputSectionBase *IS) {
> if (!Config->PrintGcSections)
> return;
> @@ -291,18 +305,8 @@
> }
>
> void OutputSection::sort(std::function<int(InputSectionBase *S)> Order) {
> - typedef std::pair<int, InputSection *> Pair;
> - auto Comp = [](const Pair &A, const Pair &B) { return A.first < B.first; };
> -
> - std::vector<Pair> V;
> assert(Commands.size() == 1);
> - auto *ISD = cast<InputSectionDescription>(Commands[0]);
> - for (InputSection *S : ISD->Sections)
> - V.push_back({Order(S), S});
> - std::stable_sort(V.begin(), V.end(), Comp);
> - ISD->Sections.clear();
> - for (Pair &P : V)
> - ISD->Sections.push_back(P.second);
> + sortByOrder(cast<InputSectionDescription>(Commands[0])->Sections, Order);
> }
>
> // Fill [Buf, Buf + Size) with Filler.
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -228,6 +228,29 @@
> std::stable_sort(Begin, End, getComparator(K));
> }
>
> +static llvm::DenseMap<SectionBase *, int> getSectionOrder() {
> + switch (Config->EKind) {
> + case ELF32LEKind:
> + return buildSectionOrder<ELF32LE>();
> + case ELF32BEKind:
> + return buildSectionOrder<ELF32BE>();
> + case ELF64LEKind:
> + return buildSectionOrder<ELF64LE>();
> + case ELF64BEKind:
> + return buildSectionOrder<ELF64BE>();
> + default:
> + llvm_unreachable("unknown ELF type");
> + }
> +}
> +
> +static void sortBySymbolOrder(InputSection **Begin, InputSection **End) {
> + if (Config->SymbolOrderingFile.empty())
> + return;
> + static llvm::DenseMap<SectionBase *, int> Order = getSectionOrder();
> + MutableArrayRef<InputSection *> In(Begin, End - Begin);
> + sortByOrder(In, [&](InputSectionBase *S) { return Order.lookup(S); });
> +}
> +
> // Compute and remember which sections the InputSectionDescription matches.
> std::vector<InputSection *>
> LinkerScript::computeInputSections(const InputSectionDescription *Cmd) {
> @@ -273,8 +296,15 @@
> // --sort-section is handled as an inner SORT command.
> // 3. If one SORT command is given, and if it is SORT_NONE, don't sort.
> // 4. If no SORT command is given, sort according to --sort-section.
> + // 5. If no SORT commands are given and --sort-section is not specified,
> + // apply sorting provided by --symbol-ordering-file if any exist.
> InputSection **Begin = Ret.data() + SizeBefore;
> InputSection **End = Ret.data() + Ret.size();
> + if (Pat.SortOuter == SortSectionPolicy::Default &&
> + Config->SortSection == SortSectionPolicy::Default) {
> + sortBySymbolOrder(Begin, End);
> + continue;
> + }
> if (Pat.SortOuter != SortSectionPolicy::None) {
> if (Pat.SortInner == SortSectionPolicy::Default)
> sortSections(Begin, End, Config->SortSection);
> Index: ELF/InputSection.h
> ===================================================================
> --- ELF/InputSection.h
> +++ ELF/InputSection.h
> @@ -331,6 +331,9 @@
> // The list of all input sections.
> extern std::vector<InputSectionBase *> InputSections;
>
> +// Builds section order for handling --symbol-ordering-file.
> +template <class ELFT> llvm::DenseMap<SectionBase *, int> buildSectionOrder();
> +
> } // namespace elf
>
> std::string toString(const elf::InputSectionBase *);
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -44,6 +44,29 @@
> return (toString(Sec->File) + ":(" + Sec->Name + ")").str();
> }
>
> +template <class ELFT> DenseMap<SectionBase *, int> elf::buildSectionOrder() {
> + // Build a map from symbols to their priorities. Symbols that didn't
> + // appear in the symbol ordering file have the lowest priority 0.
> + // All explicitly mentioned symbols have negative (higher) priorities.
> + DenseMap<StringRef, int> SymbolOrder;
> + int Priority = -Config->SymbolOrderingFile.size();
> + for (StringRef S : Config->SymbolOrderingFile)
> + SymbolOrder.insert({S, Priority++});
> +
> + // Build a map from sections to their priorities.
> + DenseMap<SectionBase *, int> SectionOrder;
> + for (ObjFile<ELFT> *File : ObjFile<ELFT>::Instances) {
> + for (SymbolBody *Body : File->getSymbols()) {
> + auto *D = dyn_cast<DefinedRegular>(Body);
> + if (!D || !D->Section)
> + continue;
> + int &Priority = SectionOrder[D->Section];
> + Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
> + }
> + }
> + return SectionOrder;
> +}
> +
> template <class ELFT>
> static ArrayRef<uint8_t> getSectionContents(ObjFile<ELFT> *File,
> const typename ELFT::Shdr *Hdr) {
> @@ -982,6 +1005,11 @@
> return Piece.OutputOff + Addend;
> }
>
> +template DenseMap<SectionBase *, int> elf::buildSectionOrder<ELF32LE>();
> +template DenseMap<SectionBase *, int> elf::buildSectionOrder<ELF32BE>();
> +template DenseMap<SectionBase *, int> elf::buildSectionOrder<ELF64LE>();
> +template DenseMap<SectionBase *, int> elf::buildSectionOrder<ELF64BE>();
> +
> template InputSection::InputSection(ObjFile<ELF32LE> *, const ELF32LE::Shdr *,
> StringRef);
> template InputSection::InputSection(ObjFile<ELF32BE> *, const ELF32BE::Shdr *,
More information about the llvm-commits
mailing list