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

[grimar]

Summary

We have no good test for --needed-libs option.
The one we have as a part of Object/readobj-shared-object.test
is not complete.

In this patch I've did a minor NFC changes to the implementation and
added a test. This allowed to remove this piece from
Object/readobj-shared-object.test

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/needed-libs.test

10

libraries -> library

34

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.

59

when the dynamic...

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

llvm/tools/llvm-readobj/ELFDumper.cpp

2474

What's the reasoning for this change? Won't this potentially change the behaviour where two DT_NEEDED entries have the same string?

2476

StringRef?

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

2474

We have a std::vector<std::string> here and sorting it to print below.
What is the point to use stable_sort?
If we have "aaa", "bbb", "aaa" then we might end up with one of the following:

1. "aaa" (1), "aaa" (2), "bbb"
2. "aaa" (2), "aaa" (1), "bbb"

May be I am missing something, but what is the difference? :)

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

2476

(I think keeping the original type does not hurt.)

[grimar]

• Addressed review comments.

llvm/tools/llvm-readobj/ELFDumper.cpp

2476

I'd also keep the original type. Having a StringRef here is not faster, as adds a one more constructor call,
but I see no benefits from that. It does not improve readability. Instead it raises a question
"why a conversion to StringRef is needed here, is there something wrong with operator << for std::string?"

[jhenderson]

LGTM.

llvm/tools/llvm-readobj/ELFDumper.cpp

2474

Good point!

llvm/test/Object/readobj-shared-object.test

-# RUN: llvm-readobj --sections --symbols --dyn-syms --needed-libs \
+# RUN: llvm-readobj --sections --symbols --dyn-syms \
 # RUN:   %p/Inputs/shared-object-test.elf-i386 | FileCheck %s
 
-# RUN: llvm-readobj --sections --symbols --dyn-syms --needed-libs \
+# RUN: llvm-readobj --sections --symbols --dyn-syms \
 # RUN:   %p/Inputs/shared-object-test.elf-x86-64 | FileCheck %s
 
 # CHECK:   Sections [
 # CHECK:   Section {
 # CHECK:     Name:  (0)
 # CHECK:     Type: SHT_NULL
 # CHECK:     Flags [ (0x0)
 # CHECK:     ]

 # CHECK:   }
 # CHECK:   Symbol {
 # CHECK:     Name: _edata
 # CHECK:     Binding: Global
 # CHECK:     Type: None
 # CHECK:     Section: Absolute (0xFFF1)
 # CHECK:   }
 # CHECK: ]
-
-# CHECK:      NeededLibraries [
-# CHECK-NEXT:  libc.so.6
-# CHECK-NEXT:  libm.so.6
-# CHECK-NEXT: ]

llvm/test/tools/llvm-readobj/ELF/needed-libs.test

+## In this test we check the --needed-libs option.
+
+# RUN: yaml2obj %s --docnum=1 -o %t1
+# RUN: llvm-readobj --needed-libs %t1 \
+# RUN:   | FileCheck %s --strict-whitespace --match-full-lines --check-prefix=NEEDED-LIBS
+# RUN: llvm-readelf --needed-libs %t1 \
+# RUN:   | FileCheck %s --strict-whitespace --match-full-lines --check-prefix=NEEDED-LIBS
+
+## Check that library names are sorted when printed.
+## Document that we also sort error entries.
+
+#      NEEDED-LIBS:NeededLibraries [
+# NEEDED-LIBS-NEXT:  <Invalid offset 0x1111111>
+# NEEDED-LIBS-NEXT:  <Invalid offset 0x9999999>
+# NEEDED-LIBS-NEXT:  aaa
+# NEEDED-LIBS-NEXT:  bbb
+# NEEDED-LIBS-NEXT:  ccc
+# NEEDED-LIBS-NEXT:]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:  .dynstr
+    Type:  SHT_STRTAB
+    Flags: [ SHF_ALLOC ]
+    Content: '00616161006262620063636300' ## 0,a,a,a,0,b,b,b,0,c,c,c,0
+  - Name:    .dynamic
+    Type:    SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_STRTAB
+        Value: 0x0000000000000000
+      - Tag:   DT_NEEDED
+        Value: 0x9999999
+      - Tag:   DT_NEEDED
+        Value: 9
+      - Tag:   DT_NEEDED
+        Value: 1
+      - Tag:   DT_NEEDED
+        Value: 5
+      - Tag:   DT_NEEDED
+        Value: 0x1111111
+      - Tag:   DT_STRSZ
+        Value: 0xD
+      - Tag:   DT_NULL
+        Value: 0x0
+ProgramHeaders:
+  - Type: PT_LOAD
+    VAddr: 0x0
+    Sections:
+      - Section: .dynstr
+      - Section: .dynamic
+
+## Check what we print when the dynamic string table is empty.
+# RUN: yaml2obj %s --docnum=2 -o %t2
+# RUN: llvm-readobj --needed-libs %t2 | FileCheck %s --check-prefix=EMPTY-DYNSTR
+# RUN: llvm-readelf --needed-libs %t2 | FileCheck %s --check-prefix=EMPTY-DYNSTR
+
+# EMPTY-DYNSTR:      NeededLibraries [
+# EMPTY-DYNSTR-NEXT:   <String table is empty or was not found>
+# EMPTY-DYNSTR-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:  .dynstr
+    Type:  SHT_STRTAB
+    Flags: [ SHF_ALLOC ]
+    Size:  0
+  - Name:    .dynamic
+    Type:    SHT_DYNAMIC
+    Address: 0x1000
+    Entries:
+      - Tag:   DT_STRTAB
+        Value: 0x0000000000000000
+      - Tag:   DT_NEEDED
+        Value: 1
+      - Tag:   DT_STRSZ
+        Value: 0x0
+      - Tag:   DT_NULL
+        Value: 0x0
+ProgramHeaders:
+  - Type: PT_LOAD
+    VAddr: 0x0
+    Sections:
+      - Section: .dynstr
+      - Section: .dynamic

llvm/tools/llvm-readobj/ELFDumper.cpp

 template <class ELFT> void ELFDumper<ELFT>::printNeededLibraries() {
   ListScope D(W, "NeededLibraries");
 
   std::vector<std::string> Libs;
   for (const auto &Entry : dynamic_table())
     if (Entry.d_tag == ELF::DT_NEEDED)
       Libs.push_back(getDynamicString(Entry.d_un.d_val));
 
-  llvm::stable_sort(Libs);
+  llvm::sort(Libs);
 
-  for (const auto &L : Libs)
+  for (const std::string &L : Libs)
     W.startLine() << L << "\n";
 }
 
 template <typename ELFT> void ELFDumper<ELFT>::printHashTable() {
   DictScope D(W, "HashTable");
   if (!HashTable)
     return;
   W.printNumber("Num Buckets", HashTable->nbucket);