[PATCH] D75498: [llvm-objdump] Add option to sort symbols during disassembly

[Stephen Neuendorffer via Phabricator via llvm-commits]

stephenneuendorffer marked 3 inline comments as done.
stephenneuendorffer added a comment.

The point of this is to disassemble symbols in a section in a fixed order.  This might be useful because linking has moved symbols around, or because some other linker disassembles symbols in alphabetical order and one wants to compare.  Basically, I found this useful and I thought other people might.  It was much easier to modify llvm-objdump directly than to post-process the disassembly.



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:299
+static cl::opt<bool> Sort(
+    "sort",
+    cl::desc(
----------------
rupprecht wrote:
> Just `--sort` does not seem to be a specific flag. e.g. should we sort section headers by name? Or what about symbols *not* when disassembling, i.e. `llvm-objdump --syms --sort`
I agree, but struggled what else to actually name it.  --sort-symbols-when-disassembling?  --sort-symbols, and then change other corresponding modes?  I'm new to this code, so I'm not sure what other modes are interesting/appropriate.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1332-1335
+    SortedSectionSymbolsTy sortedSymbols;
     for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
+      sortedSymbols.emplace_back(Symbols[SI].Name, SI);
+    }
----------------
rupprecht wrote:
> `sortedSymbols` should be preallocated (`sortedSymbols.resize(Symbols.size())`) to avoid the performance hit when increasing the size every time.
Good point.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1343-1344
       std::string SymbolName = Symbols[SI].Name.str();
       if (Demangle)
         SymbolName = demangle(SymbolName);
 
----------------
rupprecht wrote:
> Demangling should happen earlier, so that the symbols are also sorted when demangled
Makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75498/new/

https://reviews.llvm.org/D75498