[llvm-commits] [PATCH][RFC] PR10367: don't allow GlobalAlias's aliasee to be an arbitrary Constant
Jay Foad
jay.foad at gmail.com
Fri Jul 29 00:21:41 PDT 2011
> If we go this way then we eliminate the possibility of having an alias into
> the middle of a global variable. This has been requested a few times on the
> mailing list. For example, recently someone wanted to put canaries around
> global variables. To put such a buffer around a global variable G, the idea
> is to create a new global G2 of the form "{ buff_type, typeof(G), buff_type }"
> and then replace G with an alias of the middle field (of type typeof(G)) in
> G2. This seems like a useful thing to be able to do. So I reckon first we
> need to decide if want to move towards allowing this, or away from allowing
> it (your patch moves away).
OK. I don't really have an opinion on that.
>> --- 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.
Anyway, I'll put this patch on hold until we have a clearer idea of
what direction to go in.
Thanks,
Jay.
More information about the llvm-commits
mailing list