[llvm] r183933 - Fix DeleteDeadVarargs not to crash on functions referenced by BlockAddresses

[Eli Friedman]

On Thu, Jun 13, 2013 at 1:05 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Derek Schuff wrote:
>
>> Author: dschuff
>> Date: Thu Jun 13 14:51:17 2013
>> New Revision: 183933
>>
>> URL: http://llvm.org/viewvc/llvm-**project?rev=183933&view=rev<http://llvm.org/viewvc/llvm-project?rev=183933&view=rev>
>> Log:
>> 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.
>>
>> Added:
>>      llvm/trunk/test/Transforms/**DeadArgElim/2013-05-17-**
>> VarargsAndBlockAddress.ll
>> Modified:
>>      llvm/trunk/lib/IR/Constants.**cpp
>>      llvm/trunk/lib/Transforms/IPO/**DeadArgumentElimination.cpp
>>
>> Modified: llvm/trunk/lib/IR/Constants.**cpp
>> URL: http://llvm.org/viewvc/llvm-**project/llvm/trunk/lib/IR/**
>> Constants.cpp?rev=183933&r1=**183932&r2=183933&view=diff<http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Constants.cpp?rev=183933&r1=183932&r2=183933&view=diff>
>> ==============================**==============================**
>> ==================
>> --- llvm/trunk/lib/IR/Constants.**cpp (original)
>> +++ llvm/trunk/lib/IR/Constants.**cpp Thu Jun 13 14:51:17 2013
>> @@ -1389,7 +1389,7 @@ void BlockAddress::**replaceUsesOfWithOnCo
>>     BasicBlock *NewBB = getBasicBlock();
>>
>>     if (U ==&Op<0>())
>>
>> -    NewF = cast<Function>(To);
>> +    NewF = cast<Function>(To->**stripPointerCasts());
>>
>
> This makes me extremely nervous. This function is very low level, it seems
> like it shouldn't be doing anything like calling stripPointerCasts(),
> that'd be like rigging SmallVector to call eraseFromParent for you or
> something.
>
> What are the alternatives here, making BlockAddress hold a ConstantExpr
> instead of a Function? Because I can see why that's also very bad.
>

We could also add a special ReplaceAllUsesWith variant specifically for
this case, which doesn't require the types of the From and To values to be
the same, and asserts all the uses are BlockAddresses.  I mean, the added
call to replaceAllUsesWith is doing precisely what it's supposed to do; the
part that's iffy is messing with the general case of ReplaceAllUsesWith.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130613/1f1cd022/attachment.html>