[NFC] Separate high-level-dependent portions of DWARFExpression (revised) #143170
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class. Dwarf expressions are used in many contexts without Dwarf units and other higher-level Dwarf concepts. The code currently includes conditionals which fall back to defaults if some high-level construct is not available. For example, prettyPrintBaseTypeRef checks U for null. These checks mean that a Dwarf expressions can be used without high-level *run* time* dependencies on Dwarf unit. But as coded they cannot be used without high level *build* time dependencies on Dwarf unit. One in a series of NFC DebugInfo/DWARF refactoring changes to layer it more cleanly, so that binary CFI parsing can be used from low-level code, (such as byte strings created via .cfi_escape) without circular dependencies.
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-llvm-binary-utilities Author: None (Sterling-Augustine) Changes(Revised version of a previous, unreviewed, PR.) Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class. This is one in a series of refactoring PRs to separate dwarf functionality into lower-level pieces usable without object files and sections at build time. The code is already written this way via various "if (section == nullptr)" and similar conditionals. So the functionality itself is needed and exists, but only as a runtime feature. The goal of these refactors is to remove the build-time dependencies, which will allow the existing functionality to be used from lower-level parts of the compiler. Particularly from lib/MC/.... More information at: Patch is 25.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143170.diff 15 Files Affected:
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 4b2b111e08e6d..09d8bd833d543 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -80,8 +80,8 @@ void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
};
llvm::DIDumpOptions DumpOpts;
DumpOpts.GetNameForDWARFReg = GetRegName;
- llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
- .print(s->AsRawOstream(), DumpOpts, nullptr);
+ llvm::DWARFExpression E(m_data.GetAsLLVM(), m_data.GetAddressByteSize());
+ llvm::DWARFExpressionPrinter::print(&E, s->AsRawOstream(), DumpOpts, nullptr);
}
RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }
diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp
index b1a96b5e26840..9ccbd4064900f 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -87,8 +87,9 @@ static void DumpDWARFExpr(Stream &s, llvm::ArrayRef<uint8_t> expr, Thread *threa
if (auto order_and_width = GetByteOrderAndAddrSize(thread)) {
llvm::DataExtractor data(expr, order_and_width->first == eByteOrderLittle,
order_and_width->second);
- llvm::DWARFExpression(data, order_and_width->second, llvm::dwarf::DWARF32)
- .print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr);
+ llvm::DWARFExpression E(data, order_and_width->second, llvm::dwarf::DWARF32);
+ llvm::DWARFExpressionPrinter::print(&E, s.AsRawOstream(),
+ llvm::DIDumpOptions(), nullptr);
} else
s.PutCString("dwarf-expr");
}
diff --git a/lldb/unittests/Symbol/PostfixExpressionTest.cpp b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
index d56df476ebaab..1e437da5133d9 100644
--- a/lldb/unittests/Symbol/PostfixExpressionTest.cpp
+++ b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
@@ -159,8 +159,8 @@ static std::string ParseAndGenerateDWARF(llvm::StringRef expr) {
std::string result;
llvm::raw_string_ostream os(result);
- llvm::DWARFExpression(extractor, addr_size, llvm::dwarf::DWARF32)
- .print(os, llvm::DIDumpOptions(), nullptr);
+ llvm::DWARFExpression E(extractor, addr_size, llvm::dwarf::DWARF32);
+ llvm::DWARFExpressionPrinter::print(&E, os, llvm::DIDumpOptions(), nullptr);
return result;
}
diff --git a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
index efb8f720b56e1..d746e04f8a9fc 100644
--- a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -39,8 +39,8 @@ CheckValidProgramTranslation(llvm::StringRef fpo_program,
std::string result;
llvm::raw_string_ostream os(result);
- llvm::DWARFExpression(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32)
- .print(os, llvm::DIDumpOptions(), nullptr);
+ llvm::DWARFExpression E(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32);
+ llvm::DWARFExpressionPrinter::print(&E, os, llvm::DIDumpOptions(), nullptr);
// actual check
ASSERT_EQ(expected_dwarf_expression, result);
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
index 00228a32173f1..ecfb545c85798 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
@@ -76,6 +76,9 @@ class DWARFExpression {
private:
friend class DWARFExpression::iterator;
+ friend class DWARFExpressionPrinter;
+ friend class DWARFVerifier;
+
uint8_t Opcode; ///< The Op Opcode, DW_OP_<something>.
Description Desc;
bool Error = false;
@@ -98,11 +101,6 @@ class DWARFExpression {
}
uint64_t getEndOffset() const { return EndOffset; }
bool isError() const { return Error; }
- bool print(raw_ostream &OS, DIDumpOptions DumpOpts,
- const DWARFExpression *Expr, DWARFUnit *U) const;
-
- /// Verify \p Op. Does not affect the return of \a isError().
- static bool verify(const Operation &Op, DWARFUnit *U);
private:
bool extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset,
@@ -152,26 +150,12 @@ class DWARFExpression {
iterator begin() const { return iterator(this, 0); }
iterator end() const { return iterator(this, Data.getData().size()); }
- void print(raw_ostream &OS, DIDumpOptions DumpOpts, DWARFUnit *U,
- bool IsEH = false) const;
-
- /// Print the expression in a format intended to be compact and useful to a
- /// user, but not perfectly unambiguous, or capable of representing every
- /// valid DWARF expression. Returns true if the expression was sucessfully
- /// printed.
- bool printCompact(raw_ostream &OS,
- std::function<StringRef(uint64_t RegNum, bool IsEH)>
- GetNameForDWARFReg = nullptr);
-
- bool verify(DWARFUnit *U);
-
bool operator==(const DWARFExpression &RHS) const;
StringRef getData() const { return Data.getData(); }
- static bool prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
- DIDumpOptions DumpOpts, uint8_t Opcode,
- const ArrayRef<uint64_t> Operands);
+ friend class DWARFExpressionPrinter;
+ friend class DWARFVerifier;
private:
DataExtractor Data;
@@ -183,5 +167,63 @@ inline bool operator==(const DWARFExpression::iterator &LHS,
const DWARFExpression::iterator &RHS) {
return LHS.Expr == RHS.Expr && LHS.Offset == RHS.Offset;
}
-}
-#endif
+
+// This functionality is separated from the main data structure so that nothing
+// in DWARFExpression.cpp needs build-time dependencies on DWARFUnit or other
+// higher-level Dwarf structures. This approach creates better layering and
+// allows DWARFExpression to be used from code which can't have dependencies on
+// those higher-level structures.
+
+class DWARFUnit;
+struct DIDumpOptions;
+class raw_ostream;
+
+class DWARFExpressionPrinter {
+public:
+ /// Print a Dwarf expression/
+ /// \param E to be printed
+ /// \param OS to this stream
+ /// \param GetNameForDWARFReg callback to return dwarf register name
+ static void print(const DWARFExpression *E, raw_ostream &OS,
+ DIDumpOptions DumpOpts, DWARFUnit *U, bool IsEH = false);
+
+ /// Print the expression in a format intended to be compact and useful to a
+ /// user, but not perfectly unambiguous, or capable of representing every
+ /// valid DWARF expression. Returns true if the expression was sucessfully
+ /// printed.
+ ///
+ /// \param E to be printed
+ /// \param OS to this stream
+ /// \param GetNameForDWARFReg callback to return dwarf register name
+ ///
+ /// \returns true if the expression was successfully printed
+ static bool printCompact(const DWARFExpression *E, raw_ostream &OS,
+ std::function<StringRef(uint64_t RegNum, bool IsEH)>
+ GetNameForDWARFReg = nullptr);
+
+ /// Pretty print a register opcode and operands.
+ /// \param U within the context of this Dwarf unit, if any.
+ /// \param OS to this stream
+ /// \param DumpOpts with these options
+ /// \param Opcode to print
+ /// \param Operands to the opcode
+ ///
+ /// returns true if the Op was successfully printed
+ static bool prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
+ DIDumpOptions DumpOpts, uint8_t Opcode,
+ ArrayRef<uint64_t> Operands);
+
+private:
+ static bool printOp(const DWARFExpression::Operation *Op, raw_ostream &OS,
+ DIDumpOptions DumpOpts, const DWARFExpression *Expr,
+ DWARFUnit *U);
+
+ static void prettyPrintBaseTypeRef(DWARFUnit *U, raw_ostream &OS,
+ DIDumpOptions DumpOpts,
+ ArrayRef<uint64_t> Operands,
+ unsigned Operand);
+};
+
+} // end namespace llvm
+
+#endif // LLVM_DEBUGINFO_DWARF_DWARFEXPRESSION_H
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index 717f9cedc4ee3..7c998a623769a 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -14,6 +14,7 @@
#include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
#include "llvm/DebugInfo/DWARF/DWARFDie.h"
+#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
#include <cstdint>
#include <map>
@@ -319,6 +320,23 @@ class DWARFVerifier {
void verifyDebugNames(const DWARFSection &AccelSection,
const DataExtractor &StrData);
+ /// Verify that the the expression is valid within the context of unit U.
+ ///
+ /// \param E expression to verify.
+ /// \param U containing DWARFUnit, if any.
+ ///
+ /// returns true if E is a valid expression.
+ bool verifyExpression(const DWARFExpression &E, DWARFUnit *U);
+
+ /// Verify that the the expression operation is valid within the context of
+ /// unit U.
+ ///
+ /// \param Op operation to verify
+ /// \param U containing DWARFUnit, if any
+ ///
+ /// returns true if Op is a valid Dwarf operation
+ bool verifyExpressionOp(const DWARFExpression::Operation &Op, DWARFUnit *U);
+
public:
DWARFVerifier(raw_ostream &S, DWARFContext &D,
DIDumpOptions DumpOpts = DIDumpOptions::getForSingleDIE());
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp b/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp
index 1a4fc4930fdd9..d7e8f125d5cef 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp
@@ -425,7 +425,7 @@ void CFIProgram::printOperand(raw_ostream &OS, DIDumpOptions DumpOpts,
case OT_Expression:
assert(Instr.Expression && "missing DWARFExpression object");
OS << " ";
- Instr.Expression->print(OS, DumpOpts, nullptr);
+ DWARFExpressionPrinter::print(&(*Instr.Expression), OS, DumpOpts, nullptr);
break;
}
}
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index aecfc4565dbc2..c46b14b4446f7 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -14,6 +14,7 @@
#include "llvm/DebugInfo/DIContext.h"
#include "llvm/DebugInfo/DWARF/DWARFCFIProgram.h"
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
+#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/DataExtractor.h"
#include "llvm/Support/Errc.h"
@@ -110,7 +111,8 @@ void UnwindLocation::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
OS << " in addrspace" << *AddrSpace;
break;
case DWARFExpr: {
- Expr->print(OS, DumpOpts, nullptr);
+ if (Expr)
+ DWARFExpressionPrinter::print(&(*Expr), OS, DumpOpts, nullptr);
break;
}
case Constant:
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
index ec7af792efb06..fc71be32fdd79 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -116,7 +116,8 @@ static void dumpExpression(raw_ostream &OS, DIDumpOptions DumpOpts,
std::optional<dwarf::DwarfFormat> Format;
if (U)
Format = U->getFormat();
- DWARFExpression(Extractor, AddressSize, Format).print(OS, DumpOpts, U);
+ DWARFExpression E(Extractor, AddressSize, Format);
+ DWARFExpressionPrinter::print(&E, OS, DumpOpts, U);
}
bool DWARFLocationTable::dumpLocationList(
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index a0ce7810f91b0..08dd9d30812d1 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -98,8 +98,8 @@ static void dumpLocationExpr(raw_ostream &OS, const DWARFFormValue &FormValue,
ArrayRef<uint8_t> Expr = *FormValue.getAsBlock();
DataExtractor Data(StringRef((const char *)Expr.data(), Expr.size()),
Ctx.isLittleEndian(), 0);
- DWARFExpression(Data, U->getAddressByteSize(), U->getFormParams().Format)
- .print(OS, DumpOpts, U);
+ DWARFExpression DE(Data, U->getAddressByteSize(), U->getFormParams().Format);
+ DWARFExpressionPrinter::print(&DE, OS, DumpOpts, U);
}
static DWARFDie resolveReferencedType(DWARFDie D, DWARFFormValue F) {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
index 2ae5ff3efc8c5..6dda918bd0d17 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
@@ -237,10 +237,23 @@ bool DWARFExpression::Operation::extract(DataExtractor Data,
return true;
}
-static void prettyPrintBaseTypeRef(DWARFUnit *U, raw_ostream &OS,
- DIDumpOptions DumpOpts,
- ArrayRef<uint64_t> Operands,
- unsigned Operand) {
+std::optional<unsigned> DWARFExpression::Operation::getSubCode() const {
+ if (!Desc.Op.size() || Desc.Op[0] != Operation::SizeSubOpLEB)
+ return std::nullopt;
+ return Operands[0];
+}
+
+bool DWARFExpression::operator==(const DWARFExpression &RHS) const {
+ if (AddressSize != RHS.AddressSize || Format != RHS.Format)
+ return false;
+ return Data.getData() == RHS.Data.getData();
+}
+
+void DWARFExpressionPrinter::prettyPrintBaseTypeRef(DWARFUnit *U,
+ raw_ostream &OS,
+ DIDumpOptions DumpOpts,
+ ArrayRef<uint64_t> Operands,
+ unsigned Operand) {
assert(Operand < Operands.size() && "operand out of bounds");
if (!U) {
OS << format(" <base_type ref: 0x%" PRIx64 ">", Operands[Operand]);
@@ -259,10 +272,9 @@ static void prettyPrintBaseTypeRef(DWARFUnit *U, raw_ostream &OS,
}
}
-bool DWARFExpression::prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
- DIDumpOptions DumpOpts,
- uint8_t Opcode,
- ArrayRef<uint64_t> Operands) {
+bool DWARFExpressionPrinter::prettyPrintRegisterOp(
+ DWARFUnit *U, raw_ostream &OS, DIDumpOptions DumpOpts, uint8_t Opcode,
+ ArrayRef<uint64_t> Operands) {
if (!DumpOpts.GetNameForDWARFReg)
return false;
@@ -293,87 +305,84 @@ bool DWARFExpression::prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
return false;
}
-std::optional<unsigned> DWARFExpression::Operation::getSubCode() const {
- if (!Desc.Op.size() || Desc.Op[0] != Operation::SizeSubOpLEB)
- return std::nullopt;
- return Operands[0];
-}
-
-bool DWARFExpression::Operation::print(raw_ostream &OS, DIDumpOptions DumpOpts,
- const DWARFExpression *Expr,
- DWARFUnit *U) const {
- if (Error) {
+bool DWARFExpressionPrinter::printOp(const DWARFExpression::Operation *Op,
+ raw_ostream &OS, DIDumpOptions DumpOpts,
+ const DWARFExpression *Expr,
+ DWARFUnit *U) {
+ if (Op->Error) {
OS << "<decoding error>";
return false;
}
- StringRef Name = OperationEncodingString(Opcode);
+ StringRef Name = OperationEncodingString(Op->Opcode);
assert(!Name.empty() && "DW_OP has no name!");
OS << Name;
- if ((Opcode >= DW_OP_breg0 && Opcode <= DW_OP_breg31) ||
- (Opcode >= DW_OP_reg0 && Opcode <= DW_OP_reg31) ||
- Opcode == DW_OP_bregx || Opcode == DW_OP_regx ||
- Opcode == DW_OP_regval_type)
- if (prettyPrintRegisterOp(U, OS, DumpOpts, Opcode, Operands))
+ if ((Op->Opcode >= DW_OP_breg0 && Op->Opcode <= DW_OP_breg31) ||
+ (Op->Opcode >= DW_OP_reg0 && Op->Opcode <= DW_OP_reg31) ||
+ Op->Opcode == DW_OP_bregx || Op->Opcode == DW_OP_regx ||
+ Op->Opcode == DW_OP_regval_type)
+ if (prettyPrintRegisterOp(U, OS, DumpOpts, Op->Opcode, Op->Operands))
return true;
- for (unsigned Operand = 0; Operand < Desc.Op.size(); ++Operand) {
- unsigned Size = Desc.Op[Operand];
- unsigned Signed = Size & Operation::SignBit;
+ for (unsigned Operand = 0; Operand < Op->Desc.Op.size(); ++Operand) {
+ unsigned Size = Op->Desc.Op[Operand];
+ unsigned Signed = Size & DWARFExpression::Operation::SignBit;
- if (Size == Operation::SizeSubOpLEB) {
- StringRef SubName = SubOperationEncodingString(Opcode, Operands[Operand]);
+ if (Size == DWARFExpression::Operation::SizeSubOpLEB) {
+ StringRef SubName =
+ SubOperationEncodingString(Op->Opcode, Op->Operands[Operand]);
assert(!SubName.empty() && "DW_OP SubOp has no name!");
OS << " " << SubName;
- } else if (Size == Operation::BaseTypeRef && U) {
+ } else if (Size == DWARFExpression::Operation::BaseTypeRef && U) {
// For DW_OP_convert the operand may be 0 to indicate that conversion to
// the generic type should be done. The same holds for DW_OP_reinterpret,
// which is currently not supported.
- if (Opcode == DW_OP_convert && Operands[Operand] == 0)
+ if (Op->Opcode == DW_OP_convert && Op->Operands[Operand] == 0)
OS << " 0x0";
else
- prettyPrintBaseTypeRef(U, OS, DumpOpts, Operands, Operand);
- } else if (Size == Operation::WasmLocationArg) {
+ prettyPrintBaseTypeRef(U, OS, DumpOpts, Op->Operands, Operand);
+ } else if (Size == DWARFExpression::Operation::WasmLocationArg) {
assert(Operand == 1);
- switch (Operands[0]) {
+ switch (Op->Operands[0]) {
case 0:
case 1:
case 2:
case 3: // global as uint32
case 4:
- OS << format(" 0x%" PRIx64, Operands[Operand]);
+ OS << format(" 0x%" PRIx64, Op->Operands[Operand]);
break;
default: assert(false);
}
- } else if (Size == Operation::SizeBlock) {
- uint64_t Offset = Operands[Operand];
- for (unsigned i = 0; i < Operands[Operand - 1]; ++i)
+ } else if (Size == DWARFExpression::Operation::SizeBlock) {
+ uint64_t Offset = Op->Operands[Operand];
+ for (unsigned i = 0; i < Op->Operands[Operand - 1]; ++i)
OS << format(" 0x%02x", Expr->Data.getU8(&Offset));
} else {
if (Signed)
- OS << format(" %+" PRId64, (int64_t)Operands[Operand]);
- else if (Opcode != DW_OP_entry_value &&
- Opcode != DW_OP_GNU_entry_value)
- OS << format(" 0x%" PRIx64, Operands[Operand]);
+ OS << format(" %+" PRId64, (int64_t)Op->Operands[Operand]);
+ else if (Op->Opcode != DW_OP_entry_value &&
+ Op->Opcode != DW_OP_GNU_entry_value)
+ OS << format(" 0x%" PRIx64, Op->Operands[Operand]);
}
}
return true;
}
-void DWARFExpression::print(raw_ostream &OS, DIDumpOptions DumpOpts,
- DWARFUnit *U, bool IsEH) const {
+void DWARFExpressionPrinter::print(const DWARFExpression *E, raw_ostream &OS,
+ DIDumpOptions DumpOpts, DWARFUnit *U,
+ bool IsEH) {
uint32_t EntryValExprSize = 0;
uint64_t EntryValStartOffset = 0;
- if (Data.getData().empty())
+ if (E->Data.getData().empty())
OS << "<empty>";
- for (auto &Op : *this) {
+ for (auto &Op : *E) {
DumpOpts.IsEH = IsEH;
- if (!Op.print(OS, DumpOpts, this, U)) {
+ if (!printOp(&Op, OS, DumpOpts, E, U)) {
uint64_t FailOffset = Op.getEndOffset();
- while (FailOffset < Data.getData().size())
- OS << format(" %02x", Data.getU8(&FailOffset));
+ while (FailOffset < E->Data.getData().size())
+ OS << format(" %02x", E->Data.getU8(&FailOffset));
return;
}
@@ -391,39 +400,11 @@ void DWARFExpression::print(raw_ostream &OS, DIDumpOptions DumpOpts,
OS << ")";
}
- if (Op.getEndOffset() < Data.getData().size())
+ if (Op.getEndOffset() < E->Data.getData().size())
OS <<...
[truncated]
|
|
@llvm/pr-subscribers-debuginfo Author: None (Sterling-Augustine) Changes(Revised version of a previous, unreviewed, PR.) Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class. This is one in a series of refactoring PRs to separate dwarf functionality into lower-level pieces usable without object files and sections at build time. The code is already written this way via various "if (section == nullptr)" and similar conditionals. So the functionality itself is needed and exists, but only as a runtime feature. The goal of these refactors is to remove the build-time dependencies, which will allow the existing functionality to be used from lower-level parts of the compiler. Particularly from lib/MC/.... More information at: Patch is 25.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143170.diff 15 Files Affected:
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 4b2b111e08e6d..09d8bd833d543 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -80,8 +80,8 @@ void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
};
llvm::DIDumpOptions DumpOpts;
DumpOpts.GetNameForDWARFReg = GetRegName;
- llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
- .print(s->AsRawOstream(), DumpOpts, nullptr);
+ llvm::DWARFExpression E(m_data.GetAsLLVM(), m_data.GetAddressByteSize());
+ llvm::DWARFExpressionPrinter::print(&E, s->AsRawOstream(), DumpOpts, nullptr);
}
RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }
diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp
index b1a96b5e26840..9ccbd4064900f 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -87,8 +87,9 @@ static void DumpDWARFExpr(Stream &s, llvm::ArrayRef<uint8_t> expr, Thread *threa
if (auto order_and_width = GetByteOrderAndAddrSize(thread)) {
llvm::DataExtractor data(expr, order_and_width->first == eByteOrderLittle,
order_and_width->second);
- llvm::DWARFExpression(data, order_and_width->second, llvm::dwarf::DWARF32)
- .print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr);
+ llvm::DWARFExpression E(data, order_and_width->second, llvm::dwarf::DWARF32);
+ llvm::DWARFExpressionPrinter::print(&E, s.AsRawOstream(),
+ llvm::DIDumpOptions(), nullptr);
} else
s.PutCString("dwarf-expr");
}
diff --git a/lldb/unittests/Symbol/PostfixExpressionTest.cpp b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
index d56df476ebaab..1e437da5133d9 100644
--- a/lldb/unittests/Symbol/PostfixExpressionTest.cpp
+++ b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
@@ -159,8 +159,8 @@ static std::string ParseAndGenerateDWARF(llvm::StringRef expr) {
std::string result;
llvm::raw_string_ostream os(result);
- llvm::DWARFExpression(extractor, addr_size, llvm::dwarf::DWARF32)
- .print(os, llvm::DIDumpOptions(), nullptr);
+ llvm::DWARFExpression E(extractor, addr_size, llvm::dwarf::DWARF32);
+ llvm::DWARFExpressionPrinter::print(&E, os, llvm::DIDumpOptions(), nullptr);
return result;
}
diff --git a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
index efb8f720b56e1..d746e04f8a9fc 100644
--- a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -39,8 +39,8 @@ CheckValidProgramTranslation(llvm::StringRef fpo_program,
std::string result;
llvm::raw_string_ostream os(result);
- llvm::DWARFExpression(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32)
- .print(os, llvm::DIDumpOptions(), nullptr);
+ llvm::DWARFExpression E(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32);
+ llvm::DWARFExpressionPrinter::print(&E, os, llvm::DIDumpOptions(), nullptr);
// actual check
ASSERT_EQ(expected_dwarf_expression, result);
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
index 00228a32173f1..ecfb545c85798 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
@@ -76,6 +76,9 @@ class DWARFExpression {
private:
friend class DWARFExpression::iterator;
+ friend class DWARFExpressionPrinter;
+ friend class DWARFVerifier;
+
uint8_t Opcode; ///< The Op Opcode, DW_OP_<something>.
Description Desc;
bool Error = false;
@@ -98,11 +101,6 @@ class DWARFExpression {
}
uint64_t getEndOffset() const { return EndOffset; }
bool isError() const { return Error; }
- bool print(raw_ostream &OS, DIDumpOptions DumpOpts,
- const DWARFExpression *Expr, DWARFUnit *U) const;
-
- /// Verify \p Op. Does not affect the return of \a isError().
- static bool verify(const Operation &Op, DWARFUnit *U);
private:
bool extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset,
@@ -152,26 +150,12 @@ class DWARFExpression {
iterator begin() const { return iterator(this, 0); }
iterator end() const { return iterator(this, Data.getData().size()); }
- void print(raw_ostream &OS, DIDumpOptions DumpOpts, DWARFUnit *U,
- bool IsEH = false) const;
-
- /// Print the expression in a format intended to be compact and useful to a
- /// user, but not perfectly unambiguous, or capable of representing every
- /// valid DWARF expression. Returns true if the expression was sucessfully
- /// printed.
- bool printCompact(raw_ostream &OS,
- std::function<StringRef(uint64_t RegNum, bool IsEH)>
- GetNameForDWARFReg = nullptr);
-
- bool verify(DWARFUnit *U);
-
bool operator==(const DWARFExpression &RHS) const;
StringRef getData() const { return Data.getData(); }
- static bool prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
- DIDumpOptions DumpOpts, uint8_t Opcode,
- const ArrayRef<uint64_t> Operands);
+ friend class DWARFExpressionPrinter;
+ friend class DWARFVerifier;
private:
DataExtractor Data;
@@ -183,5 +167,63 @@ inline bool operator==(const DWARFExpression::iterator &LHS,
const DWARFExpression::iterator &RHS) {
return LHS.Expr == RHS.Expr && LHS.Offset == RHS.Offset;
}
-}
-#endif
+
+// This functionality is separated from the main data structure so that nothing
+// in DWARFExpression.cpp needs build-time dependencies on DWARFUnit or other
+// higher-level Dwarf structures. This approach creates better layering and
+// allows DWARFExpression to be used from code which can't have dependencies on
+// those higher-level structures.
+
+class DWARFUnit;
+struct DIDumpOptions;
+class raw_ostream;
+
+class DWARFExpressionPrinter {
+public:
+ /// Print a Dwarf expression/
+ /// \param E to be printed
+ /// \param OS to this stream
+ /// \param GetNameForDWARFReg callback to return dwarf register name
+ static void print(const DWARFExpression *E, raw_ostream &OS,
+ DIDumpOptions DumpOpts, DWARFUnit *U, bool IsEH = false);
+
+ /// Print the expression in a format intended to be compact and useful to a
+ /// user, but not perfectly unambiguous, or capable of representing every
+ /// valid DWARF expression. Returns true if the expression was sucessfully
+ /// printed.
+ ///
+ /// \param E to be printed
+ /// \param OS to this stream
+ /// \param GetNameForDWARFReg callback to return dwarf register name
+ ///
+ /// \returns true if the expression was successfully printed
+ static bool printCompact(const DWARFExpression *E, raw_ostream &OS,
+ std::function<StringRef(uint64_t RegNum, bool IsEH)>
+ GetNameForDWARFReg = nullptr);
+
+ /// Pretty print a register opcode and operands.
+ /// \param U within the context of this Dwarf unit, if any.
+ /// \param OS to this stream
+ /// \param DumpOpts with these options
+ /// \param Opcode to print
+ /// \param Operands to the opcode
+ ///
+ /// returns true if the Op was successfully printed
+ static bool prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
+ DIDumpOptions DumpOpts, uint8_t Opcode,
+ ArrayRef<uint64_t> Operands);
+
+private:
+ static bool printOp(const DWARFExpression::Operation *Op, raw_ostream &OS,
+ DIDumpOptions DumpOpts, const DWARFExpression *Expr,
+ DWARFUnit *U);
+
+ static void prettyPrintBaseTypeRef(DWARFUnit *U, raw_ostream &OS,
+ DIDumpOptions DumpOpts,
+ ArrayRef<uint64_t> Operands,
+ unsigned Operand);
+};
+
+} // end namespace llvm
+
+#endif // LLVM_DEBUGINFO_DWARF_DWARFEXPRESSION_H
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index 717f9cedc4ee3..7c998a623769a 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -14,6 +14,7 @@
#include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
#include "llvm/DebugInfo/DWARF/DWARFDie.h"
+#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
#include <cstdint>
#include <map>
@@ -319,6 +320,23 @@ class DWARFVerifier {
void verifyDebugNames(const DWARFSection &AccelSection,
const DataExtractor &StrData);
+ /// Verify that the the expression is valid within the context of unit U.
+ ///
+ /// \param E expression to verify.
+ /// \param U containing DWARFUnit, if any.
+ ///
+ /// returns true if E is a valid expression.
+ bool verifyExpression(const DWARFExpression &E, DWARFUnit *U);
+
+ /// Verify that the the expression operation is valid within the context of
+ /// unit U.
+ ///
+ /// \param Op operation to verify
+ /// \param U containing DWARFUnit, if any
+ ///
+ /// returns true if Op is a valid Dwarf operation
+ bool verifyExpressionOp(const DWARFExpression::Operation &Op, DWARFUnit *U);
+
public:
DWARFVerifier(raw_ostream &S, DWARFContext &D,
DIDumpOptions DumpOpts = DIDumpOptions::getForSingleDIE());
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp b/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp
index 1a4fc4930fdd9..d7e8f125d5cef 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp
@@ -425,7 +425,7 @@ void CFIProgram::printOperand(raw_ostream &OS, DIDumpOptions DumpOpts,
case OT_Expression:
assert(Instr.Expression && "missing DWARFExpression object");
OS << " ";
- Instr.Expression->print(OS, DumpOpts, nullptr);
+ DWARFExpressionPrinter::print(&(*Instr.Expression), OS, DumpOpts, nullptr);
break;
}
}
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index aecfc4565dbc2..c46b14b4446f7 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -14,6 +14,7 @@
#include "llvm/DebugInfo/DIContext.h"
#include "llvm/DebugInfo/DWARF/DWARFCFIProgram.h"
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
+#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/DataExtractor.h"
#include "llvm/Support/Errc.h"
@@ -110,7 +111,8 @@ void UnwindLocation::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
OS << " in addrspace" << *AddrSpace;
break;
case DWARFExpr: {
- Expr->print(OS, DumpOpts, nullptr);
+ if (Expr)
+ DWARFExpressionPrinter::print(&(*Expr), OS, DumpOpts, nullptr);
break;
}
case Constant:
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
index ec7af792efb06..fc71be32fdd79 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -116,7 +116,8 @@ static void dumpExpression(raw_ostream &OS, DIDumpOptions DumpOpts,
std::optional<dwarf::DwarfFormat> Format;
if (U)
Format = U->getFormat();
- DWARFExpression(Extractor, AddressSize, Format).print(OS, DumpOpts, U);
+ DWARFExpression E(Extractor, AddressSize, Format);
+ DWARFExpressionPrinter::print(&E, OS, DumpOpts, U);
}
bool DWARFLocationTable::dumpLocationList(
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index a0ce7810f91b0..08dd9d30812d1 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -98,8 +98,8 @@ static void dumpLocationExpr(raw_ostream &OS, const DWARFFormValue &FormValue,
ArrayRef<uint8_t> Expr = *FormValue.getAsBlock();
DataExtractor Data(StringRef((const char *)Expr.data(), Expr.size()),
Ctx.isLittleEndian(), 0);
- DWARFExpression(Data, U->getAddressByteSize(), U->getFormParams().Format)
- .print(OS, DumpOpts, U);
+ DWARFExpression DE(Data, U->getAddressByteSize(), U->getFormParams().Format);
+ DWARFExpressionPrinter::print(&DE, OS, DumpOpts, U);
}
static DWARFDie resolveReferencedType(DWARFDie D, DWARFFormValue F) {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
index 2ae5ff3efc8c5..6dda918bd0d17 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
@@ -237,10 +237,23 @@ bool DWARFExpression::Operation::extract(DataExtractor Data,
return true;
}
-static void prettyPrintBaseTypeRef(DWARFUnit *U, raw_ostream &OS,
- DIDumpOptions DumpOpts,
- ArrayRef<uint64_t> Operands,
- unsigned Operand) {
+std::optional<unsigned> DWARFExpression::Operation::getSubCode() const {
+ if (!Desc.Op.size() || Desc.Op[0] != Operation::SizeSubOpLEB)
+ return std::nullopt;
+ return Operands[0];
+}
+
+bool DWARFExpression::operator==(const DWARFExpression &RHS) const {
+ if (AddressSize != RHS.AddressSize || Format != RHS.Format)
+ return false;
+ return Data.getData() == RHS.Data.getData();
+}
+
+void DWARFExpressionPrinter::prettyPrintBaseTypeRef(DWARFUnit *U,
+ raw_ostream &OS,
+ DIDumpOptions DumpOpts,
+ ArrayRef<uint64_t> Operands,
+ unsigned Operand) {
assert(Operand < Operands.size() && "operand out of bounds");
if (!U) {
OS << format(" <base_type ref: 0x%" PRIx64 ">", Operands[Operand]);
@@ -259,10 +272,9 @@ static void prettyPrintBaseTypeRef(DWARFUnit *U, raw_ostream &OS,
}
}
-bool DWARFExpression::prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
- DIDumpOptions DumpOpts,
- uint8_t Opcode,
- ArrayRef<uint64_t> Operands) {
+bool DWARFExpressionPrinter::prettyPrintRegisterOp(
+ DWARFUnit *U, raw_ostream &OS, DIDumpOptions DumpOpts, uint8_t Opcode,
+ ArrayRef<uint64_t> Operands) {
if (!DumpOpts.GetNameForDWARFReg)
return false;
@@ -293,87 +305,84 @@ bool DWARFExpression::prettyPrintRegisterOp(DWARFUnit *U, raw_ostream &OS,
return false;
}
-std::optional<unsigned> DWARFExpression::Operation::getSubCode() const {
- if (!Desc.Op.size() || Desc.Op[0] != Operation::SizeSubOpLEB)
- return std::nullopt;
- return Operands[0];
-}
-
-bool DWARFExpression::Operation::print(raw_ostream &OS, DIDumpOptions DumpOpts,
- const DWARFExpression *Expr,
- DWARFUnit *U) const {
- if (Error) {
+bool DWARFExpressionPrinter::printOp(const DWARFExpression::Operation *Op,
+ raw_ostream &OS, DIDumpOptions DumpOpts,
+ const DWARFExpression *Expr,
+ DWARFUnit *U) {
+ if (Op->Error) {
OS << "<decoding error>";
return false;
}
- StringRef Name = OperationEncodingString(Opcode);
+ StringRef Name = OperationEncodingString(Op->Opcode);
assert(!Name.empty() && "DW_OP has no name!");
OS << Name;
- if ((Opcode >= DW_OP_breg0 && Opcode <= DW_OP_breg31) ||
- (Opcode >= DW_OP_reg0 && Opcode <= DW_OP_reg31) ||
- Opcode == DW_OP_bregx || Opcode == DW_OP_regx ||
- Opcode == DW_OP_regval_type)
- if (prettyPrintRegisterOp(U, OS, DumpOpts, Opcode, Operands))
+ if ((Op->Opcode >= DW_OP_breg0 && Op->Opcode <= DW_OP_breg31) ||
+ (Op->Opcode >= DW_OP_reg0 && Op->Opcode <= DW_OP_reg31) ||
+ Op->Opcode == DW_OP_bregx || Op->Opcode == DW_OP_regx ||
+ Op->Opcode == DW_OP_regval_type)
+ if (prettyPrintRegisterOp(U, OS, DumpOpts, Op->Opcode, Op->Operands))
return true;
- for (unsigned Operand = 0; Operand < Desc.Op.size(); ++Operand) {
- unsigned Size = Desc.Op[Operand];
- unsigned Signed = Size & Operation::SignBit;
+ for (unsigned Operand = 0; Operand < Op->Desc.Op.size(); ++Operand) {
+ unsigned Size = Op->Desc.Op[Operand];
+ unsigned Signed = Size & DWARFExpression::Operation::SignBit;
- if (Size == Operation::SizeSubOpLEB) {
- StringRef SubName = SubOperationEncodingString(Opcode, Operands[Operand]);
+ if (Size == DWARFExpression::Operation::SizeSubOpLEB) {
+ StringRef SubName =
+ SubOperationEncodingString(Op->Opcode, Op->Operands[Operand]);
assert(!SubName.empty() && "DW_OP SubOp has no name!");
OS << " " << SubName;
- } else if (Size == Operation::BaseTypeRef && U) {
+ } else if (Size == DWARFExpression::Operation::BaseTypeRef && U) {
// For DW_OP_convert the operand may be 0 to indicate that conversion to
// the generic type should be done. The same holds for DW_OP_reinterpret,
// which is currently not supported.
- if (Opcode == DW_OP_convert && Operands[Operand] == 0)
+ if (Op->Opcode == DW_OP_convert && Op->Operands[Operand] == 0)
OS << " 0x0";
else
- prettyPrintBaseTypeRef(U, OS, DumpOpts, Operands, Operand);
- } else if (Size == Operation::WasmLocationArg) {
+ prettyPrintBaseTypeRef(U, OS, DumpOpts, Op->Operands, Operand);
+ } else if (Size == DWARFExpression::Operation::WasmLocationArg) {
assert(Operand == 1);
- switch (Operands[0]) {
+ switch (Op->Operands[0]) {
case 0:
case 1:
case 2:
case 3: // global as uint32
case 4:
- OS << format(" 0x%" PRIx64, Operands[Operand]);
+ OS << format(" 0x%" PRIx64, Op->Operands[Operand]);
break;
default: assert(false);
}
- } else if (Size == Operation::SizeBlock) {
- uint64_t Offset = Operands[Operand];
- for (unsigned i = 0; i < Operands[Operand - 1]; ++i)
+ } else if (Size == DWARFExpression::Operation::SizeBlock) {
+ uint64_t Offset = Op->Operands[Operand];
+ for (unsigned i = 0; i < Op->Operands[Operand - 1]; ++i)
OS << format(" 0x%02x", Expr->Data.getU8(&Offset));
} else {
if (Signed)
- OS << format(" %+" PRId64, (int64_t)Operands[Operand]);
- else if (Opcode != DW_OP_entry_value &&
- Opcode != DW_OP_GNU_entry_value)
- OS << format(" 0x%" PRIx64, Operands[Operand]);
+ OS << format(" %+" PRId64, (int64_t)Op->Operands[Operand]);
+ else if (Op->Opcode != DW_OP_entry_value &&
+ Op->Opcode != DW_OP_GNU_entry_value)
+ OS << format(" 0x%" PRIx64, Op->Operands[Operand]);
}
}
return true;
}
-void DWARFExpression::print(raw_ostream &OS, DIDumpOptions DumpOpts,
- DWARFUnit *U, bool IsEH) const {
+void DWARFExpressionPrinter::print(const DWARFExpression *E, raw_ostream &OS,
+ DIDumpOptions DumpOpts, DWARFUnit *U,
+ bool IsEH) {
uint32_t EntryValExprSize = 0;
uint64_t EntryValStartOffset = 0;
- if (Data.getData().empty())
+ if (E->Data.getData().empty())
OS << "<empty>";
- for (auto &Op : *this) {
+ for (auto &Op : *E) {
DumpOpts.IsEH = IsEH;
- if (!Op.print(OS, DumpOpts, this, U)) {
+ if (!printOp(&Op, OS, DumpOpts, E, U)) {
uint64_t FailOffset = Op.getEndOffset();
- while (FailOffset < Data.getData().size())
- OS << format(" %02x", Data.getU8(&FailOffset));
+ while (FailOffset < E->Data.getData().size())
+ OS << format(" %02x", E->Data.getU8(&FailOffset));
return;
}
@@ -391,39 +400,11 @@ void DWARFExpression::print(raw_ostream &OS, DIDumpOptions DumpOpts,
OS << ")";
}
- if (Op.getEndOffset() < Data.getData().size())
+ if (Op.getEndOffset() < E->Data.getData().size())
OS <<...
[truncated]
|
Sterling-Augustine
requested review from
mbucko,
labath,
jmorse and
SLTozer
and removed request for
JDevlieghere
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
dwblaikie
approved these changes
Jun 9, 2025
| @@ -425,7 +425,7 @@ void CFIProgram::printOperand(raw_ostream &OS, DIDumpOptions DumpOpts, | |||
| case OT_Expression: | |||
| assert(Instr.Expression && "missing DWARFExpression object"); | |||
| OS << " "; | |||
| Instr.Expression->print(OS, DumpOpts, nullptr); | |||
| DWARFExpressionPrinter::print(&(*Instr.Expression), OS, DumpOpts, nullptr); | |||
There was a problem hiding this comment.
No need for the parents here? Could use .get instead of &* , and wouldn't need the null check in the next case
There was a problem hiding this comment.
Thanks. Done.
There was a problem hiding this comment.
Hmm, something got confused here - because it ended up as &Instr.Expression.value() - I had meant Instr.Expression.get() - did that not work/have some issues?
There was a problem hiding this comment.
std::optional does not have get(), only value().
There was a problem hiding this comment.
Ahh, right, std::optional when I was thinking of std::unique_ptr's API. Sorry about that. FWIW in that case I don't mind either way.
rorth
pushed a commit
to rorth/llvm-project
that referenced
this pull request
Jun 11, 2025
…sed) (llvm#143170) (Revised version of a previous, unreviewed, PR.) Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class. This is one in a series of refactoring PRs to separate dwarf functionality into lower-level pieces usable without object files and sections at build time. The code is already written this way via various "if (section == nullptr)" and similar conditionals. So the functionality itself is needed and exists, but only as a runtime feature. The goal of these refactors is to remove the build-time dependencies, which will allow the existing functionality to be used from lower-level parts of the compiler. Particularly from lib/MC/.... More information at: https://discourse.llvm.org/t/rfc-debuginfo-dwarf-refactor-into-to-lower-and-higher-level-libraries/86665
DhruvSrivastavaX
pushed a commit
to DhruvSrivastavaX/lldb-for-aix
that referenced
this pull request
Jun 12, 2025
…sed) (llvm#143170) (Revised version of a previous, unreviewed, PR.) Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class. This is one in a series of refactoring PRs to separate dwarf functionality into lower-level pieces usable without object files and sections at build time. The code is already written this way via various "if (section == nullptr)" and similar conditionals. So the functionality itself is needed and exists, but only as a runtime feature. The goal of these refactors is to remove the build-time dependencies, which will allow the existing functionality to be used from lower-level parts of the compiler. Particularly from lib/MC/.... More information at: https://discourse.llvm.org/t/rfc-debuginfo-dwarf-refactor-into-to-lower-and-higher-level-libraries/86665
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
…sed) (llvm#143170) (Revised version of a previous, unreviewed, PR.) Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class. This is one in a series of refactoring PRs to separate dwarf functionality into lower-level pieces usable without object files and sections at build time. The code is already written this way via various "if (section == nullptr)" and similar conditionals. So the functionality itself is needed and exists, but only as a runtime feature. The goal of these refactors is to remove the build-time dependencies, which will allow the existing functionality to be used from lower-level parts of the compiler. Particularly from lib/MC/.... More information at: https://discourse.llvm.org/t/rfc-debuginfo-dwarf-refactor-into-to-lower-and-higher-level-libraries/86665
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Revised version of a previous, unreviewed, PR.)
Move all expression verification into its only client: DWARFVerifier. Move all printing code (which was a mix of static and member functions) into a separate class.
This is one in a series of refactoring PRs to separate dwarf functionality into lower-level pieces usable without object files and sections at build time. The code is already written this way via various "if (section == nullptr)" and similar conditionals. So the functionality itself is needed and exists, but only as a runtime feature. The goal of these refactors is to remove the build-time dependencies, which will allow the existing functionality to be used from lower-level parts of the compiler. Particularly from lib/MC/.... More information at:
https://discourse.llvm.org/t/rfc-debuginfo-dwarf-refactor-into-to-lower-and-higher-level-libraries/86665