[llvm-commits] [llvm] r123477 - /llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp

[Chris Lattner]

On Jan 14, 2011, at 2:19 PM, Owen Anderson wrote:

> Author: resistor
> Date: Fri Jan 14 16:19:20 2011
> New Revision: 123477
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=123477&view=rev
> Log:
> Enhance GlobalOpt to be able evaluate initializers that involve stores through
> bitcasts, at least in simple cases.  This fixes clang's CodeGenCXX/virtual-base-dtor.cpp

Hi Owen.

Thanks for fixing the regression, but you should definitely add a testcase to the globalopt testsuite to catch this. 

In particular, the code you've added isn't safe (besides breaking the testsuite, which has already been fixed on mainline):

> +          // A bitcast'd pointer implicitly points to the first field of a
> +          // struct.  Insert implicity "gep @x, 0, 0, ..." until we get down
> +          // to the first concrete member.
> +          // FIXME: This could be extended to work for arrays as well.
> +          while (const StructType *STy = dyn_cast<StructType>(NewTy)) {
> +            NewTy = STy->getTypeAtIndex(0U);
> +            
> +            const IntegerType *IdxTy =
> +              IntegerType::get(NewTy->getContext(), 32);
> +            Constant *IdxZero = ConstantInt::get(IdxTy, 0, false);
> +            Constant * const IdxList[] = {IdxZero, IdxZero};
> +            
> +            Ptr = ConstantExpr::getGetElementPtr(Ptr, IdxList, 2);
> +          }
> +          
> +          if (!isa<PointerType>(NewTy)) return false;
> +          Val = ConstantExpr::getBitCast(Val, NewTy);
> +        }
> +          

This code seems highly confused.  If the thing being stored isn't a pointer, the final getBitCast call will abort.

-Chris