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

[Jeremy Morse via llvm-commits]

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

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.

>From b70091828b2ff5d1c9551a0247b235f412a0db30 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Tue, 5 Dec 2023 12:16:03 +0000
Subject: [PATCH] [DebugInfo][RemoveDIs] Avoid leaking trailing DPMarkers

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.
---
 llvm/lib/IR/BasicBlock.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

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