[llvm-readobj] - Don't stop dumping when the name of a relocation section can't be read.

[grimar]

Summary

This removes undesired unwrapOrError calls from printRelocations() methods.

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

6137–6138

This code looks like something that could be shared both with the GNU style and possibly even more widely with other places getting section names.

[grimar]

I'll update it after we land D84526 and D83935.

[grimar]

• Addressed review comments (introduced getPrintableSectionName).

llvm/tools/llvm-readobj/ELFDumper.cpp

6137–6138

Done.

[jhenderson]

LGTM, thanks!

llvm/test/tools/llvm-readobj/ELF/dependent-libraries.test

 # MIX-GNU-EMPTY:
 # MIX-GNU-NEXT: warning: '[[FILE]]': SHT_LLVM_DEPENDENT_LIBRARIES section at index 4 is broken: section [index 4] has a sh_offset (0xffff0000) + sh_size (0x4) that is greater than the file size (0x308)
 # MIX-GNU-NEXT: Dependent libraries section .deplibs.broken.shoffset at offset 0xffff0000 contains 0 entries:
 # MIX-GNU-EMPTY:
 # MIX-GNU-NEXT: Dependent libraries section .deplibs.multiple at offset 0x4b contains 2 entries:
 # MIX-GNU-NEXT:   [     0]  bar
 # MIX-GNU-NEXT:   [     4]  xxx
 # MIX-GNU-EMPTY:
-# MIX-GNU-NEXT: warning: '[[FILE]]': cannot get section name of SHT_LLVM_DEPENDENT_LIBRARIES section: a section [index 6] has an invalid sh_name (0x10000) offset which goes past the end of the section name string table
+# MIX-GNU-NEXT: warning: '[[FILE]]': unable to get the name of SHT_LLVM_DEPENDENT_LIBRARIES section with index 6: a section [index 6] has an invalid sh_name (0x10000) offset which goes past the end of the section name string table
 # MIX-GNU-NEXT: Dependent libraries section <?> at offset 0x53 contains 1 entries:
 # MIX-GNU-NEXT:   [     0]  baz
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL

llvm/test/tools/llvm-readobj/ELF/invalid-shstrndx.test

 # GNU:      Section to Segment mapping:
 # GNU-NEXT:   Segment Sections...
 # GNU-NEXT: error: '[[FILE]]': section header string table index 255 does not exist
 
 # LLVM:      ElfHeader {
 # LLVM:        StringTableSectionIndex: 255
 # LLVM-NEXT: }
 # LLVM-NEXT: Sections [
-# LLVM-NEXT: warning: '[[FILE]]': section header string table index 255 does not exist
 # LLVM-NEXT: Section {
 # LLVM-NEXT:   Index: 0
+# LLVM-NEXT: warning: '[[FILE]]': unable to get the name of SHT_NULL section with index 0: section header string table index 255 does not exist
 # LLVM-NEXT:   Name: <?> (0)
 # LLVM-NEXT:   Type: SHT_NULL (0x0)
 # LLVM-NEXT:   Flags [ (0x0)
 # LLVM-NEXT:   ]
 # LLVM-NEXT:   Address: 0x0
 # LLVM-NEXT:   Offset: 0x0
 # LLVM-NEXT:   Size: 0
 # LLVM-NEXT:   Link: 0
 # LLVM-NEXT:   Info: 0
 # LLVM-NEXT:   AddressAlignment: 0
 # LLVM-NEXT:   EntrySize: 0
 # LLVM-NEXT: }
 # LLVM:      Index: 1
+# LLVM-NEXT: warning: '[[FILE]]': unable to get the name of SHT_STRTAB section with index 1: section header string table index 255 does not exist
 # LLVM-NEXT: Name: <?> (11)
 # LLVM:      Index: 2
+# LLVM-NEXT: warning: '[[FILE]]': unable to get the name of SHT_STRTAB section with index 2: section header string table index 255 does not exist
 # LLVM-NEXT: Name: <?> (1)
 
 --- !ELF
 FileHeader:
   Class:     ELFCLASS64
   Data:      ELFDATA2LSB
   Type:      ET_REL
   Machine:   EM_X86_64
   EShStrNdx: 0xFF

llvm/test/tools/llvm-readobj/ELF/relocations.test

FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name: .text
     Type: SHT_PROGBITS
     Content: '00FFFFFFFF020000001234567890ABCDEF'
-  - Name: .rel.text
-    Type: SHT_REL
-    Info: .text
+  - Name:   .rel.text
+    Type:   SHT_REL
+    Info:   .text
+    ShName: [[SHNAME=<none>]]
     Relocations:
       - Symbol: rel_0
         Type:   R_X86_64_NONE
       - Offset: 0x1
         Symbol: rel_neg
         Type:   R_X86_64_PC32
       - Offset: 0x5
         Symbol: rel_pos
         Type:   R_X86_64_PLT32
       - Offset: 0x9
         Symbol: rel_64
         Type:   R_X86_64_64
   - Name: .rela.text
     Type: SHT_RELA
     Info: .text
+    ShName: [[SHNAME=<none>]]
     Relocations:
       - Symbol: rela_0
         Type:   R_X86_64_NONE
         Addend: 0
       - Offset: 0x1
         Symbol: rela_neg
         Type:   R_X86_64_PC32
         Addend: -1
- Name:    rela_pos
     Section: .text
     Value:   2
   - Name:    rela_minneg
     Section: .text
     Value:   3
   - Name:    rela_maxpos
     Section: .text
     Value:   0xFFFFFFFF
+
+## Check we report a warning when the name of a relocation section can't be read. Check we continue dumping.
+# RUN: yaml2obj %s --docnum=1 -DSHNAME=0xffffffff -o %tshname
+# RUN: llvm-readobj --relocs %tshname 2>&1 | FileCheck %s -DFILE=%tshname --check-prefix=LLVM-SHNAME
+# RUN: llvm-readelf --relocs %tshname 2>&1 | FileCheck %s -DFILE=%tshname --check-prefix=GNU-SHNAME
+
+# LLVM-SHNAME:      Relocations [
+# LLVM-SHNAME-NEXT: warning: '[[FILE]]': unable to get the name of SHT_REL section with index 2: a section [index 2] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
+# LLVM-SHNAME-NEXT:   Section (2) <?> {
+# LLVM-SHNAME-NEXT:     0x0 R_X86_64_NONE rel_0 0x0
+# LLVM-SHNAME-NEXT:     0x1 R_X86_64_PC32 rel_neg 0x0
+# LLVM-SHNAME-NEXT:     0x5 R_X86_64_PLT32 rel_pos 0x0
+# LLVM-SHNAME-NEXT:     0x9 R_X86_64_64 rel_64 0x0
+# LLVM-SHNAME-NEXT:   }
+# LLVM-SHNAME-NEXT: warning: '[[FILE]]': unable to get the name of SHT_RELA section with index 3: a section [index 3] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
+# LLVM-SHNAME-NEXT:   Section (3) <?> {
+# LLVM-SHNAME-NEXT:     0x0 R_X86_64_NONE rela_0 0x0
+# LLVM-SHNAME-NEXT:     0x1 R_X86_64_PC32 rela_neg 0xFFFFFFFFFFFFFFFF
+# LLVM-SHNAME-NEXT:     0x5 R_X86_64_PLT32 rela_pos 0x2
+# LLVM-SHNAME-NEXT:     0xFFFFFFFFFFFFFFFF R_X86_64_64 rela_minneg 0x8000000000000000
+# LLVM-SHNAME-NEXT:     0x9 R_X86_64_32S rela_maxpos 0x7FFFFFFFFFFFFFFF
+# LLVM-SHNAME-NEXT:   }
+# LLVM-SHNAME-NEXT: ]
+
+# GNU-SHNAME:      warning: '[[FILE]]': unable to get the name of SHT_REL section with index 2: a section [index 2] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
+# GNU-SHNAME:      Relocation section '<?>' at offset 0x51 contains 4 entries:
+# GNU-SHNAME-NEXT:     Offset             Info             Type               Symbol's Value  Symbol's Name
+# GNU-SHNAME-NEXT: 0000000000000000  0000000100000000 R_X86_64_NONE          0000000000000000 rel_0
+# GNU-SHNAME-NEXT: 0000000000000001  0000000200000002 R_X86_64_PC32          0000000000000001 rel_neg
+# GNU-SHNAME-NEXT: 0000000000000005  0000000300000004 R_X86_64_PLT32         0000000000000002 rel_pos
+# GNU-SHNAME-NEXT: 0000000000000009  0000000400000001 R_X86_64_64            ffffffffffffffff rel_64
+# GNU-SHNAME:      warning: '[[FILE]]': unable to get the name of SHT_RELA section with index 3: a section [index 3] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
+# GNU-SHNAME:      Relocation section '<?>' at offset 0x91 contains 5 entries:
+# GNU-SHNAME-NEXT:     Offset             Info             Type       Symbol's Value  Symbol's Name + Addend
+# GNU-SHNAME-NEXT: 0000000000000000  0000000500000000 R_X86_64_NONE  0000000000000000 rela_0 + 0
+# GNU-SHNAME-NEXT: 0000000000000001  0000000600000002 R_X86_64_PC32  0000000000000001 rela_neg - 1
+# GNU-SHNAME-NEXT: 0000000000000005  0000000700000004 R_X86_64_PLT32 0000000000000002 rela_pos + 2
+# GNU-SHNAME-NEXT: ffffffffffffffff  0000000800000001 R_X86_64_64    0000000000000003 rela_minneg - 8000000000000000
+# GNU-SHNAME-NEXT: 0000000000000009  000000090000000b R_X86_64_32S   ffffffffffffffff rela_maxpos + 7fffffffffffffff

llvm/tools/llvm-readobj/ELFDumper.cpp

virtual void printRelReloc(const ELFO *Obj, unsigned SecIndex,
                              const Elf_Shdr *SymTab, const Elf_Rel &R,
                              unsigned RelIndex) = 0;
   virtual void printRelaReloc(const ELFO *Obj, unsigned SecIndex,
                               const Elf_Shdr *SymTab, const Elf_Rela &R,
                               unsigned RelIndex) = 0;
   virtual void printRelrReloc(const Elf_Relr &R) = 0;
   void printRelocationsHelper(const ELFFile<ELFT> *Obj, const Elf_Shdr &Sec);
 
+  StringRef getPrintableSectionName(const ELFFile<ELFT> *Obj,
+                                    const Elf_Shdr &Sec) const;
+
   void reportUniqueWarning(Error Err) const;
   StringRef FileName;
 
 private:
   const ELFDumper<ELFT> *Dumper;
 };
 
 template <typename ELFT> class GNUStyle : public DumpStyle<ELFT> {

 
 template <class ELFT> void GNUStyle<ELFT>::printRelocations(const ELFO *Obj) {
   bool HasRelocSections = false;
   for (const Elf_Shdr &Sec : cantFail(Obj->sections())) {
     if (!isRelocationSec<ELFT>(Sec))
       continue;
     HasRelocSections = true;
 
-    StringRef Name = unwrapOrError(this->FileName, Obj->getSectionName(&Sec));
     unsigned Entries;
     // Android's packed relocation section needs to be unpacked first
     // to get the actual number of entries.
     if (Sec.sh_type == ELF::SHT_ANDROID_REL ||
         Sec.sh_type == ELF::SHT_ANDROID_RELA) {
       Entries = unwrapOrError(this->FileName, Obj->android_relas(&Sec)).size();
     } else if (!opts::RawRelr && (Sec.sh_type == ELF::SHT_RELR ||
                                   Sec.sh_type == ELF::SHT_ANDROID_RELR)) {
       Elf_Relr_Range Relrs = unwrapOrError(this->FileName, Obj->relrs(&Sec));
       Entries = unwrapOrError(this->FileName, Obj->decode_relrs(Relrs)).size();
     } else {
       Entries = Sec.getEntityCount();
     }
 
     uintX_t Offset = Sec.sh_offset;
+    StringRef Name = getPrintableSectionName(Obj, Sec);
     OS << "\nRelocation section '" << Name << "' at offset 0x"
        << to_hexString(Offset, false) << " contains " << Entries
        << " entries:\n";
     printRelocHeader(Sec.sh_type);
     this->printRelocationsHelper(Obj, Sec);
   }
   if (!HasRelocSections)
     OS << "\nThere are no relocations in this file.\n";
void GNUStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
   printSectionDescription(OS, Obj->getHeader()->e_machine);
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printSymtabMessage(const ELFO *Obj, const Elf_Shdr *Symtab,
                                         size_t Entries,
                                         bool NonVisibilityBitsUsed) {
   StringRef Name;
-  if (Symtab) {
-    if (Expected<StringRef> NameOrErr = Obj->getSectionName(Symtab)) {
-      Name = *NameOrErr;
-    } else {
-      this->reportUniqueWarning(createError("unable to get the name of " +
-                                            describe(Obj, *Symtab) + ": " +
-                                            toString(NameOrErr.takeError())));
-      Name = "<?>";
-    }
-  }
-
+  if (Symtab)
+    Name = getPrintableSectionName(Obj, *Symtab);
   if (!Name.empty())
     OS << "\nSymbol table '" << Name << "'";
   else
     OS << "\nSymbol table for image";
   OS << " contains " << Entries << " entries:\n";
 
   if (ELFT::Is64Bits)
     OS << "   Num:    Value          Size Type    Bind   Vis";
case ELF::SHT_ANDROID_RELA:
     for (const Elf_Rela &R :
          unwrapOrError(this->FileName, Obj->android_relas(&Sec)))
       printRelaReloc(Obj, SecNdx, SymTab, R, ++RelNdx);
     break;
   }
 }
 
 template <class ELFT>
+StringRef DumpStyle<ELFT>::getPrintableSectionName(const ELFFile<ELFT> *Obj,
+                                                   const Elf_Shdr &Sec) const {
+  StringRef Name = "<?>";
+  if (Expected<StringRef> SecNameOrErr =
+          Obj->getSectionName(&Sec, this->dumper()->WarningHandler))
+    Name = *SecNameOrErr;
+  else
+    this->reportUniqueWarning(createError("unable to get the name of " +
+                                          describe(Obj, Sec) + ": " +
+                                          toString(SecNameOrErr.takeError())));
+  return Name;
+}
+
+template <class ELFT>
 void GNUStyle<ELFT>::printDependentLibs(const ELFFile<ELFT> *Obj) {
   bool SectionStarted = false;
   struct NameOffset {
     StringRef Name;
     uint64_t Offset;
   };
   std::vector<NameOffset> SecEntries;
   NameOffset Current;
   auto PrintSection = [&]() {
     OS << "Dependent libraries section " << Current.Name << " at offset "
        << format_hex(Current.Offset, 1) << " contains " << SecEntries.size()
        << " entries:\n";
     for (NameOffset Entry : SecEntries)
       OS << "  [" << format("%6tx", Entry.Offset) << "]  " << Entry.Name
          << "\n";
     OS << "\n";
     SecEntries.clear();
   };
 
   auto OnSectionStart = [&](const Elf_Shdr &Shdr) {
     if (SectionStarted)
       PrintSection();
     SectionStarted = true;
     Current.Offset = Shdr.sh_offset;
-    Expected<StringRef> Name = Obj->getSectionName(&Shdr);
-    if (!Name) {
-      Current.Name = "<?>";
-      this->reportUniqueWarning(
-          createError("cannot get section name of "
-                      "SHT_LLVM_DEPENDENT_LIBRARIES section: " +
-                      toString(Name.takeError())));
-    } else {
-      Current.Name = *Name;
-    }
+    Current.Name = getPrintableSectionName(Obj, Shdr);
   };
   auto OnLibEntry = [&](StringRef Lib, uint64_t Offset) {
     SecEntries.push_back(NameOffset{Lib, Offset});
   };
 
   this->printDependentLibsHelper(Obj, OnSectionStart, OnLibEntry);
   if (SectionStarted)
     PrintSection();

 
 template <class ELFT> void LLVMStyle<ELFT>::printRelocations(const ELFO *Obj) {
   ListScope D(W, "Relocations");
 
   for (const Elf_Shdr &Sec : cantFail(Obj->sections())) {
     if (!isRelocationSec<ELFT>(Sec))
       continue;
 
-    StringRef Name = unwrapOrError(this->FileName, Obj->getSectionName(&Sec));
+    StringRef Name = getPrintableSectionName(Obj, Sec);
     unsigned SecNdx = &Sec - &cantFail(Obj->sections()).front();
     W.startLine() << "Section (" << SecNdx << ") " << Name << " {\n";
     W.indent();
     this->printRelocationsHelper(Obj, Sec);
     W.unindent();
     W.startLine() << "}\n";
   }
 }
 

 template <class ELFT>
 void LLVMStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
   ListScope SectionsD(W, "Sections");
 
   int SectionIndex = -1;
   std::vector<EnumEntry<unsigned>> FlagsList =
       getSectionFlagsForTarget(Obj->getHeader()->e_machine);
   for (const Elf_Shdr &Sec : cantFail(Obj->sections())) {
-    StringRef Name = "<?>";
-    if (Expected<StringRef> SecNameOrErr =
-            Obj->getSectionName(&Sec, this->dumper()->WarningHandler))
-      Name = *SecNameOrErr;
-    else
-      this->reportUniqueWarning(SecNameOrErr.takeError());
-
     DictScope SectionD(W, "Section");
     W.printNumber("Index", ++SectionIndex);
-    W.printNumber("Name", Name, Sec.sh_name);
+    W.printNumber("Name", getPrintableSectionName(Obj, Sec), Sec.sh_name);
     W.printHex(
         "Type",
         object::getELFSectionTypeName(Obj->getHeader()->e_machine, Sec.sh_type),
         Sec.sh_type);
     W.printFlags("Flags", Sec.sh_flags, makeArrayRef(FlagsList));
     W.printHex("Address", Sec.sh_addr);
     W.printHex("Offset", Sec.sh_offset);
     W.printNumber("Size", Sec.sh_size);