[llvm] 32e10aa - [DebugInfo] Change return type of DWARFDebugAbbrev::parse (#67191)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 26 13:45:49 PDT 2023
Author: Alex Langford
Date: 2023-09-26T13:45:46-07:00
New Revision: 32e10aa65c680087c1c9cad2aaeb4526deabe235
URL: https://github.com/llvm/llvm-project/commit/32e10aa65c680087c1c9cad2aaeb4526deabe235
DIFF: https://github.com/llvm/llvm-project/commit/32e10aa65c680087c1c9cad2aaeb4526deabe235.diff
LOG: [DebugInfo] Change return type of DWARFDebugAbbrev::parse (#67191)
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 (without worrying about the
malformed DWARF) 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.
Added:
Modified:
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
llvm/tools/obj2yaml/dwarf2yaml.cpp
llvm/tools/obj2yaml/macho2yaml.cpp
llvm/tools/obj2yaml/obj2yaml.h
llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
Removed:
################################################################################
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);
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp
index 7ba22d2eefff78b..871dbf1a10f438c 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