[llvm] 384f916 - Reapply 34cdc913214fd (#74455), call-site-splitting for RemoveDIs

[Jeremy Morse via llvm-commits]

Author: Jeremy Morse
Date: 2023-12-06T16:52:10Z
New Revision: 384f916ea899ea6ac9af4a3fb9d0a5b03937acfe

URL: https://github.com/llvm/llvm-project/commit/384f916ea899ea6ac9af4a3fb9d0a5b03937acfe
DIFF: https://github.com/llvm/llvm-project/commit/384f916ea899ea6ac9af4a3fb9d0a5b03937acfe.diff

LOG: Reapply 34cdc913214fd (#74455), call-site-splitting for RemoveDIs

Original commit message below; asan complained about this commit because it
transpires that the final comparison with CurrentI is in fact a comparison
of a pointer that has been freed. This seems to work fine most of the time,
but using the iterator for such an instruction causes the freed instruction
memory to be accessed, causing a use-after-free. The fix is to perform the
comparison as an instruction, not an iterator.

[NFC][DebugInfo][RemoveDIs] Use iterators to insert in callsite-splitting (#74455)

This patch gets call site splitting to use iterators for insertion
rather than instruction pointers. When we switch on non-instr debug-info
this becomes significant, as the iterators are going to signal whether
or not a position is before or after debug-info.

NFC as this isn't going to affect the output of any existing test.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
index 47af299dbd473..47f663fa0cf0c 100644
--- a/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
+++ b/llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
@@ -372,10 +372,10 @@ static void splitCallSite(CallBase &CB,
     return;
   }
 
-  auto *OriginalBegin = &*TailBB->begin();
+  BasicBlock::iterator OriginalBegin = TailBB->begin();
   // Replace users of the original call with a PHI mering call-sites split.
   if (CallPN) {
-    CallPN->insertBefore(OriginalBegin);
+    CallPN->insertBefore(*TailBB, OriginalBegin);
     CB.replaceAllUsesWith(CallPN);
   }
 
@@ -387,6 +387,7 @@ static void splitCallSite(CallBase &CB,
   // do not introduce unnecessary PHI nodes for def-use chains from the call
   // instruction to the beginning of the block.
   auto I = CB.getReverseIterator();
+  Instruction *OriginalBeginInst = &*OriginalBegin;
   while (I != TailBB->rend()) {
     Instruction *CurrentI = &*I++;
     if (!CurrentI->use_empty()) {
@@ -399,13 +400,13 @@ static void splitCallSite(CallBase &CB,
       for (auto &Mapping : ValueToValueMaps)
         NewPN->addIncoming(Mapping[CurrentI],
                            cast<Instruction>(Mapping[CurrentI])->getParent());
-      NewPN->insertBefore(&*TailBB->begin());
+      NewPN->insertBefore(*TailBB, TailBB->begin());
       CurrentI->replaceAllUsesWith(NewPN);
     }
     CurrentI->dropDbgValues();
     CurrentI->eraseFromParent();
     // We are done once we handled the first original instruction in TailBB.
-    if (CurrentI == OriginalBegin)
+    if (CurrentI == OriginalBeginInst)
       break;
   }
 }