On Mon, Apr 7, 2014 at 8:51 PM, Marc Glisse <marc.gli...@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
