[llvm] 631ff46 - [DebugInfo][NFCI] Refactor DWARFAbbreviationDeclaration::extract
Alex Langford via llvm-commits
llvm-commits at lists.llvm.org
Tue May 16 12:54:55 PDT 2023
Author: Alex Langford
Date: 2023-05-16T12:54:38-07:00
New Revision: 631ff46cbf51dce943f5c136bb6b2b283a8053c0
URL: https://github.com/llvm/llvm-project/commit/631ff46cbf51dce943f5c136bb6b2b283a8053c0
DIFF: https://github.com/llvm/llvm-project/commit/631ff46cbf51dce943f5c136bb6b2b283a8053c0.diff
LOG: [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<ExtractState>` 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
Added:
Modified:
llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h
llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h
index 44d77a3769ddc..02b402e86d233 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 @@ class DWARFAbbreviationDeclaration {
getAttributeValueFromOffset(uint32_t AttrIndex, uint64_t Offset,
const DWARFUnit &U) const;
- bool extract(DataExtractor Data, uint64_t* OffsetPtr);
+ llvm::Expected<ExtractState> 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 5b5b887e2a508..e131c42375ab5 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::ExtractState>
+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<llvm::dwarf::Tag>(Data.getULEB128(OffsetPtr));
if (Tag == DW_TAG_null) {
clear();
- return false;
+ return make_error<llvm::object::GenericBinaryError>(
+ "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<Attribute>(Data.getULEB128(OffsetPtr));
auto F = static_cast<Form>(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<uint8_t> 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<llvm::object::GenericBinaryError>(
+ "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<uint8_t> 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<llvm::object::GenericBinaryError>(
+ "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 3ea3818e7cc33..a8df634d6a63d 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<DWARFAbbreviationDeclaration::ExtractState> 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));
More information about the llvm-commits
mailing list