[NFC][llvm-readobj] Split getSectionIndexName function into two

getSectionIndexName was trying to fetch two things at once, which led to
a somewhat tricky to understand interface involving passing output
parameters in, and also made it hard to return Errors further up the
stack.

This change is in preparation for changing the error handling.

Additionally, update a related test now that yaml2obj supports
SHT_SYMTAB_SHNDX properly (see d3963051c490), and add missing LLVM-style
coverage for symbols with shndx SHN_XINDEX. This test (after fixing)
caught a mistake in my first attempt at this patch, hence I'm including
it as part of this patch.

Reviewed by: grimar, MaskRay

Differential Revision: https://reviews.llvm.org/D69670

diff --git a/llvm/test/tools/llvm-readobj/elf-symbol-shndx.test b/llvm/test/tools/llvm-readobj/elf-symbol-shndx.test
index bdf2050..da3d1fa 100644 (file)
--- a/llvm/test/tools/llvm-readobj/elf-symbol-shndx.test
+++ b/llvm/test/tools/llvm-readobj/elf-symbol-shndx.test
@@ -3,37 +3,41 @@
 # section), reserved values and processor/os-specific index values, for both GNU
 # and LLVM styles.
 
-# Use --dyn-symbols because yaml2obj does not currently support large section indexes
-# and also does not allow hand-crafting of static symbol tables.
 # RUN: yaml2obj --docnum=1 %s > %t1
-# RUN: llvm-readobj --symbols --dyn-symbols %t1 | FileCheck %s --check-prefix=LLVM
-# RUN: llvm-readelf --symbols --dyn-symbols %t1 | FileCheck %s --check-prefix=GNU1
+# RUN: llvm-readobj --symbols %t1 | FileCheck %s --check-prefix=LLVM1
+# RUN: llvm-readelf --symbols %t1 | FileCheck %s --check-prefix=GNU1
 
 # llvm-readobj doesn't accept shndx values that are not valid section indexes
 # for LLVM style, so only test GNU output in this case.
 # RUN: yaml2obj --docnum=2 %s > %t2
 # RUN: llvm-readelf --symbols %t2 | FileCheck %s --check-prefix=GNU2
 
-# LLVM: Name:    undef
-# LLVM: Section: Undefined (0x0)
-# LLVM: Name:    normal
-# LLVM: Section: .text (0x1)
-# LLVM: Name:    common
-# LLVM: Section: Common (0xFFF2)
-# LLVM: Name:    absolute
-# LLVM: Section: Absolute (0xFFF1)
-# LLVM: Name:    proc
-# LLVM: Section: Processor Specific (0xFF01)
-# LLVM: Name:    os
-# LLVM: Section: Operating System Specific (0xFF21)
-# LLVM: Name:    reserved
-# LLVM: Section: Reserved (0xFFFE)
+# LLVM1: Name:    undef
+# LLVM1: Section:
+# LLVM1-SAME:     Undefined (0x0)
+# LLVM1: Name:    normal
+# LLVM1: Section:
+# LLVM1-SAME:     .text (0x1)
+# LLVM1: Name:    common
+# LLVM1: Section:
+# LLVM1-SAME:     Common (0xFFF2)
+# LLVM1: Name:    absolute
+# LLVM1: Section:
+# LLVM1-SAME:     Absolute (0xFFF1)
+# LLVM1: Name:    proc
+# LLVM1: Section:
+# LLVM1-SAME:     Processor Specific (0xFF01)
+# LLVM1: Name:    os
+# LLVM1: Section:
+# LLVM1-SAME:     Operating System Specific (0xFF21)
+# LLVM1: Name:    reserved
+# LLVM1: Section:
+# LLVM1-SAME:     Reserved (0xFFFE)
+# LLVM1: Name:    xindex
+# LLVM1: Section:
+# LLVM1:          .text (0x1)
 
-# GNU1:      Symbol table '.dynsym' contains 2 entries:
-# GNU1-NEXT:   Num: {{.*}} Ndx Name
-# GNU1-NEXT:     0: {{.*}} UND
-# GNU1-NEXT:     1: {{.*}}   1 xindex
-# GNU1:      Symbol table '.symtab' contains 8 entries:
+# GNU1:      Symbol table '.symtab' contains 9 entries:
 # GNU1-NEXT:   Num: {{.*}} Ndx Name
 # GNU1-NEXT:     0: {{.*}} UND
 # GNU1-NEXT:     1: {{.*}} UND undef
@@ -43,6 +47,7 @@
 # GNU1-NEXT:     5: {{.*}} PRC[0xff01] proc
 # GNU1-NEXT:     6: {{.*}} OS[0xff21] os
 # GNU1-NEXT:     7: {{.*}} RSV[0xfffe] reserved
+# GNU1-NEXT:     8: {{.*}}   1 xindex
 
 # GNU2:      Symbol table '.symtab' contains 2 entries:
 # GNU2-NEXT:   Num: {{.*}} Ndx Name
@@ -58,21 +63,10 @@ FileHeader:
 Sections:
   - Name: .text
     Type: SHT_PROGBITS
-  - Name: .dynstr
-    Type: SHT_STRTAB
-    #\0xindex\0
-    Content: "0078696e64657800"
-  - Name: .dynsym
-    Type: SHT_DYNSYM
-    Link: .dynstr
-    EntSize: 16
-    # Null symbol
-    # Symbol with st_name = 1, st_shndx = SHN_XINDEX
-    Content: "000000000000000000000000000000000100000000000000000000000000ffff"
   - Name: .symtab_shndx
     Type: SHT_SYMTAB_SHNDX
-    Link: .dynsym
-    Entries: [ 0, 1 ]
+    Link: .symtab
+    Entries: [ 0, 0, 0, 0, 0, 0, 0, 0, 1 ]
 Symbols:
   - Name:    undef
     Binding: STB_GLOBAL
@@ -94,6 +88,9 @@ Symbols:
   - Name:    reserved
     Index:   0xfffe
     Binding: STB_GLOBAL
+  - Name:    xindex
+    Index:   SHN_XINDEX
+    Binding: STB_GLOBAL
 
 --- !ELF
 FileHeader:

diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 667f092..e48744b 100644 (file)
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -299,9 +299,10 @@ public:
   Elf_Relr_Range dyn_relrs() const;
   std::string getFullSymbolName(const Elf_Sym *Symbol, StringRef StrTable,
                                 bool IsDynamic) const;
-  void getSectionNameIndex(const Elf_Sym *Symbol, const Elf_Sym *FirstSym,
-                           StringRef &SectionName,
-                           unsigned &SectionIndex) const;
+  Expected<unsigned> getSymbolSectionIndex(const Elf_Sym *Symbol,
+                                           const Elf_Sym *FirstSym) const;
+  Expected<StringRef> getSymbolSectionName(const Elf_Sym *Symbol,
+                                           unsigned SectionIndex) const;
   Expected<std::string> getStaticSymbolName(uint32_t Index) const;
   std::string getDynamicString(uint64_t Value) const;
   StringRef getSymbolVersionByIndex(StringRef StrTab,
@@ -821,12 +822,12 @@ std::string ELFDumper<ELFT>::getFullSymbolName(const Elf_Sym *Symbol,
       unwrapOrError(ObjF->getFileName(), Symbol->getName(StrTable)));
 
   if (SymbolName.empty() && Symbol->getType() == ELF::STT_SECTION) {
-    unsigned SectionIndex;
-    StringRef SectionName;
     Elf_Sym_Range Syms = unwrapOrError(
         ObjF->getFileName(), ObjF->getELFFile()->symbols(DotSymtabSec));
-    getSectionNameIndex(Symbol, Syms.begin(), SectionName, SectionIndex);
-    return SectionName;
+    unsigned SectionIndex = unwrapOrError(
+        ObjF->getFileName(), getSymbolSectionIndex(Symbol, Syms.begin()));
+    return unwrapOrError(ObjF->getFileName(),
+                         getSymbolSectionName(Symbol, SectionIndex));
   }
 
   if (!IsDynamic)
@@ -842,33 +843,43 @@ std::string ELFDumper<ELFT>::getFullSymbolName(const Elf_Sym *Symbol,
 }
 
 template <typename ELFT>
-void ELFDumper<ELFT>::getSectionNameIndex(const Elf_Sym *Symbol,
-                                          const Elf_Sym *FirstSym,
-                                          StringRef &SectionName,
-                                          unsigned &SectionIndex) const {
-  SectionIndex = Symbol->st_shndx;
+Expected<unsigned>
+ELFDumper<ELFT>::getSymbolSectionIndex(const Elf_Sym *Symbol,
+                                       const Elf_Sym *FirstSym) const {
+  return Symbol->st_shndx == SHN_XINDEX
+             ? object::getExtendedSymbolTableIndex<ELFT>(Symbol, FirstSym,
+                                                         ShndxTable)
+             : Symbol->st_shndx;
+}
+
+// If the Symbol has a reserved st_shndx other than SHN_XINDEX, return a
+// descriptive interpretation of the st_shndx value. Otherwise, return the name
+// of the section with index SectionIndex. This function assumes that if the
+// Symbol has st_shndx == SHN_XINDEX the SectionIndex will be the value derived
+// from the SHT_SYMTAB_SHNDX section.
+template <typename ELFT>
+Expected<StringRef>
+ELFDumper<ELFT>::getSymbolSectionName(const Elf_Sym *Symbol,
+                                      unsigned SectionIndex) const {
   if (Symbol->isUndefined())
-    SectionName = "Undefined";
-  else if (Symbol->isProcessorSpecific())
-    SectionName = "Processor Specific";
-  else if (Symbol->isOSSpecific())
-    SectionName = "Operating System Specific";
-  else if (Symbol->isAbsolute())
-    SectionName = "Absolute";
-  else if (Symbol->isCommon())
-    SectionName = "Common";
-  else if (Symbol->isReserved() && SectionIndex != SHN_XINDEX)
-    SectionName = "Reserved";
-  else {
-    if (SectionIndex == SHN_XINDEX)
-      SectionIndex = unwrapOrError(ObjF->getFileName(),
-                                   object::getExtendedSymbolTableIndex<ELFT>(
-                                       Symbol, FirstSym, ShndxTable));
-    const ELFFile<ELFT> *Obj = ObjF->getELFFile();
-    const typename ELFT::Shdr *Sec =
-        unwrapOrError(ObjF->getFileName(), Obj->getSection(SectionIndex));
-    SectionName = unwrapOrError(ObjF->getFileName(), Obj->getSectionName(Sec));
-  }
+    return "Undefined";
+  if (Symbol->isProcessorSpecific())
+    return "Processor Specific";
+  if (Symbol->isOSSpecific())
+    return "Operating System Specific";
+  if (Symbol->isAbsolute())
+    return "Absolute";
+  if (Symbol->isCommon())
+    return "Common";
+  if (Symbol->isReserved() && Symbol->st_shndx != SHN_XINDEX)
+    return "Reserved";
+
+  const ELFFile<ELFT> *Obj = ObjF->getELFFile();
+  Expected<const Elf_Shdr *> SecOrErr =
+      Obj->getSection(SectionIndex);
+  if (!SecOrErr)
+    return SecOrErr.takeError();
+  return Obj->getSectionName(*SecOrErr);
 }
 
 template <class ELFO>
@@ -5443,9 +5454,10 @@ void LLVMStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
 template <class ELFT>
 void LLVMStyle<ELFT>::printSymbolSection(const Elf_Sym *Symbol,
                                          const Elf_Sym *First) {
-  unsigned SectionIndex = 0;
-  StringRef SectionName;
-  this->dumper()->getSectionNameIndex(Symbol, First, SectionName, SectionIndex);
+  unsigned SectionIndex = unwrapOrError(
+      this->FileName, this->dumper()->getSymbolSectionIndex(Symbol, First));
+  StringRef SectionName = unwrapOrError(
+      this->FileName, this->dumper()->getSymbolSectionName(Symbol, SectionIndex));
   W.printHex("Section", SectionName, SectionIndex);
 }