From 631ff46cbf51dce943f5c136bb6b2b283a8053c0 Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Mon, 15 May 2023 13:21:15 -0700 Subject: [PATCH] [DebugInfo][NFCI] Refactor DWARFAbbreviationDeclaration::extract The motivation behind this refactor is to be able to use DWARFAbbreviationDeclaration from LLDB. LLDB has its own implementation of DWARFAbbreviationDeclaration that is very similar to LLVM's but it has different semantics around error handling. This patch modifies llvm::DWARFAbbreviationDeclaration::extract to return an `llvm::Expected` to differentiate between "I am done extracting" and "An error has occured", something which the current return type (bool) does not accurately capture. Differential Revision: https://reviews.llvm.org/D150607 --- .../DebugInfo/DWARF/DWARFAbbreviationDeclaration.h | 3 +- .../DWARF/DWARFAbbreviationDeclaration.cpp | 127 +++++++++++---------- llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp | 21 +++- 3 files changed, 84 insertions(+), 67 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h index 44d77a37..02b402e 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h @@ -25,6 +25,7 @@ class raw_ostream; class DWARFAbbreviationDeclaration { public: + enum class ExtractState { Complete, MoreItems }; struct AttributeSpec { AttributeSpec(dwarf::Attribute A, dwarf::Form F, int64_t Value) : Attr(A), Form(F), Value(Value) { @@ -172,7 +173,7 @@ public: getAttributeValueFromOffset(uint32_t AttrIndex, uint64_t Offset, const DWARFUnit &U) const; - bool extract(DataExtractor Data, uint64_t* OffsetPtr); + llvm::Expected extract(DataExtractor Data, uint64_t *OffsetPtr); void dump(raw_ostream &OS) const; // Return an optional byte size of all attribute data in this abbreviation diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp index 5b5b887..e131c42 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp @@ -34,20 +34,20 @@ DWARFAbbreviationDeclaration::DWARFAbbreviationDeclaration() { clear(); } -bool -DWARFAbbreviationDeclaration::extract(DataExtractor Data, - uint64_t* OffsetPtr) { +llvm::Expected +DWARFAbbreviationDeclaration::extract(DataExtractor Data, uint64_t *OffsetPtr) { clear(); const uint64_t Offset = *OffsetPtr; Code = Data.getULEB128(OffsetPtr); - if (Code == 0) { - return false; - } + if (Code == 0) + return ExtractState::Complete; + CodeByteSize = *OffsetPtr - Offset; Tag = static_cast(Data.getULEB128(OffsetPtr)); if (Tag == DW_TAG_null) { clear(); - return false; + return make_error( + "abbreviation declaration requires a non-null tag"); } uint8_t ChildrenByte = Data.getU8(OffsetPtr); HasChildren = (ChildrenByte == DW_CHILDREN_yes); @@ -57,70 +57,77 @@ DWARFAbbreviationDeclaration::extract(DataExtractor Data, FixedAttributeSize = FixedSizeInfo(); // Read all of the abbreviation attributes and forms. - while (true) { + while (Data.isValidOffset(*OffsetPtr)) { auto A = static_cast(Data.getULEB128(OffsetPtr)); auto F = static_cast
(Data.getULEB128(OffsetPtr)); - if (A && F) { - bool IsImplicitConst = (F == DW_FORM_implicit_const); - if (IsImplicitConst) { - int64_t V = Data.getSLEB128(OffsetPtr); - AttributeSpecs.push_back(AttributeSpec(A, F, V)); - continue; - } - std::optional ByteSize; - // If this abbrevation still has a fixed byte size, then update the - // FixedAttributeSize as needed. - switch (F) { - case DW_FORM_addr: - if (FixedAttributeSize) - ++FixedAttributeSize->NumAddrs; - break; - case DW_FORM_ref_addr: - if (FixedAttributeSize) - ++FixedAttributeSize->NumRefAddrs; - break; + // We successfully reached the end of this abbreviation declaration + // since both attribute and form are zero. There may be more abbreviation + // declarations afterwards. + if (!A && !F) + return ExtractState::MoreItems; - case DW_FORM_strp: - case DW_FORM_GNU_ref_alt: - case DW_FORM_GNU_strp_alt: - case DW_FORM_line_strp: - case DW_FORM_sec_offset: - case DW_FORM_strp_sup: - if (FixedAttributeSize) - ++FixedAttributeSize->NumDwarfOffsets; - break; - - default: - // The form has a byte size that doesn't depend on Params. - // If it's a fixed size, keep track of it. - if ((ByteSize = dwarf::getFixedFormByteSize(F, dwarf::FormParams()))) { - if (FixedAttributeSize) - FixedAttributeSize->NumBytes += *ByteSize; - break; - } - // Indicate we no longer have a fixed byte size for this - // abbreviation by clearing the FixedAttributeSize optional value - // so it doesn't have a value. - FixedAttributeSize.reset(); - break; - } - // Record this attribute and its fixed size if it has one. - AttributeSpecs.push_back(AttributeSpec(A, F, ByteSize)); - } else if (A == 0 && F == 0) { - // We successfully reached the end of this abbreviation declaration - // since both attribute and form are zero. - break; - } else { + if (!A || !F) { // Attribute and form pairs must either both be non-zero, in which case // they are added to the abbreviation declaration, or both be zero to // terminate the abbrevation declaration. In this case only one was // zero which is an error. clear(); - return false; + return make_error( + "malformed abbreviation declaration attribute. Either the attribute " + "or the form is zero while the other is not"); + } + + bool IsImplicitConst = (F == DW_FORM_implicit_const); + if (IsImplicitConst) { + int64_t V = Data.getSLEB128(OffsetPtr); + AttributeSpecs.push_back(AttributeSpec(A, F, V)); + continue; + } + std::optional ByteSize; + // If this abbrevation still has a fixed byte size, then update the + // FixedAttributeSize as needed. + switch (F) { + case DW_FORM_addr: + if (FixedAttributeSize) + ++FixedAttributeSize->NumAddrs; + break; + + case DW_FORM_ref_addr: + if (FixedAttributeSize) + ++FixedAttributeSize->NumRefAddrs; + break; + + case DW_FORM_strp: + case DW_FORM_GNU_ref_alt: + case DW_FORM_GNU_strp_alt: + case DW_FORM_line_strp: + case DW_FORM_sec_offset: + case DW_FORM_strp_sup: + if (FixedAttributeSize) + ++FixedAttributeSize->NumDwarfOffsets; + break; + + default: + // The form has a byte size that doesn't depend on Params. + // If it's a fixed size, keep track of it. + if ((ByteSize = dwarf::getFixedFormByteSize(F, dwarf::FormParams()))) { + if (FixedAttributeSize) + FixedAttributeSize->NumBytes += *ByteSize; + break; + } + // Indicate we no longer have a fixed byte size for this + // abbreviation by clearing the FixedAttributeSize optional value + // so it doesn't have a value. + FixedAttributeSize.reset(); + break; } + // Record this attribute and its fixed size if it has one. + AttributeSpecs.push_back(AttributeSpec(A, F, ByteSize)); } - return true; + return make_error( + "abbreviation declaration attribute list was not terminated with a null " + "entry"); } void DWARFAbbreviationDeclaration::dump(raw_ostream &OS) const { diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp index 3ea3818..a8df634 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp @@ -32,14 +32,23 @@ bool DWARFAbbreviationDeclarationSet::extract(DataExtractor Data, Offset = BeginOffset; DWARFAbbreviationDeclaration AbbrDecl; uint32_t PrevAbbrCode = 0; - while (AbbrDecl.extract(Data, OffsetPtr)) { + while (true) { + llvm::Expected ES = + AbbrDecl.extract(Data, OffsetPtr); + if (!ES) { + // FIXME: We should propagate the error upwards. + llvm::consumeError(ES.takeError()); + break; + } + + if (*ES == DWARFAbbreviationDeclaration::ExtractState::Complete) + break; + if (FirstAbbrCode == 0) { FirstAbbrCode = AbbrDecl.getCode(); - } else { - if (PrevAbbrCode + 1 != AbbrDecl.getCode()) { - // Codes are not consecutive, can't do O(1) lookups. - FirstAbbrCode = UINT32_MAX; - } + } else if (PrevAbbrCode + 1 != AbbrDecl.getCode()) { + // Codes are not consecutive, can't do O(1) lookups. + FirstAbbrCode = UINT32_MAX; } PrevAbbrCode = AbbrDecl.getCode(); Decls.push_back(std::move(AbbrDecl)); -- 2.7.4