[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 · Sterling-Augustine · May 8, 2025 · May 8, 2025 · May 9, 2025 · May 9, 2025
commit 7d05067041750b70c0c10ec729316ef91a9acdc8
diff --git 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
@@ -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/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -13,6 +13,7 @@
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/DIContext.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"
@@ -109,7 +110,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:
@@ -943,7 +945,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/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
@@ -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) {