[llvm] [DebugInfo] Change return type of DWARFDebugAbbrev::parse (PR #67191)
Alex Langford via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 25 13:03:04 PDT 2023
https://github.com/bulbazord updated https://github.com/llvm/llvm-project/pull/67191
>From 58f3725ab3e514a455299e4e6992e845e2a98b68 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Fri, 22 Sep 2023 10:50:42 -0700
Subject: [PATCH 1/2] [DebugInfo] Change return type of DWARFDebugAbbrev::parse
To make DWARFDebugAbbrev more amenable to error-handling, I would like
to change the return type of DWARFDebugAbbrev::parse from `void` to
`Error`. Users of DWARFDebugAbbrev can consume the error if they want to
use all the valid DWARF that was parsed or stop when the parse fails if
the use case needs to be strict.
This also will bring the LLVM DWARFDebugAbbrev interface closer to
LLDB's which opens up the opportunity for LLDB adopt the LLVM
implementation with minimal changes.
---
.../llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h | 2 +-
llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp | 14 ++++++++------
llvm/tools/obj2yaml/dwarf2yaml.cpp | 13 ++++++++++---
llvm/tools/obj2yaml/macho2yaml.cpp | 6 ++----
llvm/tools/obj2yaml/obj2yaml.h | 2 +-
5 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
index 8e4aa3aa61e9ea5..6439827ef70f0f0 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
@@ -70,7 +70,7 @@ class DWARFDebugAbbrev {
getAbbreviationDeclarationSet(uint64_t CUAbbrOffset) const;
void dump(raw_ostream &OS) const;
- void parse() const;
+ Error parse() const;
DWARFAbbreviationDeclarationSetMap::const_iterator begin() const {
assert(!Data && "Must call parse before iterating over DWARFDebugAbbrev");
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
index 3014e61f566a909..85959ecc5e17f14 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
@@ -105,9 +105,9 @@ std::string DWARFAbbreviationDeclarationSet::getCodeRange() const {
DWARFDebugAbbrev::DWARFDebugAbbrev(DataExtractor Data)
: AbbrDeclSets(), PrevAbbrOffsetPos(AbbrDeclSets.end()), Data(Data) {}
-void DWARFDebugAbbrev::parse() const {
+Error DWARFDebugAbbrev::parse() const {
if (!Data)
- return;
+ return Error::success();
uint64_t Offset = 0;
auto I = AbbrDeclSets.begin();
while (Data->isValidOffset(Offset)) {
@@ -116,17 +116,19 @@ void DWARFDebugAbbrev::parse() const {
uint64_t CUAbbrOffset = Offset;
DWARFAbbreviationDeclarationSet AbbrDecls;
if (Error Err = AbbrDecls.extract(*Data, &Offset)) {
- // FIXME: We should propagate the error upwards.
- consumeError(std::move(Err));
- break;
+ Data = std::nullopt;
+ return Err;
}
AbbrDeclSets.insert(I, std::make_pair(CUAbbrOffset, std::move(AbbrDecls)));
}
Data = std::nullopt;
+ return Error::success();
}
void DWARFDebugAbbrev::dump(raw_ostream &OS) const {
- parse();
+ if (Error Err = parse())
+ // FIXME: We should propagate this error or otherwise display it.
+ llvm::consumeError(std::move(Err));
if (AbbrDeclSets.empty()) {
OS << "< EMPTY >\n";
diff --git a/llvm/tools/obj2yaml/dwarf2yaml.cpp b/llvm/tools/obj2yaml/dwarf2yaml.cpp
index 3d3798d5f18d784..f2c96968cc7deeb 100644
--- a/llvm/tools/obj2yaml/dwarf2yaml.cpp
+++ b/llvm/tools/obj2yaml/dwarf2yaml.cpp
@@ -22,11 +22,12 @@
using namespace llvm;
-void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
+Error dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
auto AbbrevSetPtr = DCtx.getDebugAbbrev();
if (AbbrevSetPtr) {
uint64_t AbbrevTableID = 0;
- AbbrevSetPtr->parse();
+ if (Error Err = AbbrevSetPtr->parse())
+ return Err;
for (auto AbbrvDeclSet : *AbbrevSetPtr) {
Y.DebugAbbrev.emplace_back();
Y.DebugAbbrev.back().ID = AbbrevTableID++;
@@ -48,6 +49,7 @@ void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
}
}
}
+ return Error::success();
}
Error dumpDebugAddr(DWARFContext &DCtx, DWARFYAML::Data &Y) {
@@ -220,7 +222,12 @@ void dumpDebugInfo(DWARFContext &DCtx, DWARFYAML::Data &Y) {
if (NewUnit.Version >= 5)
NewUnit.Type = (dwarf::UnitType)CU->getUnitType();
const DWARFDebugAbbrev *DebugAbbrev = DCtx.getDebugAbbrev();
- DebugAbbrev->parse();
+ // FIXME: Ideally we would propagate this error upwards, but that would
+ // prevent us from displaying any debug info at all. For now we just consume
+ // the error and display everything that was parsed successfully.
+ if (Error Err = DebugAbbrev->parse())
+ llvm::consumeError(std::move(Err));
+
NewUnit.AbbrevTableID = std::distance(
DebugAbbrev->begin(),
llvm::find_if(
diff --git a/llvm/tools/obj2yaml/macho2yaml.cpp b/llvm/tools/obj2yaml/macho2yaml.cpp
index ed678ea28a71735..efd43a5f1285f74 100644
--- a/llvm/tools/obj2yaml/macho2yaml.cpp
+++ b/llvm/tools/obj2yaml/macho2yaml.cpp
@@ -139,10 +139,8 @@ MachODumper::constructSection(MachO::section_64 Sec, size_t SecIndex) {
static Error dumpDebugSection(StringRef SecName, DWARFContext &DCtx,
DWARFYAML::Data &DWARF) {
- if (SecName == "__debug_abbrev") {
- dumpDebugAbbrev(DCtx, DWARF);
- return Error::success();
- }
+ if (SecName == "__debug_abbrev")
+ return dumpDebugAbbrev(DCtx, DWARF);
if (SecName == "__debug_aranges")
return dumpDebugARanges(DCtx, DWARF);
if (SecName == "__debug_info") {
diff --git a/llvm/tools/obj2yaml/obj2yaml.h b/llvm/tools/obj2yaml/obj2yaml.h
index e0204c6c608f4b1..03d7db5317acde1 100644
--- a/llvm/tools/obj2yaml/obj2yaml.h
+++ b/llvm/tools/obj2yaml/obj2yaml.h
@@ -46,7 +46,7 @@ struct Data;
}
} // namespace llvm
-void dumpDebugAbbrev(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
+llvm::Error dumpDebugAbbrev(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
llvm::Error dumpDebugAddr(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
llvm::Error dumpDebugARanges(llvm::DWARFContext &DCtx,
llvm::DWARFYAML::Data &Y);
>From 02cd3a2a28a37fa7cb6c93821e4eab41aefe33b8 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Mon, 25 Sep 2023 13:02:29 -0700
Subject: [PATCH 2/2] [DebugInfo] Add unit test for DWARFDebugAbbrev::parse
Just to make sure that an error is correctly propagated upwards.
---
.../DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
index 7ba22d2eefff78b..a85b81548957f1c 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
@@ -412,3 +412,28 @@ TEST(DWARFDebugAbbrevTest, DWARFAbbrevDeclSetMissingTerminator) {
"abbreviation declaration attribute list was not terminated with a "
"null entry"));
}
+
+TEST(DWARFDebugAbbrevTest, DWARFDebugAbbrevParseError) {
+ SmallString<64> RawData;
+ raw_svector_ostream OS(RawData);
+ const uint32_t FirstCode = 70;
+ // First, we're going to manually add good data.
+ writeValidAbbreviationDeclarations(OS, FirstCode, InOrder);
+
+ // Afterwards, we're going to write an Abbreviation Decl manually without a
+ // termintating sequence.
+ encodeULEB128(FirstCode - 1 , OS);
+ encodeULEB128(DW_TAG_compile_unit, OS);
+ OS << static_cast<uint8_t>(DW_CHILDREN_no);
+ encodeULEB128(DW_AT_name, OS);
+ encodeULEB128(DW_FORM_strp, OS);
+
+ // The specific error should percolate up to the DWARFDebugAbbrev::parse().
+ DataExtractor Data(RawData, sys::IsLittleEndianHost, sizeof(uint64_t));
+ DWARFDebugAbbrev DebugAbbrev(Data);
+ EXPECT_THAT_ERROR(
+ DebugAbbrev.parse(),
+ FailedWithMessage(
+ "abbreviation declaration attribute list was not terminated with a "
+ "null entry"));
+}
More information about the llvm-commits
mailing list