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

[Jay Foad]

>>>> --- 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.

Would you be in favour of the attached patch, removing asserts from
GlobalAlias on the grounds that they are checked in the verifier?

Jay.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: globalalias-asserts.diff
Type: text/x-patch
Size: 979 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110802/a45f7e06/attachment.bin>