bulbazord created this revision. bulbazord added reviewers: JDevlieghere, aprantl, fdeazeve, rastogishubham, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
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). Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152476 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp =================================================================== --- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp +++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp @@ -16,7 +16,6 @@ #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" @@ -68,252 +67,6 @@ 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 Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -40,7 +40,6 @@ // Forward Declarations for this DWARF plugin class DebugMapModule; -class DWARFAbbreviationDeclarationSet; class DWARFCompileUnit; class DWARFDebugAbbrev; class DWARFDebugAranges; Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp @@ -440,7 +440,7 @@ } 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() { Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp @@ -812,12 +812,14 @@ 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); - } - return nullptr; + if (!cu) + return nullptr; + + const DWARFAbbreviationDeclarationSet *abbrev_set = cu->GetAbbreviations(); + if (!abbrev_set) + return nullptr; + + return abbrev_set->getAbbreviationDeclaration(m_abbr_idx); } bool DWARFDebugInfoEntry::IsGlobalOrStaticScopeVariable() const { Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h @@ -13,47 +13,12 @@ #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; Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp @@ -14,96 +14,20 @@ 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; @@ -135,6 +59,12 @@ // 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 &pair : m_abbrevCollMap) { + for (const auto &decl : pair.second) { + for (const auto &attr : decl.attributes()) { + if (!DWARFFormValue::FormIsSupported(attr.Form)) + invalid_forms.insert(attr.Form); + } + } + } }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits