[llvm] e7abed3 - [llvm-readobj] - Refactor the MipsGOTParser<ELFT> to stop using report_fatal_error().

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 7 06:44:21 PDT 2020


Author: Georgii Rymar
Date: 2020-07-07T16:43:38+03:00
New Revision: e7abed3d48ec40350006bc76ad5c6c1f64b1bfad

URL: https://github.com/llvm/llvm-project/commit/e7abed3d48ec40350006bc76ad5c6c1f64b1bfad
DIFF: https://github.com/llvm/llvm-project/commit/e7abed3d48ec40350006bc76ad5c6c1f64b1bfad.diff

LOG: [llvm-readobj] - Refactor the MipsGOTParser<ELFT> to stop using report_fatal_error().

`MipsGOTParser` is a helper class that is used to dump MIPS GOT and PLT.
There is a problem with it: it might call report_fatal_error() on invalid input.
When this happens, the tool reports a crash:

```
# command stderr:
LLVM ERROR: Cannot find PLTGOT dynamic table tag.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backt
race.
Stack dump:
...
```

Such error were not tested. In this patch I've refactored `MipsGOTParser`:

I've splitted handling of GOT and PLT to separate methods. This allows to propagate
any possible errors to caller and should allow to dump the PLT when something is wrong
with the GOT and vise versa in the future.

I've added tests for each `report_fatal_error()`
and now calling the `reportError` instead. In the future we might want to switch to
reporting warnings, but it requres the additional testing and should
be performed independently.

I've kept `unwrapOrError` calls untouched for now as I'd like to focus on eliminating
`report_fatal_error` calls in this patch only.

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

Added: 
    

Modified: 
    llvm/test/tools/llvm-readobj/ELF/mips-got.test
    llvm/test/tools/llvm-readobj/ELF/mips-plt.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-readobj/ELF/mips-got.test b/llvm/test/tools/llvm-readobj/ELF/mips-got.test
index c13d57233326..54b321bcaae4 100644
--- a/llvm/test/tools/llvm-readobj/ELF/mips-got.test
+++ b/llvm/test/tools/llvm-readobj/ELF/mips-got.test
@@ -484,3 +484,38 @@
 # GNU-GOT-STATIC-NEXT:   00410118 -32744(gp) 00400000
 # GNU-GOT-STATIC-NEXT:   0041011c -32740(gp) 00400100
 # GNU-GOT-STATIC-NEXT:   00410120 -32736(gp) 00400104
+
+## Check we report errors when dynamic tags, needed for dumping GOT, are missing.
+
+# RUN: yaml2obj --docnum=1 -DTAG1=DT_MIPS_LOCAL_GOTNO -DTAG2=DT_MIPS_GOTSYM %s -o %t.err1.o
+# RUN: not llvm-readobj -A %t.err1.o 2>&1 | FileCheck %s -DFILE=%t.err1.o -check-prefix ERR1
+
+# ERR1: error: '[[FILE]]': cannot find PLTGOT dynamic tag
+
+# RUN: yaml2obj --docnum=1 -DTAG1=DT_PLTGOT -DTAG2=DT_MIPS_GOTSYM %s -o %t.err2.o
+# RUN: not llvm-readobj -A %t.err2.o 2>&1 | FileCheck %s -DFILE=%t.err2.o -check-prefix ERR2
+
+# ERR2: error: '[[FILE]]': cannot find MIPS_LOCAL_GOTNO dynamic tag
+
+# RUN: yaml2obj --docnum=1 -DTAG1=DT_PLTGOT -DTAG2=DT_MIPS_LOCAL_GOTNO %s -o %t.err3.o
+# RUN: not llvm-readobj -A %t.err3.o 2>&1 | FileCheck %s -DFILE=%t.err3.o -check-prefix ERR3
+
+# ERR3: error: '[[FILE]]': cannot find MIPS_GOTSYM dynamic tag
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_MIPS
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   [[TAG1]]
+        Value: 0
+      - Tag:   [[TAG2]]
+        Value: 0
+      - Tag:   DT_NULL
+        Value: 0
+DynamicSymbols: []

diff  --git a/llvm/test/tools/llvm-readobj/ELF/mips-plt.test b/llvm/test/tools/llvm-readobj/ELF/mips-plt.test
index b79237ce6c36..fa62aa98251a 100644
--- a/llvm/test/tools/llvm-readobj/ELF/mips-plt.test
+++ b/llvm/test/tools/llvm-readobj/ELF/mips-plt.test
@@ -62,3 +62,59 @@
 # GNU-NEXT:    Address  Initial Sym.Val. Type    Ndx Name
 # GNU-NEXT:   0041081c 004007c0 00000000 FUNC    UND puts
 # GNU-NEXT:   00410820 004007c0 00000000 FUNC    UND __libc_start_main
+
+## Check we report errors when dynamic tags, needed for dumping PLT, are missing.
+
+# RUN: yaml2obj --docnum=1 -DTAG=DT_MIPS_PLTGOT %s -o %t.err1.o
+# RUN: not llvm-readobj -A %t.err1.o 2>&1 | FileCheck %s -DFILE=%t.err1.o --check-prefix=ERR1
+
+# ERR1: error: '[[FILE]]': cannot find JMPREL dynamic tag
+
+# RUN: yaml2obj --docnum=1 -DTAG=DT_JMPREL %s -o %t.err2.o
+# RUN: not llvm-readobj -A %t.err2.o 2>&1 | FileCheck %s -DFILE=%t.err2.o --check-prefix=ERR2
+
+# ERR2: error: '[[FILE]]': cannot find MIPS_PLTGOT dynamic tag
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_MIPS
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   [[TAG]]
+        Value: 0
+      - Tag:   DT_NULL
+        Value: 0
+
+## Check we report errors when we are unable to find PLTGOT/JMPREL sections.
+# RUN: yaml2obj --docnum=2 %s -DVAL1=0xffff -o %t.err3.o
+# RUN: not llvm-readobj -A %t.err3.o 2>&1 | FileCheck %s -DFILE=%t.err3.o -check-prefix ERR3
+
+# ERR3: error: '[[FILE]]': there is no non-empty PLTGOT section at 0xffff
+
+# RUN: yaml2obj --docnum=2 %s -DVAL2=0xffff -o %t.err4.o
+# RUN: not llvm-readobj -A %t.err4.o 2>&1 | FileCheck %s -DFILE=%t.err4.o -check-prefix ERR4
+
+# ERR4: error: '[[FILE]]': there is no non-empty RELPLT section at 0xffff
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_MIPS
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_MIPS_PLTGOT
+        Value: [[VAL1=0]]
+      - Tag:   DT_JMPREL
+        Value: [[VAL2=0]]
+      - Tag:   DT_NULL
+        Value: 0
+DynamicSymbols: []

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 7b19cfd42e2d..cd3c79d208e4 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2865,9 +2865,14 @@ template <class ELFT> void ELFDumper<ELFT>::printArchSpecificInfo() {
 
     MipsGOTParser<ELFT> Parser(Obj, ObjF->getFileName(), dynamic_table(),
                                dynamic_symbols());
-    if (Parser.hasGot())
+    if (Error E = Parser.findGOT(dynamic_table(), dynamic_symbols()))
+      reportError(std::move(E), ObjF->getFileName());
+    else if (!Parser.isGotEmpty())
       ELFDumperStyle->printMipsGOT(Parser);
-    if (Parser.hasPlt())
+
+    if (Error E = Parser.findPLT(dynamic_table()))
+      reportError(std::move(E), ObjF->getFileName());
+    else if (!Parser.isPltEmpty())
       ELFDumperStyle->printMipsPLT(Parser);
     break;
   }
@@ -2929,9 +2934,11 @@ template <class ELFT> class MipsGOTParser {
 
   MipsGOTParser(const ELFO *Obj, StringRef FileName, Elf_Dyn_Range DynTable,
                 Elf_Sym_Range DynSyms);
+  Error findGOT(Elf_Dyn_Range DynTable, Elf_Sym_Range DynSyms);
+  Error findPLT(Elf_Dyn_Range DynTable);
 
-  bool hasGot() const { return !GotEntries.empty(); }
-  bool hasPlt() const { return !PltEntries.empty(); }
+  bool isGotEmpty() const { return GotEntries.empty(); }
+  bool isPltEmpty() const { return PltEntries.empty(); }
 
   uint64_t getGp() const;
 
@@ -2979,7 +2986,11 @@ MipsGOTParser<ELFT>::MipsGOTParser(const ELFO *Obj, StringRef FileName,
                                    Elf_Sym_Range DynSyms)
     : IsStatic(DynTable.empty()), Obj(Obj), GotSec(nullptr), LocalNum(0),
       GlobalNum(0), PltSec(nullptr), PltRelSec(nullptr), PltSymTable(nullptr),
-      FileName(FileName) {
+      FileName(FileName) {}
+
+template <class ELFT>
+Error MipsGOTParser<ELFT>::findGOT(Elf_Dyn_Range DynTable,
+                                   Elf_Sym_Range DynSyms) {
   // See "Global Offset Table" in Chapter 5 in the following document
   // for detailed GOT description.
   // ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf
@@ -2988,22 +2999,20 @@ MipsGOTParser<ELFT>::MipsGOTParser(const ELFO *Obj, StringRef FileName,
   if (IsStatic) {
     GotSec = findSectionByName(*Obj, FileName, ".got");
     if (!GotSec)
-      return;
+      return Error::success();
 
     ArrayRef<uint8_t> Content =
         unwrapOrError(FileName, Obj->getSectionContents(GotSec));
     GotEntries = Entries(reinterpret_cast<const Entry *>(Content.data()),
                          Content.size() / sizeof(Entry));
     LocalNum = GotEntries.size();
-    return;
+    return Error::success();
   }
 
-  // Lookup dynamic table tags which define GOT/PLT layouts.
+  // Lookup dynamic table tags which define the GOT layout.
   Optional<uint64_t> DtPltGot;
   Optional<uint64_t> DtLocalGotNum;
   Optional<uint64_t> DtGotSym;
-  Optional<uint64_t> DtMipsPltGot;
-  Optional<uint64_t> DtJmpRel;
   for (const auto &Entry : DynTable) {
     switch (Entry.getTag()) {
     case ELF::DT_PLTGOT:
@@ -3015,6 +3024,47 @@ MipsGOTParser<ELFT>::MipsGOTParser(const ELFO *Obj, StringRef FileName,
     case ELF::DT_MIPS_GOTSYM:
       DtGotSym = Entry.getVal();
       break;
+    }
+  }
+
+  if (!DtPltGot && !DtLocalGotNum && !DtGotSym)
+    return Error::success();
+
+  if (!DtPltGot)
+    return createError("cannot find PLTGOT dynamic tag");
+  if (!DtLocalGotNum)
+    return createError("cannot find MIPS_LOCAL_GOTNO dynamic tag");
+  if (!DtGotSym)
+    return createError("cannot find MIPS_GOTSYM dynamic tag");
+
+  size_t DynSymTotal = DynSyms.size();
+  if (*DtGotSym > DynSymTotal)
+    return createError("MIPS_GOTSYM exceeds a number of dynamic symbols");
+
+  GotSec = findNotEmptySectionByAddress(Obj, FileName, *DtPltGot);
+  if (!GotSec)
+    return createError("there is no non-empty GOT section at 0x" +
+                       Twine::utohexstr(*DtPltGot));
+
+  LocalNum = *DtLocalGotNum;
+  GlobalNum = DynSymTotal - *DtGotSym;
+
+  ArrayRef<uint8_t> Content =
+      unwrapOrError(FileName, Obj->getSectionContents(GotSec));
+  GotEntries = Entries(reinterpret_cast<const Entry *>(Content.data()),
+                       Content.size() / sizeof(Entry));
+  GotDynSyms = DynSyms.drop_front(*DtGotSym);
+
+  return Error::success();
+}
+
+template <class ELFT>
+Error MipsGOTParser<ELFT>::findPLT(Elf_Dyn_Range DynTable) {
+  // Lookup dynamic table tags which define the PLT layout.
+  Optional<uint64_t> DtMipsPltGot;
+  Optional<uint64_t> DtJmpRel;
+  for (const auto &Entry : DynTable) {
+    switch (Entry.getTag()) {
     case ELF::DT_MIPS_PLTGOT:
       DtMipsPltGot = Entry.getVal();
       break;
@@ -3024,63 +3074,35 @@ MipsGOTParser<ELFT>::MipsGOTParser(const ELFO *Obj, StringRef FileName,
     }
   }
 
-  // Find dynamic GOT section.
-  if (DtPltGot || DtLocalGotNum || DtGotSym) {
-    if (!DtPltGot)
-      report_fatal_error("Cannot find PLTGOT dynamic table tag.");
-    if (!DtLocalGotNum)
-      report_fatal_error("Cannot find MIPS_LOCAL_GOTNO dynamic table tag.");
-    if (!DtGotSym)
-      report_fatal_error("Cannot find MIPS_GOTSYM dynamic table tag.");
-
-    size_t DynSymTotal = DynSyms.size();
-    if (*DtGotSym > DynSymTotal)
-      reportError(
-          createError("MIPS_GOTSYM exceeds a number of dynamic symbols"),
-          FileName);
-
-    GotSec = findNotEmptySectionByAddress(Obj, FileName, *DtPltGot);
-    if (!GotSec)
-      reportError(createError("There is no not empty GOT section at 0x" +
-                              Twine::utohexstr(*DtPltGot)),
-                  FileName);
-
-    LocalNum = *DtLocalGotNum;
-    GlobalNum = DynSymTotal - *DtGotSym;
-
-    ArrayRef<uint8_t> Content =
-        unwrapOrError(FileName, Obj->getSectionContents(GotSec));
-    GotEntries = Entries(reinterpret_cast<const Entry *>(Content.data()),
-                         Content.size() / sizeof(Entry));
-    GotDynSyms = DynSyms.drop_front(*DtGotSym);
-  }
+  if (!DtMipsPltGot && !DtJmpRel)
+    return Error::success();
 
   // Find PLT section.
-  if (DtMipsPltGot || DtJmpRel) {
-    if (!DtMipsPltGot)
-      report_fatal_error("Cannot find MIPS_PLTGOT dynamic table tag.");
-    if (!DtJmpRel)
-      report_fatal_error("Cannot find JMPREL dynamic table tag.");
-
-    PltSec = findNotEmptySectionByAddress(Obj, FileName, * DtMipsPltGot);
-    if (!PltSec)
-      report_fatal_error("There is no not empty PLTGOT section at 0x " +
-                         Twine::utohexstr(*DtMipsPltGot));
-
-    PltRelSec = findNotEmptySectionByAddress(Obj, FileName, * DtJmpRel);
-    if (!PltRelSec)
-      report_fatal_error("There is no not empty RELPLT section at 0x" +
-                         Twine::utohexstr(*DtJmpRel));
-
-    ArrayRef<uint8_t> PltContent =
-        unwrapOrError(FileName, Obj->getSectionContents(PltSec));
-    PltEntries = Entries(reinterpret_cast<const Entry *>(PltContent.data()),
-                         PltContent.size() / sizeof(Entry));
-
-    PltSymTable = unwrapOrError(FileName, Obj->getSection(PltRelSec->sh_link));
-    PltStrTable =
-        unwrapOrError(FileName, Obj->getStringTableForSymtab(*PltSymTable));
-  }
+  if (!DtMipsPltGot)
+    return createError("cannot find MIPS_PLTGOT dynamic tag");
+  if (!DtJmpRel)
+    return createError("cannot find JMPREL dynamic tag");
+
+  PltSec = findNotEmptySectionByAddress(Obj, FileName, *DtMipsPltGot);
+  if (!PltSec)
+    return createError("there is no non-empty PLTGOT section at 0x" +
+                       Twine::utohexstr(*DtMipsPltGot));
+
+  PltRelSec = findNotEmptySectionByAddress(Obj, FileName, *DtJmpRel);
+  if (!PltRelSec)
+    return createError("there is no non-empty RELPLT section at 0x" +
+                       Twine::utohexstr(*DtJmpRel));
+
+  ArrayRef<uint8_t> PltContent =
+      unwrapOrError(FileName, Obj->getSectionContents(PltSec));
+  PltEntries = Entries(reinterpret_cast<const Entry *>(PltContent.data()),
+                       PltContent.size() / sizeof(Entry));
+
+  PltSymTable = unwrapOrError(FileName, Obj->getSection(PltRelSec->sh_link));
+  PltStrTable =
+      unwrapOrError(FileName, Obj->getStringTableForSymtab(*PltSymTable));
+
+  return Error::success();
 }
 
 template <class ELFT> uint64_t MipsGOTParser<ELFT>::getGp() const {


        


More information about the llvm-commits mailing list