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

[Rafael Espíndola]

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.

>
> +  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?

Cheers,
Rafael