[Lldb-commits] [lldb] r356190 - Return llvm::Error and llvm::Expected from DWARF parsing code.

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Thu Mar 14 12:05:55 PDT 2019


Author: zturner
Date: Thu Mar 14 12:05:55 2019
New Revision: 356190

URL: http://llvm.org/viewvc/llvm-project?rev=356190&view=rev
Log:
Return llvm::Error and llvm::Expected from DWARF parsing code.

The goal here is to improve our error handling and error recovery while
parsing DWARF, while at the same time getting us closer to being able to
merge LLDB's DWARF parser with LLVM's. To this end, I've udpated several
of the low-level parsing functions in LLDB to return llvm::Error and
llvm::Expected.

For now, this only updates LLDB parsing functions and not LLVM. In some
ways, this actually gets us *farther* from parity with the two
interfaces, because prior to this patch, at least the parsing interfaces
were the same (i.e. they all just returned bools, and now with this
patch they're diverging). But, I chose to do this for two primary
reasons.

LLDB has error logging code engrained deep within some of its parsing
functions. We don't want to lose this logging information, but obviously
LLVM has no logging mechanism at all. So if we're to merge the
interfaces, we have to find a way to still allow LLDB to properly report
parsing errors while not having the reporting code be inside of LLVM.
LLDB (and indeed, LLVM) overload the meaning of the false return value
from all of these extraction functions to mean both "We reached the null
entry at the end of a list of items, therefore everything was
successful" as well as "something bad and unrecoverable happened during
parsing". So you would have a lot code that would do something like:
while (foo.extract(...)) {
  ...
}

But when the loop stops, why did it stop? Did it stop because it
finished parsing, or because there was an error? Because of this, in
some cases we don't always know whether it is ok to proceed, or how to
proceed, but we were doing it anyway.

In this patch, I solve the second problem by introducing an
enumeration called DWARFEnumState which has two values MoreItems and
Complete. Both of these indicate success, but the latter indicates
that we reached the null entry. Then, I return this value instead of
bool, and convey parsing failure separately.

To solve the first problem (and convey parsing failure) these
functions now return either llvm::Error or llvm::Expected<DWARFEnumState>.
Having this extra bit of information allows us to properly convey all 3 of
"error, bail out", "success, call this function again", and "success,
don't call this function again".

In subsequent patches I plan to extend this pattern to the rest of the
parsing interfaces, which will ultimately get all of the log statements
and error reporting out of the low level parsing code and into the high
level parsing code (e.g. SymbolFileDWARF, DWARFASTParserClang, etc).

Eventually, these same changes will have to be backported to LLVM's
DWARF parser, but diverging in the short term is the easiest way to
converge in the long term.

Differential Revision: https://reviews.llvm.org/D59370

Modified:
    lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
    lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
    lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
    lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h
    lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
    lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp?rev=356190&r1=356189&r2=356190&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp Thu Mar 14 12:05:55 2019
@@ -11,6 +11,8 @@
 #include "lldb/Core/dwarf.h"
 #include "lldb/Utility/Stream.h"
 
+#include "llvm/Object/Error.h"
+
 #include "DWARFFormValue.h"
 
 using namespace lldb_private;
@@ -23,41 +25,48 @@ DWARFAbbreviationDeclaration::DWARFAbbre
     : m_code(InvalidCode), m_tag(tag), m_has_children(has_children),
       m_attributes() {}
 
-bool DWARFAbbreviationDeclaration::Extract(const DWARFDataExtractor &data,
-                                           lldb::offset_t *offset_ptr) {
-  return Extract(data, offset_ptr, data.GetULEB128(offset_ptr));
-}
+llvm::Expected<DWARFEnumState>
+DWARFAbbreviationDeclaration::extract(const DWARFDataExtractor &data,
+                                      lldb::offset_t *offset_ptr) {
+  m_code = data.GetULEB128(offset_ptr);
+  if (m_code == 0)
+    return DWARFEnumState::Complete;
 
-bool DWARFAbbreviationDeclaration::Extract(const DWARFDataExtractor &data,
-                                           lldb::offset_t *offset_ptr,
-                                           dw_uleb128_t code) {
-  m_code = code;
   m_attributes.clear();
-  if (m_code) {
-    m_tag = data.GetULEB128(offset_ptr);
-    m_has_children = data.GetU8(offset_ptr);
-
-    while (data.ValidOffset(*offset_ptr)) {
-      dw_attr_t attr = data.GetULEB128(offset_ptr);
-      dw_form_t form = data.GetULEB128(offset_ptr);
-      DWARFFormValue::ValueType val;
-
-      if (form == DW_FORM_implicit_const)
-        val.value.sval = data.GetULEB128(offset_ptr);
-
-      if (attr && form)
-        m_attributes.push_back(DWARFAttribute(attr, form, val));
-      else
-        break;
-    }
-
-    return m_tag != 0;
-  } else {
-    m_tag = 0;
-    m_has_children = 0;
+  m_tag = data.GetULEB128(offset_ptr);
+  if (m_tag == DW_TAG_null) {
+    // FIXME: According to the DWARF spec this may actually be malformed.
+    // Should this return an error instead?
+    return DWARFEnumState::Complete;
+  }
+
+  m_has_children = data.GetU8(offset_ptr);
+
+  while (data.ValidOffset(*offset_ptr)) {
+    dw_attr_t attr = data.GetULEB128(offset_ptr);
+    dw_form_t form = data.GetULEB128(offset_ptr);
+
+    // This is the last attribute for this abbrev decl, but there may still be
+    // more abbrev decls, so return MoreItems to indicate to the caller that
+    // they should call this function again.
+    if (!attr && !form)
+      return DWARFEnumState::MoreItems;
+
+    if (!attr || !form)
+      return llvm::make_error<llvm::object::GenericBinaryError>(
+          "malformed abbreviation declaration attribute");
+
+    DWARFFormValue::ValueType val;
+
+    if (form == DW_FORM_implicit_const)
+      val.value.sval = data.GetULEB128(offset_ptr);
+
+    m_attributes.push_back(DWARFAttribute(attr, form, val));
   }
 
-  return false;
+  return llvm::make_error<llvm::object::GenericBinaryError>(
+      "abbreviation declaration attribute list not terminated with a null "
+      "entry");
 }
 
 void DWARFAbbreviationDeclaration::Dump(Stream *s) const {

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h?rev=356190&r1=356189&r2=356190&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h Thu Mar 14 12:05:55 2019
@@ -10,7 +10,9 @@
 #define liblldb_DWARFAbbreviationDeclaration_h_
 
 #include "DWARFAttribute.h"
+#include "DWARFDefines.h"
 #include "SymbolFileDWARF.h"
+#include "llvm/Support/Error.h"
 
 class DWARFAbbreviationDeclaration {
 public:
@@ -44,10 +46,18 @@ public:
     return m_attributes[idx].get_form();
   }
   uint32_t FindAttributeIndex(dw_attr_t attr) const;
-  bool Extract(const lldb_private::DWARFDataExtractor &data,
-               lldb::offset_t *offset_ptr);
-  bool Extract(const lldb_private::DWARFDataExtractor &data,
-               lldb::offset_t *offset_ptr, dw_uleb128_t code);
+
+  /// Extract one abbreviation declaration and all of its associated attributes.
+  /// Possible return values:
+  ///   DWARFEnumState::Complete - the extraction completed successfully.  This
+  ///       was the last abbrev decl in a sequence, and the user should not call
+  ///       this function again.
+  ///   DWARFEnumState::MoreItems - the extraction completed successfully.  The
+  ///       user should call this function again to retrieve the next decl.
+  ///   llvm::Error - A parsing error occurred.  The debug info is malformed.
+  llvm::Expected<lldb_private::DWARFEnumState>
+  extract(const lldb_private::DWARFDataExtractor &data,
+          lldb::offset_t *offset_ptr);
   bool IsValid();
   void Dump(lldb_private::Stream *s) const;
   bool operator==(const DWARFAbbreviationDeclaration &rhs) const;

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp?rev=356190&r1=356189&r2=356190&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp Thu Mar 14 12:05:55 2019
@@ -25,14 +25,22 @@ void DWARFAbbreviationDeclarationSet::Cl
 //----------------------------------------------------------------------
 // DWARFAbbreviationDeclarationSet::Extract()
 //----------------------------------------------------------------------
-bool DWARFAbbreviationDeclarationSet::Extract(const DWARFDataExtractor &data,
-                                              lldb::offset_t *offset_ptr) {
+llvm::Error
+DWARFAbbreviationDeclarationSet::extract(const DWARFDataExtractor &data,
+                                         lldb::offset_t *offset_ptr) {
   const lldb::offset_t begin_offset = *offset_ptr;
   m_offset = begin_offset;
   Clear();
   DWARFAbbreviationDeclaration abbrevDeclaration;
   dw_uleb128_t prev_abbr_code = 0;
-  while (abbrevDeclaration.Extract(data, offset_ptr)) {
+  DWARFEnumState state = DWARFEnumState::MoreItems;
+  while (state == DWARFEnumState::MoreItems) {
+    llvm::Expected<DWARFEnumState> es =
+        abbrevDeclaration.extract(data, offset_ptr);
+    if (!es)
+      return es.takeError();
+
+    state = *es;
     m_decls.push_back(abbrevDeclaration);
     if (m_idx_offset == 0)
       m_idx_offset = abbrevDeclaration.Code();
@@ -43,7 +51,7 @@ bool DWARFAbbreviationDeclarationSet::Ex
     }
     prev_abbr_code = abbrevDeclaration.Code();
   }
-  return begin_offset != *offset_ptr;
+  return llvm::ErrorSuccess();
 }
 
 //----------------------------------------------------------------------
@@ -138,19 +146,21 @@ DWARFDebugAbbrev::DWARFDebugAbbrev()
 //----------------------------------------------------------------------
 // DWARFDebugAbbrev::Parse()
 //----------------------------------------------------------------------
-void DWARFDebugAbbrev::Parse(const DWARFDataExtractor &data) {
+llvm::Error DWARFDebugAbbrev::parse(const DWARFDataExtractor &data) {
   lldb::offset_t offset = 0;
 
   while (data.ValidOffset(offset)) {
     uint32_t initial_cu_offset = offset;
     DWARFAbbreviationDeclarationSet abbrevDeclSet;
 
-    if (abbrevDeclSet.Extract(data, &offset))
-      m_abbrevCollMap[initial_cu_offset] = abbrevDeclSet;
-    else
-      break;
+    llvm::Error error = abbrevDeclSet.extract(data, &offset);
+    if (error)
+      return error;
+
+    m_abbrevCollMap[initial_cu_offset] = abbrevDeclSet;
   }
   m_prev_abbr_offset_pos = m_abbrevCollMap.end();
+  return llvm::ErrorSuccess();
 }
 
 //----------------------------------------------------------------------

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h?rev=356190&r1=356189&r2=356190&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h Thu Mar 14 12:05:55 2019
@@ -35,8 +35,11 @@ public:
   void Clear();
   dw_offset_t GetOffset() const { return m_offset; }
   void Dump(lldb_private::Stream *s) const;
-  bool Extract(const lldb_private::DWARFDataExtractor &data,
-               lldb::offset_t *offset_ptr);
+
+  /// Extract all abbrev decls in a set.  Returns llvm::ErrorSuccess() on
+  /// success, and an appropriate llvm::Error object otherwise.
+  llvm::Error extract(const lldb_private::DWARFDataExtractor &data,
+                      lldb::offset_t *offset_ptr);
   // void Encode(BinaryStreamBuf& debug_abbrev_buf) const;
   dw_uleb128_t
   AppendAbbrevDeclSequential(const DWARFAbbreviationDeclaration &abbrevDecl);
@@ -64,7 +67,11 @@ public:
   const DWARFAbbreviationDeclarationSet *
   GetAbbreviationDeclarationSet(dw_offset_t cu_abbr_offset) const;
   void Dump(lldb_private::Stream *s) const;
-  void Parse(const lldb_private::DWARFDataExtractor &data);
+
+  /// Extract all abbreviations for a particular compile unit.  Returns
+  /// llvm::ErrorSuccess() on success, and an appropriate llvm::Error object
+  /// otherwise.
+  llvm::Error parse(const lldb_private::DWARFDataExtractor &data);
   void GetUnsupportedForms(std::set<dw_form_t> &invalid_forms) const;
 
 protected:

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.h?rev=356190&r1=356189&r2=356190&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.h Thu Mar 14 12:05:55 2019
@@ -14,6 +14,8 @@
 
 namespace lldb_private {
 
+enum class DWARFEnumState { MoreItems, Complete };
+
 typedef uint32_t DRC_class; // Holds DRC_* class bitfields
 
 enum DW_TAG_Category {

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=356190&r1=356189&r2=356190&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Thu Mar 14 12:05:55 2019
@@ -660,14 +660,23 @@ const DWARFDataExtractor &SymbolFileDWAR
 }
 
 DWARFDebugAbbrev *SymbolFileDWARF::DebugAbbrev() {
-  if (m_abbr == NULL) {
-    const DWARFDataExtractor &debug_abbrev_data = get_debug_abbrev_data();
-    if (debug_abbrev_data.GetByteSize() > 0) {
-      m_abbr.reset(new DWARFDebugAbbrev());
-      if (m_abbr)
-        m_abbr->Parse(debug_abbrev_data);
-    }
+  if (m_abbr)
+    return m_abbr.get();
+
+  const DWARFDataExtractor &debug_abbrev_data = get_debug_abbrev_data();
+  if (debug_abbrev_data.GetByteSize() == 0)
+    return nullptr;
+
+  auto abbr = llvm::make_unique<DWARFDebugAbbrev>();
+  llvm::Error error = abbr->parse(debug_abbrev_data);
+  if (error) {
+    Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
+    LLDB_LOG_ERROR(log, std::move(error),
+                   "Unable to read .debug_abbrev section: {0}");
+    return nullptr;
   }
+
+  m_abbr = std::move(abbr);
   return m_abbr.get();
 }
 




More information about the lldb-commits mailing list