https://github.com/Sterling-Augustine created https://github.com/llvm/llvm-project/pull/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 >From 1b2fcdccc6d7f35dc2f387259c70fe6c9825b8b0 Mon Sep 17 00:00:00 2001 From: Sterling Augustine <saugust...@google.com> Date: Wed, 4 Jun 2025 11:53:45 -0700 Subject: [PATCH 1/2] [NFC] Separate high-level-dependent portions of DWARFExpression 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/DebugInfo/DWARF/DWARFExpression.h | 88 ++++++++--- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 18 +++ llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp | 2 +- llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp | 4 +- llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp | 3 +- llvm/lib/DebugInfo/DWARF/DWARFDie.cpp | 4 +- llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp | 149 ++++++++---------- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 31 +++- .../LogicalView/Readers/LVDWARFReader.cpp | 4 +- llvm/tools/llvm-objdump/SourcePrinter.cpp | 2 +- .../DWARFExpressionCompactPrinterTest.cpp | 2 +- 11 files changed, 187 insertions(+), 120 deletions(-) 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 << ", "; } } -bool DWARFExpression::Operation::verify(const Operation &Op, DWARFUnit *U) { - for (unsigned Operand = 0; Operand < Op.Desc.Op.size(); ++Operand) { - unsigned Size = Op.Desc.Op[Operand]; - - if (Size == Operation::BaseTypeRef) { - // For DW_OP_convert the operand may be 0 to indicate that conversion to - // the generic type should be done, so don't look up a base type in that - // case. The same holds for DW_OP_reinterpret, which is currently not - // supported. - if (Op.Opcode == DW_OP_convert && Op.Operands[Operand] == 0) - continue; - auto Die = U->getDIEForOffset(U->getOffset() + Op.Operands[Operand]); - if (!Die || Die.getTag() != dwarf::DW_TAG_base_type) - return false; - } - } - - return true; -} - -bool DWARFExpression::verify(DWARFUnit *U) { - for (auto &Op : *this) - if (!Operation::verify(Op, U)) - return false; - - return true; -} - /// A user-facing string representation of a DWARF expression. This might be an /// Address expression, in which case it will be implicitly dereferenced, or a /// Value expression. @@ -546,16 +527,10 @@ static bool printCompactDWARFExpr( return true; } -bool DWARFExpression::printCompact( - raw_ostream &OS, +bool DWARFExpressionPrinter::printCompact( + const DWARFExpression *E, raw_ostream &OS, std::function<StringRef(uint64_t RegNum, bool IsEH)> GetNameForDWARFReg) { - return printCompactDWARFExpr(OS, begin(), end(), GetNameForDWARFReg); -} - -bool DWARFExpression::operator==(const DWARFExpression &RHS) const { - if (AddressSize != RHS.AddressSize || Format != RHS.Format) - return false; - return Data.getData() == RHS.Data.getData(); + return printCompactDWARFExpr(OS, E->begin(), E->end(), GetNameForDWARFReg); } } // namespace llvm diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index 43a62bdd8390d..c12786cac8686 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -659,6 +659,35 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, return NumErrors; } +bool DWARFVerifier::verifyExpressionOp(const DWARFExpression::Operation &Op, + DWARFUnit *U) { + for (unsigned Operand = 0; Operand < Op.Desc.Op.size(); ++Operand) { + unsigned Size = Op.Desc.Op[Operand]; + + if (Size == DWARFExpression::Operation::BaseTypeRef) { + // For DW_OP_convert the operand may be 0 to indicate that conversion to + // the generic type should be done, so don't look up a base type in that + // case. The same holds for DW_OP_reinterpret, which is currently not + // supported. + if (Op.Opcode == DW_OP_convert && Op.Operands[Operand] == 0) + continue; + auto Die = U->getDIEForOffset(U->getOffset() + Op.Operands[Operand]); + if (!Die || Die.getTag() != dwarf::DW_TAG_base_type) + return false; + } + } + + return true; +} + +bool DWARFVerifier::verifyExpression(const DWARFExpression &E, DWARFUnit *U) { + for (auto &Op : E) + if (!verifyExpressionOp(Op, U)) + return false; + + return true; +} + unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, DWARFAttribute &AttrValue) { unsigned NumErrors = 0; @@ -727,7 +756,7 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, any_of(Expression, [](const DWARFExpression::Operation &Op) { return Op.isError(); }); - if (Error || !Expression.verify(U)) + if (Error || !verifyExpression(Expression, U)) ReportError("Invalid DWARF expressions", "DIE contains invalid DWARF expression:"); } diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp index 3dbe11accd2d4..e810bb8e833b2 100644 --- a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp +++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp @@ -587,8 +587,8 @@ std::string LVDWARFReader::getRegisterName(LVSmall Opcode, return {}; }; DumpOpts.GetNameForDWARFReg = GetRegName; - DWARFExpression::prettyPrintRegisterOp(/*U=*/nullptr, Stream, DumpOpts, - Opcode, Operands); + DWARFExpressionPrinter::prettyPrintRegisterOp(/*U=*/nullptr, Stream, DumpOpts, + Opcode, Operands); return Stream.str(); } diff --git a/llvm/tools/llvm-objdump/SourcePrinter.cpp b/llvm/tools/llvm-objdump/SourcePrinter.cpp index 33d494bfd1ac7..bf3363795a4d9 100644 --- a/llvm/tools/llvm-objdump/SourcePrinter.cpp +++ b/llvm/tools/llvm-objdump/SourcePrinter.cpp @@ -45,7 +45,7 @@ void LiveVariable::print(raw_ostream &OS, const MCRegisterInfo &MRI) const { return {}; }; - Expression.printCompact(OS, GetRegName); + DWARFExpressionPrinter::printCompact(&Expression, OS, GetRegName); } void LiveVariablePrinter::addVariable(DWARFDie FuncDie, DWARFDie VarDie) { diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp index 62f48b6036f2a..9225ab0125f2f 100644 --- a/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp +++ b/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp @@ -70,7 +70,7 @@ void DWARFExpressionCompactPrinterTest::TestExprPrinter( return {}; }; - Expr.printCompact(OS, GetRegName); + DWARFExpressionPrinter::printCompact(&Expr, OS, GetRegName); EXPECT_EQ(OS.str(), Expected); } >From df8db8240fec9129cef4e3bcd8eb8792ce4add01 Mon Sep 17 00:00:00 2001 From: Sterling Augustine <saugust...@google.com> Date: Thu, 5 Jun 2025 16:44:35 -0700 Subject: [PATCH 2/2] Update lldb targets. --- lldb/source/Expression/DWARFExpression.cpp | 4 ++-- lldb/source/Symbol/UnwindPlan.cpp | 5 +++-- lldb/unittests/Symbol/PostfixExpressionTest.cpp | 4 ++-- .../NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) 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); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits