[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Seems fine to me from a general perspective. I think others have already checked the windows parts.



================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:892
   }
+  std::sort(export_list.begin(), export_list.end(), RVASymbolListCompareRVA);
+  return export_list;
----------------
alvinhochun wrote:
> labath wrote:
> > Can you have multiple symbols pointing to the same address? Make this should use `stable_sort` instead?
> Yes, it can happen. The ordering shouldn't affect what AppendFromCOFFSymbolTable does but I suppose stable_sort does make it more deterministic to avoid future issues down the line.
Yeah, it's better to avoid having this kind of nondeterministic behavior that can differ from run to run. LLDB is not so string about it as clang/llvm, but it's still a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134426