[llvm] [DebugInfo][RemoveDIs] Avoid leaking trailing DPMarkers (PR #74458)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

<details>
<summary>Changes</summary>

In the debug-info-splice implementation, we need to be careful to delete trailing DPMarkers from blocks when we splice their contents out. This is equivalent to removing the terminator from a block, then splicing the rest of it's contents to another block: any DPValues trailing at the end of the block get moved and we need to clean up afterwards.

Digging into this was particularly nasty because... if we don't delete the trailing DPValues, then blocks can be deallocated then reallocated, with the trailing DPMarker appearing to jump from one to the other. That wouldn't happen with a value handle, but then we wouldn't be able to put it in a map.

I think this motivates us needing a less raw-pointer based solution in the future.

This is sort of NFC because it just squelches asan leaking errors, it's really hard to write a test for though, hence there aren't any.

---
Full diff: https://github.com/llvm/llvm-project/pull/74458.diff


1 Files Affected:

- (modified) llvm/lib/IR/BasicBlock.cpp (+9-1) 


``````````diff
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 3ac5fafd887df..872386ddb54b2 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -770,6 +770,7 @@ void BasicBlock::flushTerminatorDbgValues() {
 
   // Transfer DPValues from the trailing position onto the terminator.
   Term->DbgMarker->absorbDebugValues(*TrailingDPValues, false);
+  TrailingDPValues->eraseFromParent();
   deleteTrailingDPValues();
 }
 
@@ -813,6 +814,7 @@ void BasicBlock::spliceDebugInfoEmptyBlock(BasicBlock::iterator Dest,
 
     DPMarker *M = Dest->DbgMarker;
     M->absorbDebugValues(*SrcTrailingDPValues, InsertAtHead);
+    SrcTrailingDPValues->eraseFromParent();
     Src->deleteTrailingDPValues();
     return;
   }
@@ -920,6 +922,7 @@ void BasicBlock::spliceDebugInfoImpl(BasicBlock::iterator Dest, BasicBlock *Src,
   // Use this flag to signal the abnormal case, where we don't want to copy the
   // DPValues ahead of the "Last" position.
   bool ReadFromTail = !Last.getTailBit();
+  bool LastIsEnd = (Last == Src->end());
 
   /*
     Here's an illustration of what we're about to do. We have two blocks, this
@@ -995,12 +998,16 @@ void BasicBlock::spliceDebugInfoImpl(BasicBlock::iterator Dest, BasicBlock *Src,
     DPMarker *OntoDest = getMarker(Dest);
     DPMarker *FromLast = Src->getMarker(Last);
     OntoDest->absorbDebugValues(*FromLast, true);
+    if (LastIsEnd) {
+      FromLast->eraseFromParent();
+      deleteTrailingDPValues();
+    }
   }
 
   // If we're _not_ reading from the head of First, i.e. the "++++" DPValues,
   // move their markers onto Last. They remain in the Src block. No action
   // needed.
-  if (!ReadFromHead) {
+  if (!ReadFromHead && First->hasDbgValues()) {
     DPMarker *OntoLast = Src->createMarker(Last);
     DPMarker *FromFirst = Src->createMarker(First);
     OntoLast->absorbDebugValues(*FromFirst,
@@ -1030,6 +1037,7 @@ void BasicBlock::spliceDebugInfoImpl(BasicBlock::iterator Dest, BasicBlock *Src,
     DPMarker *TrailingDPValues = getTrailingDPValues();
     if (TrailingDPValues) {
       FirstMarker->absorbDebugValues(*TrailingDPValues, true);
+      TrailingDPValues->eraseFromParent();
       deleteTrailingDPValues();
     }
   }

``````````

</details>


https://github.com/llvm/llvm-project/pull/74458