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

[Jay Foad]

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