[PATCH] D45325: [ELF] - Reorder local symbols.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1571
+  // the STT_FILE symbols, they are already naturally placed first in each group.
+  std::stable_sort(Symbols.begin(), E,
+                   [&](const SymbolTableEntry &L, const SymbolTableEntry &R) {
----------------
espindola wrote:
> ruiu wrote:
> > How does this guarantee that a local symbol group for some file always starts with a STT_FILE symbol?
> It is not obvious and needs a comment, but it should work:
> 
> We first add local symbols to the symbol table in copyLocalSymbols. Each STT_FILE in the original .o will precede other local symbols.
> 
> The hidden symbols are added in finalizeSections, so they are also after STT_FILE.
> 
> Finally this uses a stable_sort, so the above properties remain.
> 
> One thing that is not clear is where do we put STT_SECTION created by addSectionSymbols.
> 
> Also, could we merge the partition and the sort?
> 
> One thing that is not clear is where do we put STT_SECTION created by addSectionSymbols.

We create them before hidden symbols, hence they are placed before them. I updated the test case.

Should we place them right after the STT_FILE? It seems a bit nicer,
though they are created only for --emit-relocs and -r, so I am not sure we should care.


https://reviews.llvm.org/D45325