[lldb] Remove lldb's DWARFAbbreviationDeclarationSet in favor of llvm's

[bulbazord]

Summary

lldb's and llvm's implementations of DWARFAbbreviationDeclarationSet are
now close enough (almost the same, actually) to replace lldb's with
llvm's wholesale. llvm's is also tested against the same kinds of
scenarios that lldb's is tested against so we can remove lldb's tests
here. (see: llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp).

[fdeazeve]

This is awesome! I believe you said there was no measurable perf diff?

[aprantl]

Nice!

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

61–62

at some point we could modernize this into

llvm::DenseSet<dw_form_t> DWARFDebugAbbrev::GetUnsupportedForms()

63–66

nit: LLVM coding style would probably elide the {}

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

23

for a later commit: this should probably be a DenseMap for better performance?

[bulbazord]

In D152476#4408654, @fdeazeve wrote:

This is awesome! I believe you said there was no measurable perf diff?

Yeah, my initial experiments measured no significant perf difference no matter how you built lldb. llvm's and lldb's DWARFAbbreviationDeclarationSet implementations both use llvm's DWARFAbbreviationDeclaration and have the same semantics around their extraction. I intentionally made both implementations behave the exact same so that this change would be as simple as possible.

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

63–66

Will update, thanks!

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

23

Possibly? It would be interesting to switch away from std::map to a different type (like DenseMap) to measure performance. Either way, I also want to switch us to use llvm::DWARFDebugAbbrev, so it might be worth doing that work first.

[bulbazord]

Adjust formatting

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 "DWARFDefines.h"
 #include "lldb/lldb-private.h"
 
 #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
 
 #include <map>
 
 using DWARFAbbreviationDeclaration = llvm::DWARFAbbreviationDeclaration;
-
-typedef std::vector<DWARFAbbreviationDeclaration>
-    DWARFAbbreviationDeclarationColl;
-typedef DWARFAbbreviationDeclarationColl::iterator
-    DWARFAbbreviationDeclarationCollIter;
-typedef DWARFAbbreviationDeclarationColl::const_iterator
-    DWARFAbbreviationDeclarationCollConstIter;
-
-class DWARFAbbreviationDeclarationSet {
-public:
-  DWARFAbbreviationDeclarationSet() : m_offset(DW_INVALID_OFFSET), m_decls() {}
-
-  DWARFAbbreviationDeclarationSet(dw_offset_t offset, uint32_t idx_offset)
-      : m_offset(offset), m_idx_offset(idx_offset), m_decls() {}
-
-  void Clear();
-  dw_offset_t GetOffset() const { return m_offset; }
-
-  /// 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;
-  void GetUnsupportedForms(std::set<dw_form_t> &invalid_forms) const;
-
-  const DWARFAbbreviationDeclaration *
-  GetAbbreviationDeclaration(uint32_t abbrCode) const;
-
-  /// Unit test accessor functions.
-  /// @{
-  uint32_t GetIndexOffset() const { return m_idx_offset; }
-  /// @}
-private:
-  dw_offset_t m_offset;
-  uint32_t m_idx_offset = 0;
-  std::vector<DWARFAbbreviationDeclaration> m_decls;
-};
+using DWARFAbbreviationDeclarationSet = llvm::DWARFAbbreviationDeclarationSet;
 
 typedef std::map<dw_offset_t, DWARFAbbreviationDeclarationSet>
     DWARFAbbreviationDeclarationCollMap;
 typedef DWARFAbbreviationDeclarationCollMap::iterator
     DWARFAbbreviationDeclarationCollMapIter;
 typedef DWARFAbbreviationDeclarationCollMap::const_iterator
     DWARFAbbreviationDeclarationCollMapConstIter;
 
 class DWARFDebugAbbrev {
 public:

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<llvm::DWARFAbbreviationDeclaration::ExtractState> es =
-        abbrevDeclaration.extract(llvm_data, offset_ptr);
-    if (!es)
-      return es.takeError();
-    if (*es == llvm::DWARFAbbreviationDeclaration::ExtractState::Complete)
-      break;
-    if (m_idx_offset == 0)
-      m_idx_offset = abbrevDeclaration.getCode();
-    else if (prev_abbr_code + 1 != abbrevDeclaration.getCode())
-      m_idx_offset = UINT32_MAX;
-
-    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) {
-    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 &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
-// section.
-// void
-// DWARFAbbreviationDeclarationSet::Encode(BinaryStreamBuf& debug_abbrev_buf)
-// const
-//{
-//  DWARFAbbreviationDeclarationCollConstIter pos;
-//  DWARFAbbreviationDeclarationCollConstIter end = m_decls.end();
-//  for (pos = m_decls.begin(); pos != end; ++pos)
-//      pos->Append(debug_abbrev_buf);
-//  debug_abbrev_buf.Append8(0);
-//}
-
 // DWARFDebugAbbrev constructor
 DWARFDebugAbbrev::DWARFDebugAbbrev()
     : m_abbrevCollMap(), m_prev_abbr_offset_pos(m_abbrevCollMap.end()) {}
 
 // DWARFDebugAbbrev::Parse()
 llvm::Error DWARFDebugAbbrev::parse(const DWARFDataExtractor &data) {
+  llvm::DataExtractor llvm_data = data.GetAsLLVM();
   lldb::offset_t offset = 0;
 
-  while (data.ValidOffset(offset)) {
+  while (llvm_data.isValidOffset(offset)) {
     uint32_t initial_cu_offset = offset;
     DWARFAbbreviationDeclarationSet abbrevDeclSet;
 
-    llvm::Error error = abbrevDeclSet.extract(data, &offset);
+    llvm::Error error = abbrevDeclSet.extract(llvm_data, &offset);
     if (error)
       return error;
 
     m_abbrevCollMap[initial_cu_offset] = abbrevDeclSet;
   }
   m_prev_abbr_offset_pos = m_abbrevCollMap.end();
   return llvm::ErrorSuccess();
 }
DWARFDebugAbbrev::GetAbbreviationDeclarationSet(
 
   if (pos != m_abbrevCollMap.end())
     return &(pos->second);
   return nullptr;
 }
 
 // DWARFDebugAbbrev::GetUnsupportedForms()
 void DWARFDebugAbbrev::GetUnsupportedForms(
     std::set<dw_form_t> &invalid_forms) const {
   for (const auto &pair : m_abbrevCollMap)
-    pair.second.GetUnsupportedForms(invalid_forms);
+    for (const auto &decl : pair.second)
+      for (const auto &attr : decl.attributes())
+        if (!DWARFFormValue::FormIsSupported(attr.Form))
+          invalid_forms.insert(attr.Form);
 }

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

 }
 
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
 
 const DWARFAbbreviationDeclaration *
 DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(const DWARFUnit *cu) const {
-  if (cu) {
-    const DWARFAbbreviationDeclarationSet *abbrev_set = cu->GetAbbreviations();
-    if (abbrev_set)
-      return abbrev_set->GetAbbreviationDeclaration(m_abbr_idx);
-  }
+  if (!cu)
+    return nullptr;
+
+  if (!abbrev_set)
     return nullptr;
+
+  return abbrev_set->getAbbreviationDeclaration(m_abbr_idx);
 }
 
 bool DWARFDebugInfoEntry::IsGlobalOrStaticScopeVariable() const {
   if (Tag() != DW_TAG_variable)
     return false;
   const DWARFDebugInfoEntry *parent_die = GetParent();
   while (parent_die != nullptr) {
     switch (parent_die->Tag()) {

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

size_t DWARFUnit::GetDebugInfoSize() const {
   return GetLengthByteSize() + GetLength() - GetHeaderByteSize();
 }
 
 const DWARFAbbreviationDeclarationSet *DWARFUnit::GetAbbreviations() const {
   return m_abbrevs;
 }
 
 dw_offset_t DWARFUnit::GetAbbrevOffset() const {
-  return m_abbrevs ? m_abbrevs->GetOffset() : DW_INVALID_OFFSET;
+  return m_abbrevs ? m_abbrevs->getOffset() : DW_INVALID_OFFSET;
 }
 
 dw_offset_t DWARFUnit::GetLineTableOffset() {
   ExtractUnitDIENoDwoIfNeeded();
   return m_line_table_offset;
 }
 
 void DWARFUnit::SetAddrBase(dw_addr_t addr_base) { m_addr_base = addr_base; }

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 DWARFAbbreviationDeclarationSet;
 class DWARFCompileUnit;
 class DWARFDebugAbbrev;
 class DWARFDebugAranges;
 class DWARFDebugInfo;
 class DWARFDebugInfoEntry;
 class DWARFDebugLine;
 class DWARFDebugRanges;
 class DWARFDeclContext;

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/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"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Core/Address.h"
TEST_F(SymbolFileDWARFTests, TestAbilitiesForDWARF) {
   SymbolFile *symfile = module->GetSymbolFile();
   ASSERT_NE(nullptr, symfile);
   EXPECT_EQ(symfile->GetPluginName(), SymbolFileDWARF::GetPluginNameStatic());
 
   uint32_t expected_abilities = SymbolFile::kAllAbilities;
   EXPECT_EQ(expected_abilities, symfile->CalculateAbilities());
 }
 
-TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start1) {
-  // Test that if we have a .debug_abbrev that contains ordered abbreviation
-  // codes that start at 1, that we get O(1) access.
-
-  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(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_yes);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(2); // Abbrev code 2
-  encoder.PutULEB128(DW_TAG_subprogram);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  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);
-  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->getTag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->hasChildren());
-  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
-  auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(2);
-  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;
-  StreamString encoder(Stream::eBinary, addr_size, byte_order);
-  encoder.PutULEB128(5); // Abbrev code 5
-  encoder.PutULEB128(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_yes);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(6); // Abbrev code 6
-  encoder.PutULEB128(DW_TAG_subprogram);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  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);
-  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->getTag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->hasChildren());
-  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
-  auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(6);
-  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;
-  StreamString encoder(Stream::eBinary, addr_size, byte_order);
-  encoder.PutULEB128(2); // Abbrev code 2
-  encoder.PutULEB128(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_yes);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(1); // Abbrev code 1
-  encoder.PutULEB128(DW_TAG_subprogram);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  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);
-  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->getTag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->hasChildren());
-  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
-  auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(1);
-  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("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;
-  StreamString encoder(Stream::eBinary, addr_size, byte_order);
-  encoder.PutULEB128(1); // Abbrev code 1
-  encoder.PutULEB128(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(0); // Invalid NULL DW_AT
-  encoder.PutULEB128(DW_FORM_strp); // With a valid form
-  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("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;
-  const uint8_t addr_size = 4;
-  StreamString encoder(Stream::eBinary, addr_size, byte_order);
-  encoder.PutULEB128(1); // Abbrev code 1
-  encoder.PutULEB128(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name); // Valid attribute
-  encoder.PutULEB128(0); // NULL form
-  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("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
-
-  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(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  // Don't add the NULL DW_AT and NULL DW_FORM terminator
-
-  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 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)
       0, 2,        // DWARF version number (half)
       0, 0, 0, 0, // offset into the .debug_info_table (ignored for the purposes