[NFC] Separate high-level-dependent portions of DWARFExpression (revised) (#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

Author: Sterling-Augustine

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; }

lldb/source/Symbol/UnwindPlan.cpp

@@ -87,8 +87,10 @@ 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");
 }

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;
 }
 

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);

llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h

@@ -77,6 +77,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;
@@ -99,11 +102,6 @@ class DWARFExpression {
     }
     uint64_t getEndOffset() const { return EndOffset; }
     bool isError() const { return Error; }
-    LLVM_ABI bool print(raw_ostream &OS, DIDumpOptions DumpOpts,
-                        const DWARFExpression *Expr, DWARFUnit *U) const;
-
-    /// Verify \p Op. Does not affect the return of \a isError().
-    LLVM_ABI static bool verify(const Operation &Op, DWARFUnit *U);
 
   private:
     LLVM_ABI bool extract(DataExtractor Data, uint8_t AddressSize,
@@ -154,28 +152,12 @@ class DWARFExpression {
   iterator begin() const { return iterator(this, 0); }
   iterator end() const { return iterator(this, Data.getData().size()); }
 
-  LLVM_ABI 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.
-  LLVM_ABI bool printCompact(
-      raw_ostream &OS,
-      std::function<StringRef(uint64_t RegNum, bool IsEH)> GetNameForDWARFReg =
-          nullptr);
-
-  LLVM_ABI bool verify(DWARFUnit *U);
-
   LLVM_ABI bool operator==(const DWARFExpression &RHS) const;
 
   StringRef getData() const { return Data.getData(); }
 
-  LLVM_ABI 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;
@@ -187,5 +169,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

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 "llvm/Support/Compiler.h"
 #include <cstdint>
@@ -323,6 +324,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:
   LLVM_ABI
   DWARFVerifier(raw_ostream &S, DWARFContext &D,

llvm/lib/DebugInfo/DWARF/DWARFCFIProgram.cpp

@@ -425,7 +425,8 @@ 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.value(), OS, DumpOpts,
+                                  nullptr);
     break;
   }
 }

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:

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(

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) {