[PATCH] D41527: [llvm-readobj] Support -needed-libs option for Mach-O files

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: tools/llvm-readobj/MachODumper.cpp:701
+
+  std::stable_sort(Libs.begin(), Libs.end());
+
----------------
davide wrote:
> phosek wrote:
> > davide wrote:
> > > phosek wrote:
> > > > davide wrote:
> > > > > Why you need this stable sort?
> > > > I have followed the ELF implementation which also uses stable sort, I'm not sure why it's used there.
> > > I'd rather not cargo cult this here unless we have a good reason to. We shouldn't need this call, should we?
> > I think having a stable output order may be useful in some cases. Also if that's what we're already doing for ELF version, we should probably be consistent across all formats?
> I think we should have a stable order of these *regardless*. I'm nervous about adding a call to `std::stable_sort()` unless we have a good reason for it, because it might hide bugs elsewhere in the reader.
Understood, the problem is that `load_command()` doesn't provide any guarantees about the ordering, the order should be the same as the order in which the commands were read from the file, but commands are also not guaranteed to be sorted and after briefly checking a few binaries they definitely aren't.  So the way to guarantee ordering would be to sort `LoadCommands` in `MachOObjectFile` constructor when reading the file, is that what you're suggesting?


Repository:
  rL LLVM

https://reviews.llvm.org/D41527