labath updated this revision to Diff 205973.
labath marked 7 inline comments as done.
labath added a comment.

- remove fancy references
- remove llvm_unreachable


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63591/new/

https://reviews.llvm.org/D63591

Files:
  include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
  lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
  lib/DebugInfo/DWARF/DWARFDie.cpp
  test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
  test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s

Index: test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s
===================================================================
--- /dev/null
+++ test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s
@@ -0,0 +1,62 @@
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t1.o
+# RUN: llvm-dwarfdump -debug-loclists %t1.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE2=0 -o %t2.o
+# RUN: llvm-dwarfdump -debug-loclists %t2.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE3=0 -o %t3.o
+# RUN: llvm-dwarfdump -debug-loclists %t3.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE4=0 -o %t4.o
+# RUN: llvm-dwarfdump -debug-loclists %t4.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t5.o
+# RUN: llvm-dwarfdump -debug-loclists %t5.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE6=0 -o %t6.o
+# RUN: llvm-dwarfdump -debug-loclists %t6.o 2>&1 | FileCheck %s --check-prefix=UNIMPL
+
+# CHECK: error: location list overflows the debug_loclists section
+
+# UNIMPL: error: LLE of kind 47 not implemented
+
+.section  .debug_loclists,"",@progbits
+  .long  .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0
+.Ldebug_loclist_table_start0:
+ .short 5                        # Version.
+ .byte 8                         # Address size.
+ .byte 0                         # Segment selector size.
+ .long 0                         # Offset entry count.
+.Lloclists_table_base0:
+.Ldebug_loc0:
+.ifdef CASE1
+  .byte  4                       # DW_LLE_offset_pair
+.endif
+.ifdef CASE2
+  .byte  4                       # DW_LLE_offset_pair
+  .uleb128 0x0                   #   starting offset
+.endif
+.ifdef CASE3
+  .byte  4                       # DW_LLE_offset_pair
+  .uleb128 0x0                   #   starting offset
+  .uleb128 0x10                  #   ending offset
+.endif
+.ifdef CASE4
+  .byte  4                       # DW_LLE_offset_pair
+  .uleb128 0x0                   #   starting offset
+  .uleb128 0x10                  #   ending offset
+  .byte  1                       # Loc expr size
+.endif
+.ifdef CASE5
+  .byte  4                       # DW_LLE_offset_pair
+  .uleb128 0x0                   #   starting offset
+  .uleb128 0x10                  #   ending offset
+  .byte  1                       # Loc expr size
+  .byte  117                     # DW_OP_breg5
+.endif
+.ifdef CASE6
+  .byte 0x47
+.endif
+
+.Ldebug_loclist_table_end0:
+
Index: test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
===================================================================
--- /dev/null
+++ test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
@@ -0,0 +1,52 @@
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t1.o
+# RUN: llvm-dwarfdump -debug-loc %t1.o 2>&1 | FileCheck %s --check-prefix=CONSUME
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE2=0 -o %t2.o
+# RUN: llvm-dwarfdump -debug-loc %t2.o 2>&1 | FileCheck %s --check-prefix=CONSUME
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE3=0 -o %t3.o
+# RUN: llvm-dwarfdump -debug-loc %t3.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE4=0 -o %t4.o
+# RUN: llvm-dwarfdump -debug-loc %t4.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t5.o
+# RUN: llvm-dwarfdump -debug-loc %t5.o 2>&1 | FileCheck %s
+
+# CONSUME: error: failed to consume entire .debug_loc section
+
+# CHECK: error: location list overflows the debug_loc section
+
+.section  .debug_loc,"",@progbits
+.ifdef CASE1
+  .byte  1                       # bogus
+.endif
+.ifdef CASE2
+  .long  0                       # starting offset
+.endif
+.ifdef CASE3
+  .long  0                       # starting offset
+  .long  1                       # ending offset
+.endif
+.ifdef CASE4
+  .long  0                       # starting offset
+  .long  1                       # ending offset
+  .word  0                       # Loc expr size
+.endif
+.ifdef CASE5
+  .long  0                       # starting offset
+  .long  1                       # ending offset
+  .word  0                       # Loc expr size
+  .long  0                       # starting offset
+.endif
+
+# A minimal compile unit is needed to deduce the address size of the location
+# lists
+.section  .debug_info,"",@progbits
+  .long  .Lcu_end0-.Lcu_begin0   # Length of Unit
+.Lcu_begin0:
+  .short  4                      # DWARF version number
+  .long  0                       # Offset Into Abbrev. Section
+  .byte  8                       # Address Size (in bytes)
+  .byte  0                       # End Of Children Mark
+.Lcu_end0:
Index: lib/DebugInfo/DWARF/DWARFDie.cpp
===================================================================
--- lib/DebugInfo/DWARF/DWARFDie.cpp
+++ lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -97,15 +97,15 @@
       DWARFDebugLoc DebugLoc;
       DWARFDataExtractor Data(Obj, *U->getLocSection(), Ctx.isLittleEndian(),
                               Obj.getAddressSize());
-      auto LL = DebugLoc.parseOneLocationList(Data, &Offset);
-      if (LL) {
+      if (auto LL = DebugLoc.parseOneLocationList(Data, &Offset)) {
         uint64_t BaseAddr = 0;
         if (Optional<object::SectionedAddress> BA = U->getBaseAddress())
           BaseAddr = BA->Address;
         LL->dump(OS, Ctx.isLittleEndian(), Obj.getAddressSize(), MRI, U,
                  BaseAddr, Indent);
       } else
-        OS << "error extracting location list.";
+        logAllUnhandledErrors(LL.takeError(), OS,
+                              "error extracting location list:");
       return;
     }
 
@@ -131,8 +131,10 @@
       if (LL)
         LL->dump(OS, BaseAddr, Ctx.isLittleEndian(), Obj.getAddressSize(), MRI,
                  U, Indent);
-      else
-        OS << "error extracting location list.";
+      else {
+        logAllUnhandledErrors(LL.takeError(), OS,
+                              "error extracting location list:");
+      }
     }
   }
 }
Index: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
===================================================================
--- lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -23,6 +23,15 @@
 
 using namespace llvm;
 
+template <typename... Ts>
+static llvm::Error createError(const char *Fmt, const Ts &... Vals) {
+  return createStringError(inconvertibleErrorCode(), Fmt, Vals...);
+}
+
+static llvm::Error createOverflowError(const char *Section) {
+  return createError("location list overflows the %s section", Section);
+}
+
 // When directly dumping the .debug_loc without a compile unit, we have to guess
 // at the DWARF version. This only affects DW_OP_call_ref, which is a rare
 // expression that LLVM doesn't produce. Guessing the wrong version means we
@@ -83,7 +92,7 @@
   }
 }
 
-Optional<DWARFDebugLoc::LocationList>
+Expected<DWARFDebugLoc::LocationList>
 DWARFDebugLoc::parseOneLocationList(DWARFDataExtractor Data, unsigned *Offset) {
   LocationList LL;
   LL.Offset = *Offset;
@@ -92,10 +101,8 @@
   // A location list entry consists of:
   while (true) {
     Entry E;
-    if (!Data.isValidOffsetForDataOfSize(*Offset, 2 * Data.getAddressSize())) {
-      WithColor::error() << "location list overflows the debug_loc section.\n";
-      return None;
-    }
+    if (!Data.isValidOffsetForDataOfSize(*Offset, 2 * Data.getAddressSize()))
+      return createOverflowError("debug_loc");
 
     // 1. A beginning address offset. ...
     E.Begin = Data.getRelocatedAddress(Offset);
@@ -109,16 +116,13 @@
     if (E.Begin == 0 && E.End == 0)
       return LL;
 
-    if (!Data.isValidOffsetForDataOfSize(*Offset, 2)) {
-      WithColor::error() << "location list overflows the debug_loc section.\n";
-      return None;
-    }
+    if (!Data.isValidOffsetForDataOfSize(*Offset, 2))
+      return createOverflowError("debug_loc");
 
     unsigned Bytes = Data.getU16(Offset);
-    if (!Data.isValidOffsetForDataOfSize(*Offset, Bytes)) {
-      WithColor::error() << "location list overflows the debug_loc section.\n";
-      return None;
-    }
+    if (!Data.isValidOffsetForDataOfSize(*Offset, Bytes))
+      return createOverflowError("debug_loc");
+
     // A single location description describing the location of the object...
     StringRef str = Data.getData().substr(*Offset, Bytes);
     *Offset += Bytes;
@@ -136,28 +140,49 @@
   while (data.isValidOffset(Offset + data.getAddressSize() - 1)) {
     if (auto LL = parseOneLocationList(data, &Offset))
       Locations.push_back(std::move(*LL));
-    else
+    else {
+      logAllUnhandledErrors(LL.takeError(), WithColor::error());
       break;
+    }
   }
   if (data.isValidOffset(Offset))
     WithColor::error() << "failed to consume entire .debug_loc section\n";
 }
 
-Optional<DWARFDebugLoclists::LocationList>
+Expected<DWARFDebugLoclists::LocationList>
 DWARFDebugLoclists::parseOneLocationList(DataExtractor Data, unsigned *Offset,
                                          unsigned Version) {
   LocationList LL;
   LL.Offset = *Offset;
 
-  // dwarf::DW_LLE_end_of_list_entry is 0 and indicates the end of the list.
-  while (auto Kind =
-             static_cast<dwarf::LocationListEntry>(Data.getU8(Offset))) {
+  while (true) {
+    if (!Data.isValidOffset(*Offset))
+      return createOverflowError("debug_loclists");
 
     Entry E;
-    E.Kind = Kind;
-    switch (Kind) {
+    E.Kind = static_cast<dwarf::LocationListEntry>(Data.getU8(Offset));
+    if (E.Kind == dwarf::DW_LLE_end_of_list)
+      return LL;
+
+    unsigned SavedOffset = *Offset;
+    switch (E.Kind) {
     case dwarf::DW_LLE_startx_length:
+    case dwarf::DW_LLE_offset_pair:
       E.Value0 = Data.getULEB128(Offset);
+      break;
+    case dwarf::DW_LLE_start_length:
+    case dwarf::DW_LLE_base_address:
+      E.Value0 = Data.getAddress(Offset);
+      break;
+    default:
+      return createError("LLE of kind %x not implemented", (int)E.Kind);
+    }
+    if (SavedOffset == *Offset)
+      return createOverflowError("debug_loclists");
+
+    SavedOffset = *Offset;
+    switch (E.Kind) {
+    case dwarf::DW_LLE_startx_length:
       // Pre-DWARF 5 has different interpretation of the length field. We have
       // to support both pre- and standartized styles for the compatibility.
       if (Version < 5)
@@ -166,25 +191,23 @@
         E.Value1 = Data.getULEB128(Offset);
       break;
     case dwarf::DW_LLE_start_length:
-      E.Value0 = Data.getAddress(Offset);
-      E.Value1 = Data.getULEB128(Offset);
-      break;
     case dwarf::DW_LLE_offset_pair:
-      E.Value0 = Data.getULEB128(Offset);
       E.Value1 = Data.getULEB128(Offset);
       break;
     case dwarf::DW_LLE_base_address:
-      E.Value0 = Data.getAddress(Offset);
       break;
-    default:
-      WithColor::error() << "dumping support for LLE of kind " << (int)Kind
-                         << " not implemented\n";
-      return None;
     }
+    if (E.Kind != dwarf::DW_LLE_base_address) {
+      if (SavedOffset == *Offset)
+        return createOverflowError("debug_loclists");
 
-    if (Kind != dwarf::DW_LLE_base_address) {
+      SavedOffset = *Offset;
       unsigned Bytes =
           Version >= 5 ? Data.getULEB128(Offset) : Data.getU16(Offset);
+      if (SavedOffset == *Offset ||
+          !Data.isValidOffsetForDataOfSize(*Offset, Bytes))
+        return createOverflowError("debug_loclists");
+
       // A single location description describing the location of the object...
       StringRef str = Data.getData().substr(*Offset, Bytes);
       *Offset += Bytes;
@@ -194,7 +217,6 @@
 
     LL.Entries.push_back(std::move(E));
   }
-  return LL;
 }
 
 void DWARFDebugLoclists::parse(DataExtractor data, unsigned Version) {
@@ -205,8 +227,10 @@
   while (data.isValidOffset(Offset)) {
     if (auto LL = parseOneLocationList(data, &Offset, Version))
       Locations.push_back(std::move(*LL));
-    else
+    else {
+      logAllUnhandledErrors(LL.takeError(), WithColor::error());
       return;
+    }
   }
 }
 
Index: include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
===================================================================
--- include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
+++ include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
@@ -68,8 +68,8 @@
   /// Return the location list at the given offset or nullptr.
   LocationList const *getLocationListAtOffset(uint64_t Offset) const;
 
-  Optional<LocationList> parseOneLocationList(DWARFDataExtractor Data,
-                                              uint32_t *Offset);
+  static Expected<LocationList> parseOneLocationList(DWARFDataExtractor Data,
+                                                     uint32_t *Offset);
 };
 
 class DWARFDebugLoclists {
@@ -106,7 +106,7 @@
   /// Return the location list at the given offset or nullptr.
   LocationList const *getLocationListAtOffset(uint64_t Offset) const;
 
-  static Optional<LocationList>
+  static Expected<LocationList>
   parseOneLocationList(DataExtractor Data, unsigned *Offset, unsigned Version);
 };
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to