[PATCH] D58236: Make address space conversions a bit stricter.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D58236#1416765 <https://reviews.llvm.org/D58236#1416765>, @Anastasia wrote:

> In D58236#1414069 <https://reviews.llvm.org/D58236#1414069>, @efriedma wrote:
>
> > > I think trying to reject code that is doing something dangerous is a good thing!
> >
> > Refusing to compile code which is suspicious, but not forbidden by the specification, will likely cause compatibility issues; there are legitimate reasons to use casts which look weird.
>
>
> The spec dioesn't allow these conversions either, it just simply doesn't cover this corner case at all. I don't think we are changing anything in terms of compatibility. If you have any examples of such casts that can be legitimate I would like to understand them better. What I have seen so far were the examples where `addrspacecast` was lost in IR for the memory segments translation and therefore wrong memory areas were accessed.


The spec just says that the casts follow C rules... and C says you can cast a pointer to an object type to a pointer to another object type (subject to alignment restrictions).  By default, a pointer to a pointer isn't special.

In practice, unusual casts tend to show up in code building a datastructure using union-like constructs.  In plain C, for example, sometimes you have a pointer to a float, and sometimes you have a pointer to an int, determined dynamically.  I expect similar cases show up where a pointer points to memory which contains either a pointer in the global address-space, or a pointer in the local address-space, determined dynamically.  In some cases, it might be clearer to use void* in more places, but that's mostly style issue.

>> I'm not against adding some sort of address space suspicious cast warning to catch cases where we think the user meant to do something else.
> 
> I simply don't see how these conversions can be useful and some are definitely indirectly forbidden (there is no precise wording however). There are other ways to perform such conversions differently (by being more explicit) where correct IR can be then generated with `addrspacecast`. I don't think we are loosing anything in terms of functionality.

In some cases, the correct code may not involve an addrspacecast at all; the pointer could just have the "wrong" pointee type, and the code expects to explicitly fix it.  In that case, how do you fix it?  Write `(foo*)(void*)p` instead of `(foo*)p`?

>> But that's a separate issue, and it needs a proper cost-benefit analysis, including an analysis of the false-positive rate on existing code.
> 
> Do you have any suggestions how to do this in practice with such rare corner case?

If the warning never triggers on any code you have access to, that would still be a useful datapoint.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58236/new/

https://reviews.llvm.org/D58236