Warn when returning the address of a temporary (middle-end)

[Richard Biener]

On Mon, Apr 7, 2014 at 8:51 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 7 Apr 2014, Jeff Law wrote:
>
>> On 04/05/14 07:52, Marc Glisse wrote:
>>>
>>> Hello,
>>>
>>> we have front-end warnings about returning the address of a local
>>> variable. However, quite often in C++, people don't directly return the
>>> address of a temporary, it goes through a few functions which hide that
>>> fact. After some inlining, the fact that we are returning the address of
>>> a local variable can become obvious to the compiler, to the point where
>>> I have used, for debugging purposes, grep 'return &' on the optimized
>>> dump produced with -O3 -fkeep-inline-functions (I then had to sort
>>> through the global/local variables).
>>>
>>> fold_stmt looks like a good place for this, but it could go almost
>>> anywhere. It has to happen after enough inlining / copy propagation to
>>> make it useful though.
>>
>> I was wondering if this would be better implemented as a propagation
>> problem so that cases where some, but not all paths to the return statement
>> have &local which reaches the return.  ie
>>
>> ...
>> if (foo)
>>  x = &local
>> else
>>  x = &global
>>
>> return x;
>>
>> ISTM it ought to be a standard propagation problem and that the
>> problematical ADDR_EXPRs are all going to be in PHI nodes.  So I think you
>> just search for problematical ADDR_EXPRs in PHI nodes as your seeds, then
>> forward propagate through the CFG.  Ultimately looking for any cases where
>> those ADDR_EXPRs ultimately reach the return statement.
>>
>> Thoughts?
>
>
> I would tend to start from the return statements (assuming the return type
> is a pointer), look at the defining statement, do things if it is an
> assignment of an addr_expr, and recurse if it is a PHI. But maybe my brain
> is cabled backwards ;-)
>
> It would be good to piggy back on a pass that already does something
> similar, if we go that way. Do you know a convenient one?
>
> I am also afraid we may get more false positives, but maybe not.

If you don't mind false positives you can also do the check in
points-to analysis, looking at the points-to result for the SSA name
we return.

Btw, the idea to add this to -fisolate-errorneous-paths sounds good to me,
as well as returning NULL for them.

Richard.

> Last, the simple version actually works well enough that it discovered at
> least one real bug in real code, and I am afraid that by refining it too
> much we'll delay and get nothing in the end (my time and my knowledge of the
> compiler are limited enough to make it a real possibility). But I admit
> that's not a good argument.
>
> Thanks for the comments,
>
> --
> Marc Glisse