[llvm] 1b79bed - [dsymutil] Verify the keep chain when asserts are enabled

[Jonas Devlieghere via llvm-commits]

Author: Jonas Devlieghere
Date: 2022-12-20T12:53:17-08:00
New Revision: 1b79bed8f53231144ad6e14f8cc3e8673c7fb469

URL: https://github.com/llvm/llvm-project/commit/1b79bed8f53231144ad6e14f8cc3e8673c7fb469
DIFF: https://github.com/llvm/llvm-project/commit/1b79bed8f53231144ad6e14f8cc3e8673c7fb469.diff

LOG: [dsymutil] Verify the keep chain when asserts are enabled

Verify that every DIE that's marked as kept, has a parent that's kept as
well. This invariant should always hold and is easy to verify when
asserts are enabled.

Differential revision: https://reviews.llvm.org/D140227

Added: 
    

Modified: 
    llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
    llvm/lib/DWARFLinker/DWARFLinker.cpp
    llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
index 517cab8a9c11..d21126f3ef3b 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -81,6 +81,10 @@ class CompileUnit {
 
     /// Is this a reference to a DIE that hasn't been cloned yet?
     bool UnclonedReference : 1;
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+    LLVM_DUMP_METHOD void dump();
+#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   };
 
   CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR,

diff  --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index 6096ffc5ad5f..112be367d1b5 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -850,6 +850,58 @@ void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
   }
 }
 
+#ifndef NDEBUG
+/// A broken link in the keep chain. By recording both the parent and the child
+/// we can show only broken links for DIEs with multiple children.
+struct BrokenLink {
+  BrokenLink(DWARFDie Parent, DWARFDie Child) : Parent(Parent), Child(Child) {}
+  DWARFDie Parent;
+  DWARFDie Child;
+};
+
+/// Verify the keep chain by looking for DIEs that are kept but who's parent
+/// isn't.
+static void verifyKeepChain(CompileUnit &CU) {
+  std::vector<DWARFDie> Worklist;
+  Worklist.push_back(CU.getOrigUnit().getUnitDIE());
+
+  // List of broken links.
+  std::vector<BrokenLink> BrokenLinks;
+
+  while (!Worklist.empty()) {
+    const DWARFDie Current = Worklist.back();
+    Worklist.pop_back();
+
+    const bool CurrentDieIsKept = CU.getInfo(Current).Keep;
+
+    for (DWARFDie Child : reverse(Current.children())) {
+      Worklist.push_back(Child);
+
+      const bool ChildDieIsKept = CU.getInfo(Child).Keep;
+      if (!CurrentDieIsKept && ChildDieIsKept)
+        BrokenLinks.emplace_back(Current, Child);
+    }
+  }
+
+  if (!BrokenLinks.empty()) {
+    for (BrokenLink Link : BrokenLinks) {
+      WithColor::error() << formatv(
+          "Found invalid link in keep chain between {0:x} and {1:x}\n",
+          Link.Parent.getOffset(), Link.Child.getOffset());
+
+      errs() << "Parent:";
+      Link.Parent.dump(errs(), 0, {});
+      CU.getInfo(Link.Parent).dump();
+
+      errs() << "Child:";
+      Link.Child.dump(errs(), 2, {});
+      CU.getInfo(Link.Child).dump();
+    }
+    report_fatal_error("invalid keep chain");
+  }
+}
+#endif
+
 /// Assign an abbreviation number to \p Abbrev.
 ///
 /// Our DIEs get freed after every DebugMapObject has been processed,
@@ -2535,12 +2587,16 @@ Error DWARFLinker::link() {
         CurrentUnit->markEverythingAsKept();
       copyInvariantDebugSection(*OptContext.File.Dwarf);
     } else {
-      for (auto &CurrentUnit : OptContext.CompileUnits)
+      for (auto &CurrentUnit : OptContext.CompileUnits) {
         lookForDIEsToKeep(*OptContext.File.Addresses,
                           OptContext.File.Addresses->getValidAddressRanges(),
                           OptContext.CompileUnits,
                           CurrentUnit->getOrigUnit().getUnitDIE(),
                           OptContext.File, *CurrentUnit, 0);
+#ifndef NDEBUG
+        verifyKeepChain(*CurrentUnit);
+#endif
+      }
     }
 
     // The calls to applyValidRelocs inside cloneDIE will walk the reloc

diff  --git a/llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp b/llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
index d8bac503fe65..72cb95fc7804 100644
--- a/llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
@@ -8,9 +8,28 @@
 
 #include "llvm/DWARFLinker/DWARFLinkerCompileUnit.h"
 #include "llvm/DWARFLinker/DWARFLinkerDeclContext.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace llvm {
 
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void CompileUnit::DIEInfo::dump() {
+  llvm::errs() << "{\n";
+  llvm::errs() << "  AddrAdjust: " << AddrAdjust << '\n';
+  llvm::errs() << "  Ctxt: " << formatv("{0:x}", Ctxt) << '\n';
+  llvm::errs() << "  Clone: " << formatv("{0:x}", Clone) << '\n';
+  llvm::errs() << "  ParentIdx: " << ParentIdx << '\n';
+  llvm::errs() << "  Keep: " << Keep << '\n';
+  llvm::errs() << "  InDebugMap: " << InDebugMap << '\n';
+  llvm::errs() << "  Prune: " << Prune << '\n';
+  llvm::errs() << "  Incomplete: " << Incomplete << '\n';
+  llvm::errs() << "  InModuleScope: " << InModuleScope << '\n';
+  llvm::errs() << "  ODRMarkingDone: " << ODRMarkingDone << '\n';
+  llvm::errs() << "  UnclonedReference: " << UnclonedReference << '\n';
+  llvm::errs() << "}\n";
+}
+#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+
 /// Check if the DIE at \p Idx is in the scope of a function.
 static bool inFunctionScope(CompileUnit &U, unsigned Idx) {
   while (Idx) {