Fix use-after-free bug in mergeTwoFunctions().

[aemerson]

This was caught by Apple's Probabilistic Guard Malloc which detected
that OldF's memory is freed inside mergeTwoFunctions(), and then
back in insert() the now dangling pointer is dereferenced again.

rdar://163874208

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Amara Emerson (aemerson)

Changes

This was caught by Apple's Probabilistic Guard Malloc which detected
that OldF's memory is freed inside mergeTwoFunctions(), and then
back in insert() the now dangling pointer is dereferenced again.

rdar://163874208

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/IPO/MergeFunctions.cpp (+7-3)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index f5525deb0172f..0faa36a495acc 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -1027,12 +1027,16 @@ bool MergeFunctions::insert(Function *NewFunction) {
     assert(OldF.getFunc() != F && "Must have swapped the functions.");
   }
 
-  LLVM_DEBUG(dbgs() << "  " << OldF.getFunc()->getName()
+  // Capture the Function pointer before mergeTwoFunctions, which may invalidate
+  // OldF by erasing it from FnTree via removeUsers().
+  Function *OldFunc = OldF.getFunc();
+
+  LLVM_DEBUG(dbgs() << "  " << OldFunc->getName()
                     << " == " << NewFunction->getName() << '\n');
 
   Function *DeleteF = NewFunction;
-  mergeTwoFunctions(OldF.getFunc(), DeleteF);
-  this->DelToNewMap.insert({DeleteF, OldF.getFunc()});
+  mergeTwoFunctions(OldFunc, DeleteF);
+  this->DelToNewMap.insert({DeleteF, OldFunc});
   return true;
 }
 

[boomanaiden154]

LGTM.

Cool to see hardening features catching issues like this.

[aemerson]

LGTM.

Cool to see hardening features catching issues like this.

Thanks, and agreed!