[PATCH] D113957: DebugInfo: Stop modifying Operation::Error inside of verify()

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith created this revision.
dexonsmith added reviewers: krasimir, aprantl, dblaikie.
Herald added a subscriber: hiraditya.
dexonsmith requested review of this revision.
Herald added a project: LLVM.

The only caller of Operation::verify() is DWARFExpression::verify(),
which iterates past the (ephemeral) Operation immediately after.

- Stop setting Operation::Error because the mutation will never be observed.
- Change verify() to a static function to be sure all callers are updated.

This is motivated by a follow-up patch, which will make
DWARFExpression::iterator iterate over `const Operation`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113957

Files:
  llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
  llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp


Index: llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
===================================================================
--- llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
@@ -357,10 +357,9 @@
   }
 }
 
-bool DWARFExpression::Operation::verify(DWARFUnit *U) {
-
+bool DWARFExpression::Operation::verify(const Operation &Op, DWARFUnit *U) {
   for (unsigned Operand = 0; Operand < 2; ++Operand) {
-    unsigned Size = Desc.Op[Operand];
+    unsigned Size = Op.Desc.Op[Operand];
 
     if (Size == Operation::SizeNA)
       break;
@@ -370,13 +369,11 @@
       // 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 (Opcode == DW_OP_convert && Operands[Operand] == 0)
+      if (Op.Opcode == DW_OP_convert && Op.Operands[Operand] == 0)
         continue;
-      auto Die = U->getDIEForOffset(U->getOffset() + Operands[Operand]);
-      if (!Die || Die.getTag() != dwarf::DW_TAG_base_type) {
-        Error = true;
+      auto Die = U->getDIEForOffset(U->getOffset() + Op.Operands[Operand]);
+      if (!Die || Die.getTag() != dwarf::DW_TAG_base_type)
         return false;
-      }
     }
   }
 
@@ -385,7 +382,7 @@
 
 bool DWARFExpression::verify(DWARFUnit *U) {
   for (auto &Op : *this)
-    if (!Op.verify(U))
+    if (!Operation::verify(Op, U))
       return false;
 
   return true;
Index: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
===================================================================
--- llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
+++ llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
@@ -97,7 +97,9 @@
     bool print(raw_ostream &OS, DIDumpOptions DumpOpts,
                const DWARFExpression *Expr, const MCRegisterInfo *RegInfo,
                DWARFUnit *U, bool isEH) const;
-    bool verify(DWARFUnit *U);
+
+    /// 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,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113957.387456.patch
Type: text/x-patch
Size: 2148 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211116/9e63c71d/attachment.bin>