[PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: avl, friss, aprantl.
Herald added a subscriber: hiraditya.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D125469 <https://reviews.llvm.org/D125469> changed the assumptions regarding forward references. Before the patch, a reference to an uncloned DIE was always a forward reference. After the change, that assumption no longer holds. It was enforced by the assertion in `DWARFLinker::DIECloner::cloneDieReferenceAttribute` (DWARFLinker.cpp:937):

  assert(Ref > InputDIE.getOffset());

This patch is relatively straightforward: it adds a field to the `DIEInfo` to remember the DIE is a forward or backward reference. We then treat backward references like a forward references by fixing them up after the fact, when all DIEs have been cloned.

We tripped this assertion for an internal project built with LTO. Unfortunately that means I'm not able to share a test case. I've spent a few hours trying to craft a test case that would hit this case but didn't succeed. Getting a backward reference is easy, but a backward reference to a DIE that hasn't been cloned yet is not something I managed to reproduce. I'd love to have a test case to avoid regressing this in the future, so please let me know if you have an idea on how to trigger this behavior from an artificial test case.


https://reviews.llvm.org/D138176

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


Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===================================================================
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-    assert(Ref > InputDIE.getOffset());
     // We haven't cloned this DIE yet. Just create an empty one and
     // store it. It'll get really cloned when we process it.
+    RefInfo.Reference = true;
     RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -948,17 +948,16 @@
     // to find the unit offset. (We don't have a DwarfDebug)
     // FIXME: we should be able to design DIEEntry reliance on
     // DwarfDebug away.
-    uint64_t Attr;
-    if (Ref < InputDIE.getOffset()) {
+    if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
       // We must have already cloned that DIE.
       uint32_t NewRefOffset =
           RefUnit->getStartOffset() + NewRefDie->getOffset();
-      Attr = NewRefOffset;
+      uint64_t Attr = NewRefOffset;
       Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
                    dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
     } else {
       // A forward reference. Note and fixup later.
-      Attr = 0xBADDEF;
+      uint64_t Attr = 0xBADDEF;
       Unit.noteForwardReference(
           NewRefDie, RefUnit, RefInfo.Ctxt,
           Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===================================================================
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
 
     /// Is ODR marking done?
     bool ODRMarkingDone : 1;
+
+    /// Is DIE a forward or backward reference?
+    bool Reference : 1;
   };
 
   CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138176.475983.patch
Type: text/x-patch
Size: 1928 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221117/bbf04ef9/attachment.bin>