Fix DeleteDeadVarargs not to crash on functions referenced by BlockAddresses

This pass was assuming that if hasAddressTaken() returns false for a
function, the function's only uses are call sites.  That's not true
because there can be references by BlockAddresses too.

Fix the pass to handle this case.  Fix
BlockAddress::replaceUsesOfWithOnConstant() to allow a function's type
to be changed by RAUW'ing the function with a bitcast of the recreated
function.

Patch by Mark Seaborn.

llvm-svn: 183933

3 files changed, 35 insertions, 3 deletions

diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index d370d409e00..e04be1f59d5 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -1389,7 +1389,7 @@ void BlockAddress::replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) {
   BasicBlock *NewBB = getBasicBlock();
 
   if (U == &Op<0>())
-    NewF = cast<Function>(To);
+    NewF = cast<Function>(To->stripPointerCasts());
   else
     NewBB = cast<BasicBlock>(To);
 
diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
index 3fdb5f0e583..70d03178cb7 100644
--- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -263,8 +263,10 @@ bool DAE::DeleteDeadVarargs(Function &Fn) {
   // to pass in a smaller number of arguments into the new function.
   //
   std::vector<Value*> Args;
-  while (!Fn.use_empty()) {
-    CallSite CS(Fn.use_back());
+  for (Value::use_iterator I = Fn.use_begin(), E = Fn.use_end(); I != E; ) {
+    CallSite CS(*I++);
+    if (!CS)
+      continue;
     Instruction *Call = CS.getInstruction();
 
     // Pass all the same arguments.
@@ -330,6 +332,11 @@ bool DAE::DeleteDeadVarargs(Function &Fn) {
   if (DI != FunctionDIs.end())
     DI->second.replaceFunction(NF);
 
+  // Fix up any BlockAddresses that refer to the function.
+  Fn.replaceAllUsesWith(ConstantExpr::getBitCast(NF, Fn.getType()));
+  // Delete the bitcast that we just created, so that NF does not
+  // appear to be address-taken.
+  NF->removeDeadConstantUsers();
   // Finally, nuke the old function.
   Fn.eraseFromParent();
   return true;
diff --git a/llvm/test/Transforms/DeadArgElim/2013-05-17-VarargsAndBlockAddress.ll b/llvm/test/Transforms/DeadArgElim/2013-05-17-VarargsAndBlockAddress.ll
new file mode 100644
index 00000000000..23216031b58
--- /dev/null
+++ b/llvm/test/Transforms/DeadArgElim/2013-05-17-VarargsAndBlockAddress.ll
@@ -0,0 +1,25 @@
+; RUN: opt %s -deadargelim -S | FileCheck %s
+
+
+@block_addr = global i8* blockaddress(@varargs_func, %l1)
+; CHECK: @block_addr = global i8* blockaddress(@varargs_func, %l1)
+
+
+; This function is referenced by a "blockaddress" constant but it is
+; not address-taken, so the pass should be able to remove its unused
+; varargs.
+
+define internal i32 @varargs_func(i8* %addr, ...) {
+  indirectbr i8* %addr, [ label %l1, label %l2 ]
+l1:
+  ret i32 1
+l2:
+  ret i32 2
+}
+; CHECK: define internal i32 @varargs_func(i8* %addr) {
+
+define i32 @caller(i8* %addr) {
+  %r = call i32 (i8*, ...)* @varargs_func(i8* %addr)
+  ret i32 %r
+}
+; CHECK: %r = call i32 @varargs_func(i8* %addr)