[patch][pr17535] Reject alias attributes that point to undefined name (V2).

[Richard Smith]

On Mon, Oct 21, 2013 at 5:47 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> On 21 October 2013 20:03, Richard Smith <richard at metafoo.co.uk> wrote:
> > @@ -929,9 +929,8 @@ static RValue EmitNewDeleteCall(CodeGenFunction &CGF,
> >    ///   to a replaceable global allocation function.
> >    ///
> >    /// We model such elidable calls with the 'builtin' attribute.
> > -  llvm::Function *Fn = dyn_cast<llvm::Function>(CalleeAddr);
> >    if (Callee->isReplaceableGlobalAllocationFunction() &&
> > -      Fn && Fn->hasFnAttribute(llvm::Attribute::NoBuiltin)) {
> > +      !Callee->hasAttr<AliasAttr>()) {
> >
> > What's the purpose of this change? Was the previous code broken by
> deferring
> > the emission of aliases?
>
> Correct. Since the alias was not emitted yet the hasFnAttribute return
> false.


OK, that makes sense.


> >
> > +  llvm::GlobalValue *GV = dyn_cast<llvm::GlobalValue>(Aliasee);
> > +  if (!GV) {
> > +    llvm::ConstantExpr *CE = cast<llvm::ConstantExpr>(Aliasee);
> > +    GV = cast<llvm::GlobalValue>(CE->getOperand(0));
> >
> > Hm, what kinds of ConstantExpr can appear here? This would be more
> > self-documenting if you asserted that you'd got the right kind of
> > ConstantExpr.
>
> I can add an assert, but it would be exactly the same as what the Verifier
> does:
>
>  Assert1(CE &&
>             (CE->getOpcode() == Instruction::BitCast ||
>              CE->getOpcode() == Instruction::GetElementPtr) && GV,
>             "Aliasee should be either GlobalValue or bitcast of
> GlobalValue",
>             &GA);
>
> Do you think it is worth it?


I think we should have *something* here. Just pulling out the first operand
of a ConstantExpr without checking what it is looks very suspicious. Can
this happen recursively (gep of a bitcast or similar)?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131021/e603cf92/attachment.html>