[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.

Author: Sterling-Augustine

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

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

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

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