[llvm-commits] [PATCH][RFC] PR10367: don't allow GlobalAlias's aliasee to be an arbitrary Constant

[Duncan Sands]

Hi Jay,

>>> --- llvm/trunk/lib/VMCore/Verifier.cpp  (revision 136357)
>>> +++ llvm/trunk/lib/VMCore/Verifier.cpp  (working copy)
>>> @@ -455,20 +455,8 @@
>>>             "Alias should have external or external weak linkage!",&GA);
>>>     Assert1(GA.getAliasee(),
>>>             "Aliasee cannot be NULL!",&GA);
>>> -  Assert1(GA.getType() == GA.getAliasee()->getType(),
>>> -          "Alias and aliasee types should match!",&GA);
>>>     Assert1(!GA.hasUnnamedAddr(), "Alias cannot have unnamed_addr!",&GA);
>>>
>>> -  if (!isa<GlobalValue>(GA.getAliasee())) {
>>> -    const ConstantExpr *CE = dyn_cast<ConstantExpr>(GA.getAliasee());
>>> -    Assert1(CE&&
>>> -            (CE->getOpcode() == Instruction::BitCast ||
>>> -             CE->getOpcode() == Instruction::GetElementPtr)&&
>>> -            isa<GlobalValue>(CE->getOperand(0)),
>>> -            "Aliasee should be either GlobalValue or bitcast of GlobalValue",
>>> -&GA);
>>> -  }
>>> -
>>>     const GlobalValue* Aliasee = GA.resolveAliasedGlobal(/*stopOnWeak*/ false);
>>>     Assert1(Aliasee,
>>>             "Aliasing chain should end with function or global variable",&GA);
>>
>> Don't you still need to check that the operand is either a global, or a bitcast
>> or GEP of a global?
>
> The call to GA.getAliasee() a few lines above has already asserted all that.
>
> But I suppose, since this is the verifier, we should be using
> Assert1(), not assert(), so maybe I should (a)
> s/getAliasee()/getOperand(0)/, to bypass the checks that getAliasee
> does, and then (b) reinstate the checks I've removed above.

right.  In fact getAliasee should not really contain the assertions it does: it
is the job of the verifier to check that code is well formed; everywhere else
can just assume it.

> Anyway, I'll put this patch on hold until we have a clearer idea of
> what direction to go in.

It would be good if Chris and/or Anton commented on this.  For example I don't
know if aliases into the middle of a global are actually implementable on any of
LLVM's supported platforms.

Ciao, Duncan.