[llvm] 710d9d6 - [DebugInfo] - DWARFDebugFrame: do not call abort() on errors.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Fri May 15 03:05:54 PDT 2020


Author: Georgii Rymar
Date: 2020-05-15T13:05:35+03:00
New Revision: 710d9d66f8e2f8ac148ef9a3d226c6a8e22f15c5

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

LOG: [DebugInfo] - DWARFDebugFrame: do not call abort() on errors.

Imagine we have a broken .eh_frame.
Below is a possible sample output of llvm-readelf:

```
...
    entry 2 {
      initial_location: 0x10f5
      address: 0x2080
    }
  }
}
.eh_frame section at offset 0x2028 address 0x2028:
LLVM ERROR: Parsing entry instructions at 0 failed
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.  Program arguments: /home/umb/LLVM/LLVM/llvm-project/build/bin/llvm-readelf -a 1
 #0 0x000055f4a2ff5a1a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/home/umb/LLVM/LLVM/llvm-project/build/bin/llvm-readelf+0x2b9a1a)
...
#15 0x00007fdae5dc209b __libc_start_main /build/glibc-B9XfQf/glibc-2.28/csu/../csu/libc-start.c:342:3
#16 0x000055f4a2db746a _start (/home/umb/LLVM/LLVM/llvm-project/build/bin/llvm-readelf+0x7b46a)
Aborted
```

I.e. it calls abort(), suggests to submit a bug report and exits with the code 134.
This patch changes the logic to propagate errors to callers.
This fixes the behavior for llvm-dwarfdump, llvm-readobj and other possible tools.

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

Added: 
    

Modified: 
    llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
    llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
    llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
    llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
    llvm/test/DebugInfo/X86/eh-frame-cie-id.s
    llvm/test/tools/llvm-readobj/ELF/unwind.test
    llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
index e58a46bc9d0a..3e387343d54f 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
@@ -277,10 +277,10 @@ class DWARFContext : public DIContext {
   const DWARFDebugAranges *getDebugAranges();
 
   /// Get a pointer to the parsed frame information object.
-  const DWARFDebugFrame *getDebugFrame();
+  Expected<const DWARFDebugFrame *> getDebugFrame();
 
   /// Get a pointer to the parsed eh frame information object.
-  const DWARFDebugFrame *getEHFrame();
+  Expected<const DWARFDebugFrame *> getEHFrame();
 
   /// Get a pointer to the parsed DebugMacinfo information object.
   const DWARFDebugMacro *getDebugMacinfo();

diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
index 1eb22b2eae80..233b55cc55c1 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
@@ -290,7 +290,7 @@ class DWARFDebugFrame {
 
   /// Parse the section from raw data. \p Data is assumed to contain the whole
   /// frame section contents to be parsed.
-  void parse(DWARFDataExtractor Data);
+  Error parse(DWARFDataExtractor Data);
 
   /// Return whether the section has any entries.
   bool empty() const { return Entries.empty(); }

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index b3de75b501bb..5e7e287e1c5f 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -436,13 +436,23 @@ void DWARFContext::dump(
     }
   }
 
-  if (const auto *Off = shouldDump(Explicit, ".debug_frame", DIDT_ID_DebugFrame,
-                                   DObj->getFrameSection().Data))
-    getDebugFrame()->dump(OS, getRegisterInfo(), *Off);
+  if (const Optional<uint64_t> *Off =
+          shouldDump(Explicit, ".debug_frame", DIDT_ID_DebugFrame,
+                     DObj->getFrameSection().Data)) {
+    if (Expected<const DWARFDebugFrame *> DF = getDebugFrame())
+      (*DF)->dump(OS, getRegisterInfo(), *Off);
+    else
+      RecoverableErrorHandler(DF.takeError());
+  }
 
-  if (const auto *Off = shouldDump(Explicit, ".eh_frame", DIDT_ID_DebugFrame,
-                                   DObj->getEHFrameSection().Data))
-    getEHFrame()->dump(OS, getRegisterInfo(), *Off);
+  if (const Optional<uint64_t> *Off =
+          shouldDump(Explicit, ".eh_frame", DIDT_ID_DebugFrame,
+                     DObj->getEHFrameSection().Data)) {
+    if (Expected<const DWARFDebugFrame *> DF = getEHFrame())
+      (*DF)->dump(OS, getRegisterInfo(), *Off);
+    else
+      RecoverableErrorHandler(DF.takeError());
+  }
 
   if (shouldDump(Explicit, ".debug_macro", DIDT_ID_DebugMacro,
                  DObj->getMacroSection().Data)) {
@@ -791,7 +801,7 @@ const DWARFDebugAranges *DWARFContext::getDebugAranges() {
   return Aranges.get();
 }
 
-const DWARFDebugFrame *DWARFContext::getDebugFrame() {
+Expected<const DWARFDebugFrame *> DWARFContext::getDebugFrame() {
   if (DebugFrame)
     return DebugFrame.get();
 
@@ -806,19 +816,25 @@ const DWARFDebugFrame *DWARFContext::getDebugFrame() {
   // http://lists.dwarfstd.org/htdig.cgi/dwarf-discuss-dwarfstd.org/2011-December/001173.html
   DWARFDataExtractor debugFrameData(*DObj, DObj->getFrameSection(),
                                     isLittleEndian(), DObj->getAddressSize());
-  DebugFrame.reset(new DWARFDebugFrame(getArch(), false /* IsEH */));
-  DebugFrame->parse(debugFrameData);
+  auto DF = std::make_unique<DWARFDebugFrame>(getArch(), /*IsEH=*/false);
+  if (Error E = DF->parse(debugFrameData))
+    return std::move(E);
+
+  DebugFrame.swap(DF);
   return DebugFrame.get();
 }
 
-const DWARFDebugFrame *DWARFContext::getEHFrame() {
+Expected<const DWARFDebugFrame *> DWARFContext::getEHFrame() {
   if (EHFrame)
     return EHFrame.get();
 
   DWARFDataExtractor debugFrameData(*DObj, DObj->getEHFrameSection(),
                                     isLittleEndian(), DObj->getAddressSize());
-  DebugFrame.reset(new DWARFDebugFrame(getArch(), true /* IsEH */));
-  DebugFrame->parse(debugFrameData);
+
+  auto DF = std::make_unique<DWARFDebugFrame>(getArch(), /*IsEH=*/true);
+  if (Error E = DF->parse(debugFrameData))
+    return std::move(E);
+  DebugFrame.swap(DF);
   return DebugFrame.get();
 }
 

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index 5b38b29227ab..269a45e57a8d 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -365,20 +365,7 @@ static void LLVM_ATTRIBUTE_UNUSED dumpDataAux(DataExtractor Data,
   errs() << "\n";
 }
 
-// This is a workaround for old compilers which do not allow
-// noreturn attribute usage in lambdas. Once the support for those
-// compilers are phased out, we can remove this and return back to
-// a ReportError lambda: [StartOffset](const char *ErrorMsg).
-static void LLVM_ATTRIBUTE_NORETURN ReportError(uint64_t StartOffset,
-                                                const char *ErrorMsg) {
-  std::string Str;
-  raw_string_ostream OS(Str);
-  OS << format(ErrorMsg, StartOffset);
-  OS.flush();
-  report_fatal_error(Str);
-}
-
-void DWARFDebugFrame::parse(DWARFDataExtractor Data) {
+Error DWARFDebugFrame::parse(DWARFDataExtractor Data) {
   uint64_t Offset = 0;
   DenseMap<uint64_t, CIE *> CIEs;
 
@@ -427,51 +414,55 @@ void DWARFDebugFrame::parse(DWARFDataExtractor Data) {
         // Walk the augmentation string to get all the augmentation data.
         for (unsigned i = 0, e = AugmentationString.size(); i != e; ++i) {
           switch (AugmentationString[i]) {
-            default:
-              ReportError(
-                  StartOffset,
-                  "Unknown augmentation character in entry at %" PRIx64);
-            case 'L':
-              LSDAPointerEncoding = Data.getU8(&Offset);
-              break;
-            case 'P': {
-              if (Personality)
-                ReportError(StartOffset,
-                            "Duplicate personality in entry at %" PRIx64);
-              PersonalityEncoding = Data.getU8(&Offset);
-              Personality = Data.getEncodedPointer(
-                  &Offset, *PersonalityEncoding,
-                  EHFrameAddress ? EHFrameAddress + Offset : 0);
-              break;
-            }
-            case 'R':
-              FDEPointerEncoding = Data.getU8(&Offset);
-              break;
-            case 'S':
-              // Current frame is a signal trampoline.
-              break;
-            case 'z':
-              if (i)
-                ReportError(StartOffset,
-                            "'z' must be the first character at %" PRIx64);
-              // Parse the augmentation length first.  We only parse it if
-              // the string contains a 'z'.
-              AugmentationLength = Data.getULEB128(&Offset);
-              StartAugmentationOffset = Offset;
-              EndAugmentationOffset = Offset + *AugmentationLength;
-              break;
-            case 'B':
-              // B-Key is used for signing functions associated with this
-              // augmentation string
-              break;
+          default:
+            return createStringError(
+                errc::invalid_argument,
+                "unknown augmentation character in entry at 0x%" PRIx64,
+                StartOffset);
+          case 'L':
+            LSDAPointerEncoding = Data.getU8(&Offset);
+            break;
+          case 'P': {
+            if (Personality)
+              return createStringError(
+                  errc::invalid_argument,
+                  "duplicate personality in entry at 0x%" PRIx64, StartOffset);
+            PersonalityEncoding = Data.getU8(&Offset);
+            Personality = Data.getEncodedPointer(
+                &Offset, *PersonalityEncoding,
+                EHFrameAddress ? EHFrameAddress + Offset : 0);
+            break;
+          }
+          case 'R':
+            FDEPointerEncoding = Data.getU8(&Offset);
+            break;
+          case 'S':
+            // Current frame is a signal trampoline.
+            break;
+          case 'z':
+            if (i)
+              return createStringError(
+                  errc::invalid_argument,
+                  "'z' must be the first character at 0x%" PRIx64, StartOffset);
+            // Parse the augmentation length first.  We only parse it if
+            // the string contains a 'z'.
+            AugmentationLength = Data.getULEB128(&Offset);
+            StartAugmentationOffset = Offset;
+            EndAugmentationOffset = Offset + *AugmentationLength;
+            break;
+          case 'B':
+            // B-Key is used for signing functions associated with this
+            // augmentation string
+            break;
           }
         }
 
         if (AugmentationLength.hasValue()) {
           if (Offset != EndAugmentationOffset)
-            ReportError(StartOffset,
-                        "Parsing augmentation data at %" PRIx64 " failed");
-
+            return createStringError(errc::invalid_argument,
+                                     "parsing augmentation data at 0x%" PRIx64
+                                     " failed",
+                                     StartOffset);
           AugmentationData = Data.getData().slice(StartAugmentationOffset,
                                                   EndAugmentationOffset);
         }
@@ -496,9 +487,10 @@ void DWARFDebugFrame::parse(DWARFDataExtractor Data) {
       if (IsEH) {
         // The address size is encoded in the CIE we reference.
         if (!Cie)
-          ReportError(StartOffset, "Parsing FDE data at %" PRIx64
-                                   " failed due to missing CIE");
-
+          return createStringError(errc::invalid_argument,
+                                   "parsing FDE data at 0x%" PRIx64
+                                   " failed due to missing CIE",
+                                   StartOffset);
         if (auto Val = Data.getEncodedPointer(
                 &Offset, Cie->getFDEPointerEncoding(),
                 EHFrameAddress ? EHFrameAddress + Offset : 0)) {
@@ -524,8 +516,10 @@ void DWARFDebugFrame::parse(DWARFDataExtractor Data) {
           }
 
           if (Offset != EndAugmentationOffset)
-            ReportError(StartOffset,
-                        "Parsing augmentation data at %" PRIx64 " failed");
+            return createStringError(errc::invalid_argument,
+                                     "parsing augmentation data at 0x%" PRIx64
+                                     " failed",
+                                     StartOffset);
         }
       } else {
         InitialLocation = Data.getRelocatedAddress(&Offset);
@@ -538,14 +532,16 @@ void DWARFDebugFrame::parse(DWARFDataExtractor Data) {
     }
 
     if (Error E =
-            Entries.back()->cfis().parse(Data, &Offset, EndStructureOffset)) {
-      report_fatal_error(toString(std::move(E)));
-    }
+            Entries.back()->cfis().parse(Data, &Offset, EndStructureOffset))
+      return E;
 
     if (Offset != EndStructureOffset)
-      ReportError(StartOffset,
-                  "Parsing entry instructions at %" PRIx64 " failed");
+      return createStringError(
+          errc::invalid_argument,
+          "parsing entry instructions at 0x%" PRIx64 " failed", StartOffset);
   }
+
+  return Error::success();
 }
 
 FrameEntry *DWARFDebugFrame::getEntryAtOffset(uint64_t Offset) const {

diff  --git a/llvm/test/DebugInfo/X86/eh-frame-cie-id.s b/llvm/test/DebugInfo/X86/eh-frame-cie-id.s
index 51098e67c08f..bc448183a530 100644
--- a/llvm/test/DebugInfo/X86/eh-frame-cie-id.s
+++ b/llvm/test/DebugInfo/X86/eh-frame-cie-id.s
@@ -1,8 +1,7 @@
-# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
-# RUN:   not --crash llvm-dwarfdump -debug-frame - 2>&1 | \
-# RUN:   FileCheck %s
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t
+# RUN: not llvm-dwarfdump -debug-frame %t 2>&1 | FileCheck %s
 
-# CHECK: Parsing FDE data at 0 failed due to missing CIE
+# CHECK: parsing FDE data at 0x0 failed due to missing CIE
 
         .section .eh_frame,"a", at unwind
 ## This FDE was formerly wrongly interpreted as a CIE because its CIE pointer

diff  --git a/llvm/test/tools/llvm-readobj/ELF/unwind.test b/llvm/test/tools/llvm-readobj/ELF/unwind.test
index 17cd2f572e53..dbdc9617aae3 100644
--- a/llvm/test/tools/llvm-readobj/ELF/unwind.test
+++ b/llvm/test/tools/llvm-readobj/ELF/unwind.test
@@ -1,5 +1,5 @@
-# RUN: yaml2obj %s -o %t.exe
-# RUN: llvm-readobj --unwind %t.exe | FileCheck %s
+# RUN: yaml2obj --docnum=1 %s -o %t1.exe
+# RUN: llvm-readobj --unwind %t1.exe | FileCheck %s
 
 # CHECK:      EHFrameHeader {
 # CHECK-NEXT:  Address: 0x4013c0
@@ -215,3 +215,31 @@ ProgramHeaders:
     Sections:
       - Section: .eh_frame_hdr
 ...
+
+## Check we report an error when the tool is unable to parse .eh_frame section.
+# RUN: yaml2obj --docnum=2 %s -o %t2.exe
+# RUN: not llvm-readobj --unwind %t2.exe 2>&1 | FileCheck %s -DFILE=%t2.exe --check-prefix=NO-CIE-ERR
+
+# NO-CIE-ERR:      .eh_frame section at offset 0x40 address 0x0:
+# NO-CIE-ERR-NEXT: error: '[[FILE]]': parsing FDE data at 0x0 failed due to missing CIE
+# NO-CIE-ERR-NOT:  {{.}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .eh_frame
+    Type: SHT_X86_64_UNWIND
+## The content is generated from the following code. It has no CIE.
+## See the DebugInfoX86/eh-frame-cie-id.s test case for more history.
+## .section .eh_frame,"a", at unwind
+## .long .Lend - .LCIEptr  # Length
+## .LCIEptr:
+##   .long 0xffffffff # CIE pointer
+##   .quad 0x1111abcd # Initial location
+##   .quad 0x00010000 # Address range
+## .Lend:
+    Content: 14000000FFFFFFFFCDAB1111000000000000010000000000

diff  --git a/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h b/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
index 3d0b75f93771..00a0b691b76b 100644
--- a/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
+++ b/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
@@ -191,7 +191,8 @@ void PrinterContext<ELFT>::printEHFrame(
                         ELFT::Is64Bits ? 8 : 4);
   DWARFDebugFrame EHFrame(Triple::ArchType(ObjF->getArch()), /*IsEH=*/true,
                           /*EHFrameAddress=*/Address);
-  EHFrame.parse(DE);
+  if (Error E = EHFrame.parse(DE))
+    reportError(std::move(E), ObjF->getFileName());
 
   for (const auto &Entry : EHFrame) {
     if (const auto *CIE = dyn_cast<dwarf::CIE>(&Entry)) {


        


More information about the llvm-commits mailing list