[PATCH] D73174: [llvm-readobj] - Refine --needed-libs implementation and add a test.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/needed-libs.test:9
+
+## Check that libraries names are sorted when printed.
+## Document that we also sort error entries.
----------------
libraries -> library


================
Comment at: llvm/test/tools/llvm-readobj/ELF/needed-libs.test:33
+    Type:    SHT_DYNAMIC
+    Address: 0x1000
+    Entries:
----------------
This address won't match the computed-via-phdr address. I'd either get rid of it, or add an AddressAlign 0x1000, or possibly just set it to 13(?), or something along those lines.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/needed-libs.test:58
+
+## Check what we print when dynamic string table section is empty.
+# RUN: yaml2obj %s --docnum=2 -o %t2
----------------
when the dynamic...

You can probably get rid of the word "section" here.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2474
 
-  llvm::stable_sort(Libs);
+  llvm::sort(Libs);
 
----------------
What's the reasoning for this change? Won't this potentially change the behaviour where two DT_NEEDED entries have the same string?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2476
 
-  for (const auto &L : Libs)
+  for (const std::string &L : Libs)
     W.startLine() << L << "\n";
----------------
StringRef?


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

https://reviews.llvm.org/D73174