[Object/llvm-readelf/llvm-readobj] - Improve error reporting when e_shstrndx is broken.

When e_shstrndx is broken, it is impossible to get a section name.
In this patch I improved the error message we show and 
added tests for Object and for llvm-readelf/llvm-readobj

Message was changed in two places:
1) llvm-readelf/llvm-readobj previously used a code from Object/ELF.h,
now they have a modified version of it (it has less checks and allows
dumping broken things).
2) Code in Object/ELF.h is still used for generic cases.

Differential revision: https://reviews.llvm.org/D64714

llvm-svn: 366203

6 files changed, 65 insertions, 12 deletions

diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index 7bc6dc4620c..cf8e4529bad 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -466,9 +466,10 @@ ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections) const {
 
   if (!Index) // no section string table.
     return "";
+  // TODO: Test a case when the sh_link of the section with index 0 is broken.
   if (Index >= Sections.size())
-    // TODO: this error is untested.
-    return createError("invalid section index");
+    return createError("section header string table index " + Twine(Index) +
+                       " does not exist");
   return getStringTable(&Sections[Index]);
 }
 
diff --git a/llvm/test/Object/invalid.test b/llvm/test/Object/invalid.test
index 97ebb9f8573..5723c4aef33 100644
--- a/llvm/test/Object/invalid.test
+++ b/llvm/test/Object/invalid.test
@@ -536,3 +536,19 @@ ProgramHeaders:
     FileSize: 0xffff0000
     Sections:
       - Section: .dynamic
+
+# RUN: yaml2obj --docnum=25 %s -o %t25
+# RUN: not obj2yaml 2>&1 %t25 | FileCheck %s -DFILE=%t25 --check-prefix=INVALID-SHSTRNDX
+
+# INVALID-SHSTRNDX: Error reading file: [[FILE]]: section header string table index 255 does not exist
+
+--- !ELF
+FileHeader:
+  Class:    ELFCLASS64
+  Data:     ELFDATA2LSB
+  Type:     ET_REL
+  Machine:  EM_X86_64
+  SHStrNdx: 0xFF
+Sections:
+  - Name: .foo
+    Type: SHT_PROGBITS
diff --git a/llvm/test/tools/llvm-readobj/elf-invalid-shstrndx.test b/llvm/test/tools/llvm-readobj/elf-invalid-shstrndx.test
new file mode 100644
index 00000000000..6dc3b11dfd2
--- /dev/null
+++ b/llvm/test/tools/llvm-readobj/elf-invalid-shstrndx.test
@@ -0,0 +1,26 @@
+# RUN: yaml2obj %s -o %t
+# RUN: not llvm-readelf --headers -S 2>&1 %t | FileCheck %s -DFILE=%t --check-prefix=GNU
+# RUN: not llvm-readobj --headers -S 2>&1 %t | FileCheck %s -DFILE=%t --check-prefix=LLVM
+
+# GNU:      ELF Header:
+# GNU:        Section header string table index: 255
+# GNU-NEXT:   There are 4 section headers, starting at offset 0x40:
+# GNU:      Section Headers:
+# GNU-NEXT:  [Nr] Name
+# GNU-EMPTY:
+# GNU-NEXT:  error: '[[FILE]]': section header string table index 255 does not exist
+
+# LLVM:      ElfHeader {
+# LLVM:        StringTableSectionIndex: 255
+# LLVM-NEXT: }
+# LLVM-NEXT: Sections [
+# LLVM-EMPTY:
+# LLVM-NEXT: error: '[[FILE]]': section header string table index 255 does not exist
+
+--- !ELF
+FileHeader:
+  Class:     ELFCLASS64
+  Data:      ELFDATA2LSB
+  Type:      ET_REL
+  Machine:   EM_X86_64
+  SHStrNdx:  0xFF
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 589199c009f..4e1cb7d544e 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -183,6 +183,8 @@ public:
 
   void printELFLinkerOptions() override;
 
+  const object::ELFObjectFile<ELFT> *getElfObject() const { return ObjF; };
+
 private:
   std::unique_ptr<DumpStyle<ELFT>> ELFDumperStyle;
 
@@ -3009,15 +3011,19 @@ static std::string getSectionTypeString(unsigned Arch, unsigned Type) {
 
 template <class ELFT>
 static StringRef getSectionName(const typename ELFT::Shdr &Sec,
-                                const ELFFile<ELFT> &Obj,
+                                const ELFObjectFile<ELFT> &ElfObj,
                                 ArrayRef<typename ELFT::Shdr> Sections) {
+  const ELFFile<ELFT> &Obj = *ElfObj.getELFFile();
   uint32_t Index = Obj.getHeader()->e_shstrndx;
   if (Index == ELF::SHN_XINDEX)
     Index = Sections[0].sh_link;
   if (!Index) // no section string table.
     return "";
+  // TODO: Test a case when the sh_link of the section with index 0 is broken.
   if (Index >= Sections.size())
-    reportError("invalid section index");
+    reportError(ElfObj.getFileName(),
+                createError("section header string table index " +
+                            Twine(Index) + " does not exist"));
   StringRef Data = toStringRef(unwrapOrError(
       Obj.template getSectionContentsAsArray<uint8_t>(&Sections[Index])));
   return unwrapOrError(Obj.getSectionName(&Sec, Data));
@@ -3040,10 +3046,11 @@ void GNUStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
     printField(F);
   OS << "\n";
 
+  const ELFObjectFile<ELFT> *ElfObj = this->dumper()->getElfObject();
   size_t SectionIndex = 0;
   for (const Elf_Shdr &Sec : Sections) {
     Fields[0].Str = to_string(SectionIndex);
-    Fields[1].Str = getSectionName(Sec, *Obj, Sections);
+    Fields[1].Str = getSectionName(Sec, *ElfObj, Sections);
     Fields[2].Str =
         getSectionTypeString(Obj->getHeader()->e_machine, Sec.sh_type);
     Fields[3].Str =
@@ -4590,8 +4597,9 @@ void LLVMStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
 
   int SectionIndex = -1;
   ArrayRef<Elf_Shdr> Sections = unwrapOrError(Obj->sections());
+  const ELFObjectFile<ELFT> *ElfObj = this->dumper()->getElfObject();
   for (const Elf_Shdr &Sec : Sections) {
-    StringRef Name = getSectionName(Sec, *Obj, Sections);
+    StringRef Name = getSectionName(Sec, *ElfObj, Sections);
     DictScope SectionD(W, "Section");
     W.printNumber("Index", ++SectionIndex);
     W.printNumber("Name", Name, Sec.sh_name);
diff --git a/llvm/tools/llvm-readobj/llvm-readobj.cpp b/llvm/tools/llvm-readobj/llvm-readobj.cpp
index b6d0493af70..1bd5bb74bf2 100644
--- a/llvm/tools/llvm-readobj/llvm-readobj.cpp
+++ b/llvm/tools/llvm-readobj/llvm-readobj.cpp
@@ -371,11 +371,18 @@ namespace opts {
 namespace llvm {
 
 LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg) {
+  fouts().flush();
   errs() << "\n";
   WithColor::error(errs()) << Msg << "\n";
   exit(1);
 }
 
+void reportError(StringRef Input, Error Err) {
+  if (Input == "-")
+    Input = "<stdin>";
+  error(createFileError(Input, std::move(Err)));
+}
+
 void reportWarning(Twine Msg) {
   fouts().flush();
   errs() << "\n";
@@ -403,12 +410,6 @@ void error(std::error_code EC) {
 
 } // namespace llvm
 
-static void reportError(StringRef Input, Error Err) {
-  if (Input == "-")
-    Input = "<stdin>";
-  error(createFileError(Input, std::move(Err)));
-}
-
 static void reportError(StringRef Input, std::error_code EC) {
   reportError(Input, errorCodeToError(EC));
 }
diff --git a/llvm/tools/llvm-readobj/llvm-readobj.h b/llvm/tools/llvm-readobj/llvm-readobj.h
index ac8ced6121f..0e02da4cb84 100644
--- a/llvm/tools/llvm-readobj/llvm-readobj.h
+++ b/llvm/tools/llvm-readobj/llvm-readobj.h
@@ -22,6 +22,7 @@ namespace llvm {
 
   // Various helper functions.
   LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg);
+  void reportError(StringRef Input, Error Err); 
   void reportWarning(Twine Msg);
   void warn(llvm::Error Err);
   void error(std::error_code EC);