[lldb][NFCI] Switch to using llvm::DWARFAbbreviationDeclaration

[bulbazord]

Summary

Both LLVM and LLDB implement DWARFAbbreviationDeclaration. As of
631ff46cbf51, llvm's implementation of
DWARFAbbreviationDeclaration::extract behaves the same as LLDB's
implementation, making it easier to merge the implementations.

The only major difference between LLDB's implementation and LLVM's
implementation is that LLVM's DWARFAbbreviationDeclaration is slightly
larger. Specifically, it has some metadata that keeps track of the size
of a declaration (if it has a fixed size) so that it can potentially
optimize extraction in some scenarios. I think this increase in size
should be acceptable and possibly useful on the LLDB side.

[aprantl]

Overall seems good to me. Minor comments inline.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp

27

Is this intentionally a copy or should this be a reference? I have no idea how heavyweight this object is...

56

would std::find_if be shorter or would it look worse?

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h

18

Nit: we usually do local includes first and then the stdlib at the bottom.

[bulbazord]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp

27

This isn't exactly a copy. DataExtractor::GetAsLLVM creates an entirely new object, it can't be a reference. Luckily it is fairly lightweight -- We just copy a few numbers and a pointer I believe.

56

for (const auto &decl : m_decls) {
  if (decl.getCode() == abbrCode)
    return &decl;
}

vs.

auto pos = std::find_if(
     m_decls.begin(), m_decls.end(),
     [abbrCode](const auto &decl) { return decl.getCode() == abbrCode; });
if (pos != m_decls.end())
      return &*pos;

I think it would look worse, personally.

[bulbazord]

Redo include order in DWARFDebugAbbrev.h to match project guidelines

[aprantl]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp

56

Agreed!

lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt

 lldb_tablegen(SymbolFileDWARFProperties.inc -gen-lldb-property-defs
   SOURCE SymbolFileDWARFProperties.td
   TARGET LLDBPluginSymbolFileDWARFPropertiesGen)
 
 lldb_tablegen(SymbolFileDWARFPropertiesEnum.inc -gen-lldb-property-enum-defs
   SOURCE SymbolFileDWARFProperties.td
   TARGET LLDBPluginSymbolFileDWARFPropertiesEnumGen)
 
 add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   AppleDWARFIndex.cpp
   DebugNamesDWARFIndex.cpp
   DIERef.cpp
-  DWARFAbbreviationDeclaration.cpp
   DWARFASTParser.cpp
   DWARFASTParserClang.cpp
   DWARFAttribute.cpp
   DWARFBaseDIE.cpp
   DWARFCompileUnit.cpp
   DWARFContext.cpp
   DWARFDataExtractor.cpp
   DWARFDebugAbbrev.cpp

lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h

-//===-- DWARFAbbreviationDeclaration.h --------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFABBREVIATIONDECLARATION_H
-#define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFABBREVIATIONDECLARATION_H
-
-#include "DWARFAttribute.h"
-#include "DWARFDefines.h"
-#include "SymbolFileDWARF.h"
-#include "llvm/Support/Error.h"
-
-class DWARFAbbreviationDeclaration {
-public:
-  struct AttributeSpec {
-    AttributeSpec(dw_attr_t attr, dw_form_t form, int64_t value)
-        : m_attr(attr), m_form(form), m_value(value) {}
-
-    AttributeSpec(dw_attr_t attr, dw_form_t form)
-        : m_attr(attr), m_form(form), m_value(0) {}
-
-    bool IsImplicitConst() const {
-      return m_form == lldb_private::dwarf::DW_FORM_implicit_const;
-    }
-
-    int64_t GetImplicitConstValue() const { return m_value; }
-
-    dw_attr_t GetAttribute() const { return m_attr; }
-
-    dw_form_t GetForm() const { return m_form; }
-
-  private:
-    dw_attr_t m_attr;
-    dw_form_t m_form;
-    int64_t m_value;
-  };
-
-  enum { InvalidCode = 0 };
-  DWARFAbbreviationDeclaration();
-
-  // For hand crafting an abbreviation declaration
-  DWARFAbbreviationDeclaration(dw_tag_t tag, uint8_t has_children);
-
-  uint32_t Code() const { return m_code; }
-  void SetCode(uint32_t code) { m_code = code; }
-  dw_tag_t Tag() const { return m_tag; }
-  bool HasChildren() const { return m_has_children; }
-  size_t NumAttributes() const { return m_attributes.size(); }
-  dw_form_t GetFormByIndex(uint32_t idx) const {
-    return m_attributes.size() > idx ? m_attributes[idx].GetForm()
-                                     : dw_form_t(0);
-  }
-
-  // idx is assumed to be valid when calling GetAttrAndFormByIndex()
-  void GetAttrAndFormValueByIndex(uint32_t idx, dw_attr_t &attr,
-                                  DWARFFormValue &form_value) const {
-    const AttributeSpec &spec = m_attributes[idx];
-    attr = spec.GetAttribute();
-    form_value.FormRef() = spec.GetForm();
-    if (spec.IsImplicitConst())
-      form_value.SetSigned(spec.GetImplicitConstValue());
-  }
-  dw_form_t GetFormByIndexUnchecked(uint32_t idx) const {
-    return m_attributes[idx].GetForm();
-  }
-  uint32_t FindAttributeIndex(dw_attr_t attr) const;
-
-  /// 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();
-
-protected:
-  uint32_t m_code = InvalidCode;
-  dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
-  uint8_t m_has_children = 0;
-  llvm::SmallVector<AttributeSpec, 4> m_attributes;
-};
-
-#endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFABBREVIATIONDECLARATION_H

lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp

-//===-- DWARFAbbreviationDeclaration.cpp ----------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "DWARFAbbreviationDeclaration.h"
-
-#include "lldb/Core/dwarf.h"
-#include "lldb/Utility/Stream.h"
-
-#include "llvm/Object/Error.h"
-
-#include "DWARFFormValue.h"
-
-using namespace lldb_private;
-using namespace lldb_private::dwarf;
-
-DWARFAbbreviationDeclaration::DWARFAbbreviationDeclaration() : m_attributes() {}
-
-DWARFAbbreviationDeclaration::DWARFAbbreviationDeclaration(dw_tag_t tag,
-                                                           uint8_t has_children)
-    : m_tag(tag), m_has_children(has_children), m_attributes() {}
-
-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;
-
-  m_attributes.clear();
-  m_tag = static_cast<dw_tag_t>(data.GetULEB128(offset_ptr));
-  if (m_tag == DW_TAG_null)
-    return llvm::make_error<llvm::object::GenericBinaryError>(
-        "abbrev decl requires non-null tag.");
-
-  m_has_children = data.GetU8(offset_ptr);
-
-  while (data.ValidOffset(*offset_ptr)) {
-    auto attr = static_cast<dw_attr_t>(data.GetULEB128(offset_ptr));
-    auto form = static_cast<dw_form_t>(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");
-
-    if (form == DW_FORM_implicit_const) {
-      int64_t value = data.GetSLEB128(offset_ptr);
-      m_attributes.emplace_back(attr, form, value);
-      continue;
-    }
-
-    m_attributes.emplace_back(attr, form);
-  }
-
-  return llvm::make_error<llvm::object::GenericBinaryError>(
-      "abbreviation declaration attribute list not terminated with a null "
-      "entry");
-}
-
-bool DWARFAbbreviationDeclaration::IsValid() {
-  return m_code != 0 && m_tag != llvm::dwarf::DW_TAG_null;
-}
-
-uint32_t
-DWARFAbbreviationDeclaration::FindAttributeIndex(dw_attr_t attr) const {
-  for (size_t i = 0; i < m_attributes.size(); ++i) {
-    if (m_attributes[i].GetAttribute() == attr)
-      return i;
-  }
-  return DW_INVALID_INDEX;
-}

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h

 //===-- DWARFDebugAbbrev.h --------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGABBREV_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGABBREV_H
 
-#include <list>
 #include <map>
 
 #include "lldb/lldb-private.h"
 
-#include "DWARFAbbreviationDeclaration.h"
 #include "DWARFDefines.h"
 
+#include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
+
+using DWARFAbbreviationDeclaration = llvm::DWARFAbbreviationDeclaration;
+
 typedef std::vector<DWARFAbbreviationDeclaration>
     DWARFAbbreviationDeclarationColl;
 typedef DWARFAbbreviationDeclarationColl::iterator
     DWARFAbbreviationDeclarationCollIter;
 typedef DWARFAbbreviationDeclarationColl::const_iterator
     DWARFAbbreviationDeclarationCollConstIter;
 
 class DWARFAbbreviationDeclarationSet {

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp

 //===-- DWARFDebugAbbrev.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DWARFDebugAbbrev.h"
 #include "DWARFDataExtractor.h"
+#include "DWARFFormValue.h"
 #include "lldb/Utility/Stream.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 // DWARFAbbreviationDeclarationSet::Clear()
 void DWARFAbbreviationDeclarationSet::Clear() {
   m_idx_offset = 0;
   m_decls.clear();
 }
 
 // DWARFAbbreviationDeclarationSet::Extract()
 llvm::Error
 DWARFAbbreviationDeclarationSet::extract(const DWARFDataExtractor &data,
                                          lldb::offset_t *offset_ptr) {
+  llvm::DataExtractor llvm_data = data.GetAsLLVM();
   const lldb::offset_t begin_offset = *offset_ptr;
   m_offset = begin_offset;
   Clear();
   DWARFAbbreviationDeclaration abbrevDeclaration;
   uint32_t prev_abbr_code = 0;
   while (true) {
-    llvm::Expected<DWARFEnumState> es =
-        abbrevDeclaration.extract(data, offset_ptr);
+    llvm::Expected<llvm::DWARFAbbreviationDeclaration::ExtractState> es =
+        abbrevDeclaration.extract(llvm_data, offset_ptr);
     if (!es)
       return es.takeError();
-    if (*es == DWARFEnumState::Complete)
+    if (*es == llvm::DWARFAbbreviationDeclaration::ExtractState::Complete)
       break;
-    m_decls.push_back(abbrevDeclaration);
     if (m_idx_offset == 0)
-      m_idx_offset = abbrevDeclaration.Code();
-    else if (prev_abbr_code + 1 != abbrevDeclaration.Code()) {
-      // Out of order indexes, we can't do O(1) lookups...
+      m_idx_offset = abbrevDeclaration.getCode();
+    else if (prev_abbr_code + 1 != abbrevDeclaration.getCode())
       m_idx_offset = UINT32_MAX;
-    }
-    prev_abbr_code = abbrevDeclaration.Code();
+
+    prev_abbr_code = abbrevDeclaration.getCode();
+    m_decls.push_back(abbrevDeclaration);
   }
   return llvm::ErrorSuccess();
 }
 
 // DWARFAbbreviationDeclarationSet::GetAbbreviationDeclaration()
 const DWARFAbbreviationDeclaration *
 DWARFAbbreviationDeclarationSet::GetAbbreviationDeclaration(
     uint32_t abbrCode) const {
   if (m_idx_offset == UINT32_MAX) {
-    DWARFAbbreviationDeclarationCollConstIter pos;
-    DWARFAbbreviationDeclarationCollConstIter end = m_decls.end();
-    for (pos = m_decls.begin(); pos != end; ++pos) {
-      if (pos->Code() == abbrCode)
-        return &(*pos);
-    }
-  } else {
-    uint32_t idx = abbrCode - m_idx_offset;
-    if (idx < m_decls.size())
-      return &m_decls[idx];
+    for (const auto &decl : m_decls) {
+      if (decl.getCode() == abbrCode)
+        return &decl;
+    }
+    return nullptr;
   }
+  if (abbrCode < m_idx_offset || abbrCode >= m_idx_offset + m_decls.size())
     return nullptr;
+  return &m_decls[abbrCode - m_idx_offset];
 }
 
 // DWARFAbbreviationDeclarationSet::GetUnsupportedForms()
 void DWARFAbbreviationDeclarationSet::GetUnsupportedForms(
     std::set<dw_form_t> &invalid_forms) const {
-  for (const auto &abbr_decl : m_decls) {
-    const size_t num_attrs = abbr_decl.NumAttributes();
-    for (size_t i=0; i<num_attrs; ++i) {
-      dw_form_t form = abbr_decl.GetFormByIndex(i);
-      if (!DWARFFormValue::FormIsSupported(form))
-        invalid_forms.insert(form);
+  for (const auto &decl : m_decls) {
+    for (const auto &attr : decl.attributes()) {
+      if (!DWARFFormValue::FormIsSupported(attr.Form))
+        invalid_forms.insert(attr.Form);
     }
   }
 }
 
 // Encode
 //
 // Encode the abbreviation table onto the end of the buffer provided into a
 // byte representation as would be found in a ".debug_abbrev" debug information

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

 //===-- DWARFDebugInfoEntry.h -----------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGINFOENTRY_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGINFOENTRY_H
 
 #include "SymbolFileDWARF.h"
 #include "llvm/ADT/SmallVector.h"
 
-#include "DWARFAbbreviationDeclaration.h"
+#include "DWARFAttribute.h"
 #include "DWARFBaseDIE.h"
 #include "DWARFDebugAbbrev.h"
 #include "DWARFDebugRanges.h"
 #include <map>
 #include <optional>
 #include <set>
 #include <vector>
 
+#include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
+
 class DWARFDeclContext;
 
 #define DIE_SIBLING_IDX_BITSIZE 31
 
 /// DWARFDebugInfoEntry objects assume that they are living in one big
 /// vector and do pointer arithmetic on their this pointers. Don't
 /// pass them by value. Due to the way they are constructed in a
 /// std::vector, we cannot delete the copy constructor.
public:
   bool GetDIENamesAndRanges(
       DWARFUnit *cu, const char *&name, const char *&mangled,
       DWARFRangeList &rangeList, std::optional<int> &decl_file,
       std::optional<int> &decl_line, std::optional<int> &decl_column,
       std::optional<int> &call_file, std::optional<int> &call_line,
       std::optional<int> &call_column,
       lldb_private::DWARFExpressionList *frame_base = nullptr) const;
 
-  const DWARFAbbreviationDeclaration *
+  const llvm::DWARFAbbreviationDeclaration *
   GetAbbreviationDeclarationPtr(const DWARFUnit *cu) const;
 
   lldb::offset_t GetFirstAttributeOffset() const;
 
   dw_tag_t Tag() const { return m_tag; }
 
   bool IsNULL() const { return m_abbr_idx == 0; }
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

cu->GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
         "[{0:x16}]: invalid abbreviation code {1}, "
         "please file a bug and "
         "attach the file at the start of this error message",
         (uint64_t)m_offset, (unsigned)abbr_idx);
     // WE can't parse anymore if the DWARF is borked...
     *offset_ptr = UINT32_MAX;
     return false;
   }
-  m_tag = abbrevDecl->Tag();
-  m_has_children = abbrevDecl->HasChildren();
+  m_tag = abbrevDecl->getTag();
+  m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  const uint32_t numAttributes = abbrevDecl->NumAttributes();
-  uint32_t i;
   dw_form_t form;
-  for (i = 0; i < numAttributes; ++i) {
-    form = abbrevDecl->GetFormByIndexUnchecked(i);
+  for (const auto &attribute : abbrevDecl->attributes()) {
+    form = attribute.Form;
     std::optional<uint8_t> fixed_skip_size =
         DWARFFormValue::GetFixedSize(form, cu);
     if (fixed_skip_size)
       offset += *fixed_skip_size;
     else {
       bool form_is_indirect = false;
       do {
         form_is_indirect = false;
unit.GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
       "range extraction failed ({3}), please file a bug "
       "and attach the file at the start of this error message",
       die.GetOffset(),
       llvm::dwarf::FormEncodingString(value.Form()).str().c_str(),
       value.Unsigned(), toString(expected_ranges.takeError()).c_str());
   return DWARFRangeList();
 }
 
+static void ExtractAttrAndFormValue(
+    const llvm::DWARFAbbreviationDeclaration::AttributeSpec &attr_spec,
+    dw_attr_t &attr, DWARFFormValue &form_value) {
+  attr = attr_spec.Attr;
+  form_value.FormRef() = attr_spec.Form;
+  if (attr_spec.isImplicitConst())
+    form_value.SetSigned(attr_spec.getImplicitConstValue());
+}
+
 // GetDIENamesAndRanges
 //
 // Gets the valid address ranges for a given DIE by looking for a
 // DW_AT_low_pc/DW_AT_high_pc pair, DW_AT_entry_pc, or DW_AT_ranges attributes.
 bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
     DWARFUnit *cu, const char *&name, const char *&mangled,
     DWARFRangeList &ranges, std::optional<int> &decl_file,
     std::optional<int> &decl_line, std::optional<int> &decl_column,
     std::optional<int> &call_file, std::optional<int> &call_line,
     std::optional<int> &call_column, DWARFExpressionList *frame_base) const {
   dw_addr_t lo_pc = LLDB_INVALID_ADDRESS;
   dw_addr_t hi_pc = LLDB_INVALID_ADDRESS;
   std::vector<DWARFDIE> dies;
   bool set_frame_base_loclist_addr = false;
 
-  const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
-
   SymbolFileDWARF &dwarf = cu->GetSymbolFileDWARF();
   lldb::ModuleSP module = dwarf.GetObjectFile()->GetModule();
 
-  if (abbrevDecl) {
+  if (const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu)) {
     const DWARFDataExtractor &data = cu->GetData();
     lldb::offset_t offset = GetFirstAttributeOffset();
 
     if (!data.ValidOffset(offset))
       return false;
 
-    const uint32_t numAttributes = abbrevDecl->NumAttributes();
     bool do_offset = false;
 
-    for (uint32_t i = 0; i < numAttributes; ++i) {
+    for (const auto &attribute : abbrevDecl->attributes()) {
       DWARFFormValue form_value(cu);
       dw_attr_t attr;
-      abbrevDecl->GetAttrAndFormValueByIndex(i, attr, form_value);
+      ExtractAttrAndFormValue(attribute, attr, form_value);
 
       if (form_value.ExtractValue(data, &offset)) {
         switch (attr) {
         case DW_AT_low_pc:
           lo_pc = form_value.Address();
 
           if (do_offset)
             hi_pc += lo_pc;

 // specification or abstract origin attributes and including those in the
 // results. Any duplicate attributes will have the first instance take
 // precedence (this can happen for declaration attributes).
 void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
                                         DWARFAttributes &attributes,
                                         Recurse recurse,
                                         uint32_t curr_depth) const {
   const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
-  if (abbrevDecl) {
-    const DWARFDataExtractor &data = cu->GetData();
-    lldb::offset_t offset = GetFirstAttributeOffset();
+  if (!abbrevDecl) {
+    attributes.Clear();
+    return;
+  }
+
 
-    const uint32_t num_attributes = abbrevDecl->NumAttributes();
-    for (uint32_t i = 0; i < num_attributes; ++i) {
-      DWARFFormValue form_value(cu);
-      dw_attr_t attr;
-      abbrevDecl->GetAttrAndFormValueByIndex(i, attr, form_value);
-      const dw_form_t form = form_value.Form();
+  for (const auto &attribute : abbrevDecl->attributes()) {
+    ExtractAttrAndFormValue(attribute, attr, form_value);
 
-      // If we are tracking down DW_AT_specification or DW_AT_abstract_origin
-      // attributes, the depth will be non-zero. We need to omit certain
-      // attributes that don't make sense.
-      switch (attr) {
-      case DW_AT_sibling:
-      case DW_AT_declaration:
-        if (curr_depth > 0) {
-          // This attribute doesn't make sense when combined with the DIE that
-          // references this DIE. We know a DIE is referencing this DIE because
-          // curr_depth is not zero
-          break;
-        }
-        [[fallthrough]];
-      default:
-        attributes.Append(form_value, offset, attr);
-        break;
-      }
 
-      if (recurse == Recurse::yes &&
-          ((attr == DW_AT_specification) || (attr == DW_AT_abstract_origin))) {
-        if (form_value.ExtractValue(data, &offset)) {
-          DWARFDIE spec_die = form_value.Reference();
-          if (spec_die)
-            spec_die.GetDIE()->GetAttributes(spec_die.GetCU(), attributes,
-                                             recurse, curr_depth + 1);
-        }
-      } else {
-        std::optional<uint8_t> fixed_skip_size =
-            DWARFFormValue::GetFixedSize(form, cu);
-        if (fixed_skip_size)
-          offset += *fixed_skip_size;
-        else
-          DWARFFormValue::SkipValue(form, data, &offset, cu);
-      }
-    }
-  } else {
-    attributes.Clear();
-  }
+      const dw_form_t form = form_value.Form();
 }
 
 // GetAttributeValue
 //
 // Get the value of an attribute and return the .debug_info or .debug_types
 // offset of the attribute if it was properly extracted into form_value,
 // or zero if we fail since an offset of zero is invalid for an attribute (it
 // would be a compile unit header).
 dw_offset_t DWARFDebugInfoEntry::GetAttributeValue(
     const DWARFUnit *cu, const dw_attr_t attr, DWARFFormValue &form_value,
     dw_offset_t *end_attr_offset_ptr,
     bool check_specification_or_abstract_origin) const {
   if (const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu)) {
-    uint32_t attr_idx = abbrevDecl->FindAttributeIndex(attr);
+    std::optional<uint32_t> attr_idx = abbrevDecl->findAttributeIndex(attr);
 
-    if (attr_idx != DW_INVALID_INDEX) {
+    if (attr_idx) {
       const DWARFDataExtractor &data = cu->GetData();
       lldb::offset_t offset = GetFirstAttributeOffset();
 
       uint32_t idx = 0;
-      while (idx < attr_idx)
-        DWARFFormValue::SkipValue(abbrevDecl->GetFormByIndex(idx++),
-                                  data, &offset, cu);
+      while (idx < *attr_idx)
+        DWARFFormValue::SkipValue(abbrevDecl->getFormByIndex(idx++), data,
+                                  &offset, cu);
 
       const dw_offset_t attr_offset = offset;
       form_value.SetUnit(cu);
-      form_value.SetForm(abbrevDecl->GetFormByIndex(idx));
+      form_value.SetForm(abbrevDecl->getFormByIndex(idx));
       if (form_value.ExtractValue(data, &offset)) {
         if (end_attr_offset_ptr)
           *end_attr_offset_ptr = offset;
         return attr_offset;
       }
     }
   }
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h

 //===-- DWARFDefines.h ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEFINES_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEFINES_H
 
 #include "lldb/Core/dwarf.h"
 #include <cstdint>
 
 namespace lldb_private {
 
-enum class DWARFEnumState { MoreItems, Complete };
-
 typedef uint32_t DRC_class; // Holds DRC_* class bitfields
 
 const char *DW_TAG_value_to_name(uint32_t val);
 
 const char *DW_AT_value_to_name(uint32_t val);
 
 const char *DW_FORM_value_to_name(uint32_t val);
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

 #include "DWARFContext.h"
 #include "DWARFDataExtractor.h"
 #include "DWARFDefines.h"
 #include "DWARFIndex.h"
 #include "UniqueDWARFASTType.h"
 
 // Forward Declarations for this DWARF plugin
 class DebugMapModule;
-class DWARFAbbreviationDeclaration;
 class DWARFAbbreviationDeclarationSet;
 class DWARFCompileUnit;
 class DWARFDebugAbbrev;
 class DWARFDebugAranges;
 class DWARFDebugInfo;
 class DWARFDebugInfoEntry;
 class DWARFDebugLine;
 class DWARFDebugRanges;

lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
-#include "Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAranges.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/SubsystemRAII.h"
TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start1) {
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   EXPECT_FALSE(bool(error));
   // Make sure we have O(1) access to each abbreviation by making sure the
   // index offset is 1 and not UINT32_MAX
   EXPECT_EQ(abbrev_set.GetIndexOffset(), 1u);
 
   auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(1);
-  EXPECT_EQ(abbrev1->Tag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->HasChildren());
-  EXPECT_EQ(abbrev1->NumAttributes(), 1u);
+  EXPECT_EQ(abbrev1->getTag(), DW_TAG_compile_unit);
+  EXPECT_TRUE(abbrev1->hasChildren());
+  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
   auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(2);
-  EXPECT_EQ(abbrev2->Tag(), DW_TAG_subprogram);
-  EXPECT_FALSE(abbrev2->HasChildren());
-  EXPECT_EQ(abbrev2->NumAttributes(), 1u);
+  EXPECT_EQ(abbrev2->getTag(), DW_TAG_subprogram);
+  EXPECT_FALSE(abbrev2->hasChildren());
+  EXPECT_EQ(abbrev2->getNumAttributes(), 1u);
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start5) {
   // Test that if we have a .debug_abbrev that contains ordered abbreviation
   // codes that start at 5, that we get O(1) access.
 
   const auto byte_order = eByteOrderLittle;
   const uint8_t addr_size = 4;
TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start5) {
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   EXPECT_FALSE(bool(error));
   // Make sure we have O(1) access to each abbreviation by making sure the
   // index offset is 5 and not UINT32_MAX
   EXPECT_EQ(abbrev_set.GetIndexOffset(), 5u);
 
   auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(5);
-  EXPECT_EQ(abbrev1->Tag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->HasChildren());
-  EXPECT_EQ(abbrev1->NumAttributes(), 1u);
+  EXPECT_EQ(abbrev1->getTag(), DW_TAG_compile_unit);
+  EXPECT_TRUE(abbrev1->hasChildren());
+  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
   auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(6);
-  EXPECT_EQ(abbrev2->Tag(), DW_TAG_subprogram);
-  EXPECT_FALSE(abbrev2->HasChildren());
-  EXPECT_EQ(abbrev2->NumAttributes(), 1u);
+  EXPECT_EQ(abbrev2->getTag(), DW_TAG_subprogram);
+  EXPECT_FALSE(abbrev2->hasChildren());
+  EXPECT_EQ(abbrev2->getNumAttributes(), 1u);
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevOutOfOrder) {
   // Test that if we have a .debug_abbrev that contains unordered abbreviation
   // codes, that we can access the information correctly.
 
   const auto byte_order = eByteOrderLittle;
   const uint8_t addr_size = 4;
TEST_F(SymbolFileDWARFTests, TestAbbrevOutOfOrder) {
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   EXPECT_FALSE(bool(error));
   // Make sure we don't have O(1) access to each abbreviation by making sure
   // the index offset is UINT32_MAX
   EXPECT_EQ(abbrev_set.GetIndexOffset(), UINT32_MAX);
 
   auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(2);
-  EXPECT_EQ(abbrev1->Tag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->HasChildren());
-  EXPECT_EQ(abbrev1->NumAttributes(), 1u);
+  EXPECT_EQ(abbrev1->getTag(), DW_TAG_compile_unit);
+  EXPECT_TRUE(abbrev1->hasChildren());
+  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
   auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(1);
-  EXPECT_EQ(abbrev2->Tag(), DW_TAG_subprogram);
-  EXPECT_FALSE(abbrev2->HasChildren());
-  EXPECT_EQ(abbrev2->NumAttributes(), 1u);
+  EXPECT_EQ(abbrev2->getTag(), DW_TAG_subprogram);
+  EXPECT_FALSE(abbrev2->hasChildren());
+  EXPECT_EQ(abbrev2->getNumAttributes(), 1u);
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevInvalidNULLTag) {
   // Test that we detect when an abbreviation has a NULL tag and that we get
   // an error when decoding.
 
   const auto byte_order = eByteOrderLittle;
   const uint8_t addr_size = 4;
   StreamString encoder(Stream::eBinary, addr_size, byte_order);
   encoder.PutULEB128(1); // Abbrev code 1
   encoder.PutULEB128(0); // Invalid NULL tag here!
   encoder.PutHex8(DW_CHILDREN_no);
   encoder.PutULEB128(0);
   encoder.PutULEB128(0);
 
   encoder.PutULEB128(0); // Abbrev code 0 (termination)
 
   DWARFDataExtractor data;
   data.SetData(encoder.GetData(), encoder.GetSize(), byte_order);
   DWARFAbbreviationDeclarationSet abbrev_set;
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   // Verify we get an error
   EXPECT_TRUE(bool(error));
-  EXPECT_EQ("abbrev decl requires non-null tag.",
+  EXPECT_EQ("abbreviation declaration requires a non-null tag",
             llvm::toString(std::move(error)));
-
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevNullAttrValidForm) {
   // Test that we detect when an abbreviation has a NULL attribute and a non
   // NULL form and that we get an error when decoding.
 
   const auto byte_order = eByteOrderLittle;
   const uint8_t addr_size = 4;
TEST_F(SymbolFileDWARFTests, TestAbbrevNullAttrValidForm) {
 
   DWARFDataExtractor data;
   data.SetData(encoder.GetData(), encoder.GetSize(), byte_order);
   DWARFAbbreviationDeclarationSet abbrev_set;
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   // Verify we get an error
   EXPECT_TRUE(bool(error));
-  EXPECT_EQ("malformed abbreviation declaration attribute",
+  EXPECT_EQ("malformed abbreviation declaration attribute. Either the "
+            "attribute or the form is zero while the other is not",
             llvm::toString(std::move(error)));
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevValidAttrNullForm) {
   // Test that we detect when an abbreviation has a valid attribute and a
   // NULL form and that we get an error when decoding.
 
   const auto byte_order = eByteOrderLittle;
TEST_F(SymbolFileDWARFTests, TestAbbrevValidAttrNullForm) {
 
   DWARFDataExtractor data;
   data.SetData(encoder.GetData(), encoder.GetSize(), byte_order);
   DWARFAbbreviationDeclarationSet abbrev_set;
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   // Verify we get an error
   EXPECT_TRUE(bool(error));
-  EXPECT_EQ("malformed abbreviation declaration attribute",
+  EXPECT_EQ("malformed abbreviation declaration attribute. Either the "
+            "attribute or the form is zero while the other is not",
             llvm::toString(std::move(error)));
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevMissingTerminator) {
   // Test that we detect when an abbreviation has a valid attribute and a
   // form, but is missing the NULL attribute and form that terminates an
   // abbreviation
 
TEST_F(SymbolFileDWARFTests, TestAbbrevMissingTerminator) {
 
   DWARFDataExtractor data;
   data.SetData(encoder.GetData(), encoder.GetSize(), byte_order);
   DWARFAbbreviationDeclarationSet abbrev_set;
   lldb::offset_t data_offset = 0;
   llvm::Error error = abbrev_set.extract(data, &data_offset);
   // Verify we get an error
   EXPECT_TRUE(bool(error));
-  EXPECT_EQ("abbreviation declaration attribute list not terminated with a "
-            "null entry", llvm::toString(std::move(error)));
+  EXPECT_EQ("abbreviation declaration attribute list was not terminated with a "
+            "null entry",
+            llvm::toString(std::move(error)));
 }
 
 TEST_F(SymbolFileDWARFTests, ParseArangesNonzeroSegmentSize) {
   // This `.debug_aranges` table header is a valid 32bit big-endian section
   // according to the DWARFv5 spec:6.2.1, but contains segment selectors which
   // are not supported by lldb, and should be gracefully rejected
   const unsigned char binary_data[] = {
       0, 0, 0, 41, // unit_length (length field not including this field itself)