Bug 49974 - missing -Wreturn-local-addr for indirectly returning reference to local/temporary
Summary: missing -Wreturn-local-addr for indirectly returning reference to local/tempo...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 57528 79307 95911 (view as bug list)
Depends on: 63446
Blocks: Wreturn-local-addr
  Show dependency treegraph
 
Reported: 2011-08-04 12:22 UTC by Jonathan Wakely
Modified: 2021-04-16 17:17 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-05-30 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2011-08-04 12:22:54 UTC
Although probably not easy, it would be useful to get warnings from this code which creates dangling references:

struct X { };

inline const X& f(const X& r) { return r; }

const X& g()
{
    X x;
    return f(x);  // !!!
}

const X& h()
{
    return f(X()); // !!!
}

Another case involves a reference as a member of a class:

struct Y {
    Y(int& i) : r(i) { }
    int& r;
};

Y f()
{
    int i=0;
    return Y(i);
}
Comment 1 Manuel López-Ibáñez 2011-08-04 12:29:21 UTC
Isn't this related to PR986?
Comment 2 Jonathan Wakely 2011-08-04 12:40:15 UTC
Maybe related, but not the same.  PR986 involves creating a temporary and binding the reference to it, that should be easier to warn about.  Here's a similar case to PR986 where a reference member is dangling after the constructor, which we fail to warn about:

struct Y {
    Y(int local) : ref(local) { }
    int& ref;
};

That, and the example in 986, should be easy to warn about.  When the reference is bound we can know if the initializer is local/temporary.

This PR is different, the references passed to f and Y::Y are valid during those calls. The reference only becomes invalid when it escapes from the function that declares the local variable.  This requires some more complex analysis, only possible if the bodies of f and Y::Y are visible, and maybe only possible with optimization enabled so inlining happens.
Comment 3 Richard Biener 2011-08-04 12:48:48 UTC
You could try to rely on points-to information.  When it says that all possible
pointees are automatic variable it's for sure an error (or a points-to bug).
Comment 4 Jonathan Wakely 2012-04-08 18:15:04 UTC
As pointed out in PR 52901 comment 3, this missing warning is likely to bite people misusing std::move like so:

X&& f()
{
  X x;
  return std::move(x);
}
Comment 5 Paolo Carlini 2013-06-05 18:26:44 UTC
*** Bug 57528 has been marked as a duplicate of this bug. ***
Comment 6 Jonathan Wakely 2017-02-01 14:50:56 UTC
*** Bug 79307 has been marked as a duplicate of this bug. ***
Comment 7 Marc Glisse 2017-02-01 14:56:16 UTC
We currently warn on all the examples involving X, with -O2. We don't for Y, we might if there was a caller and the dangling reference was used there...
Comment 8 Szikra 2017-02-02 17:09:01 UTC
(In reply to Marc Glisse from comment #7)
> We currently warn on all the examples involving X, with -O2. We don't for Y,
> we might if there was a caller and the dangling reference was used there...

Hi, I assume you mean like this:
#include <iostream>
using namespace std;
struct Y {
    Y(int& i) : r(i) { }
    int& r;
};

Y f() {
    int i=1;
    return Y(i);
}

int main() {
    Y y1= f();
    cout << y1.r << endl; ///  warning: 'i' is used uninitialized in this function [-Wuninitialized]
}

/* compiled with gcc version 6.3.0 (x86_64-posix-seh-rev1, Built by MinGW-W64 project)
g++ -std=c++11 -O2 -g3 -Wall -Wextra -Wconversion -c -fmessage-length=0 -v -o "src\\test3.o" "..\\src\\test3.cpp" 
*/

What about this?
struct Y2 {
    Y2(const int& i) : r(i) { }
    const int& r;
};

We might or might not get a warning even if the reference is used.

int main() {
    Y2 y2(2);
    cout << y2.r << endl; ///  warning: '<anonymous>' is used uninitialized in this function [-Wuninitialized]
}

and 

Y2 g_y2(22);
int main() {
    cout << g_y2.r << endl; /// no warning at all
}

Or what about this?
struct Z {
    Z(int i) : r(i) { }
    int& r;
};

Z g_z(33);
int main() {
    cout << g_z.r << endl; /// no warning
    Z z(3);
    cout << z.r << endl; /// no warning
}
Comment 9 Marc Glisse 2017-02-12 14:45:15 UTC
gcc is unable to look through dynamic static initializers, even with -flto or -fwhole-program. If you use constexpr, you can make the program fail to compile.
(adding your warning would be good, I just wanted to clarify what we already have)
Comment 10 Eric Gallager 2018-08-20 19:32:49 UTC
(In reply to Manuel López-Ibáñez from comment #1)
> Isn't this related to PR986?

(In reply to Jonathan Wakely from comment #2)
> Maybe related, but not the same.

So still worth putting under "See Also" then.
Comment 11 Jonathan Wakely 2018-08-20 19:35:42 UTC
(In reply to Jonathan Wakely from comment #4)
> As pointed out in PR 52901 comment 3, this missing warning is likely to bite
> people misusing std::move like so:
> 
> X&& f()
> {
>   X x;
>   return std::move(x);
> }

I created PR 86982 for the special cases of std::move and std::forward, as I think they're important.

This bug is for the more general case.
Comment 12 Eric Gallager 2018-11-20 13:37:14 UTC
(In reply to Jonathan Wakely from comment #11)
> (In reply to Jonathan Wakely from comment #4)
> > As pointed out in PR 52901 comment 3, this missing warning is likely to bite
> > people misusing std::move like so:
> > 
> > X&& f()
> > {
> >   X x;
> >   return std::move(x);
> > }
> 
> I created PR 86982 for the special cases of std::move and std::forward, as I
> think they're important.
> 
> This bug is for the more general case.

86982 requested the warning go under -Wreturn-local-addr; is that what this one would go under, too?
Comment 13 Jonathan Wakely 2018-11-21 10:04:36 UTC
Yes, I think so. The bug is returning a reference (indirectly) bound to a local, so I don't see a reason to have a different warning.
Comment 14 Eric Gallager 2018-11-21 11:57:31 UTC
(In reply to Jonathan Wakely from comment #13)
> Yes, I think so. The bug is returning a reference (indirectly) bound to a
> local, so I don't see a reason to have a different warning.

ok, updating title
Comment 15 Martin Sebor 2019-09-24 15:32:53 UTC
GCC 10 issues two warnings for the first test case in comment #0:

pr49974.C: In function ‘const X& g()’:
pr49974.C:8:15: warning: function returns address of local variable [-Wreturn-local-addr]
    8 |     return f(x);  // !!!
      |               ^
pr49974.C:7:7: note: declared here
    7 |     X x;
      |       ^
In function ‘const X& h()’:
cc1plus: warning: function returns address of local variable [-Wreturn-local-addr]
pr49974.C:7:7: note: declared here


The second test case is not diagnosed (due to a similar problem as pr90905).  The test cases in comment #8  are also not diagnosed.
Comment 16 Jonathan Wakely 2020-06-26 13:48:04 UTC
*** Bug 95911 has been marked as a duplicate of this bug. ***
Comment 17 Jonathan Wakely 2020-06-26 13:54:57 UTC
95911 boils down to this:

#include <utility>

struct X { int i = 0; };

X&& f(X&& x) { return std::move(x); }

int main()
{
  X&& x = f(X{});
  return x.i;
}

With recent releases we get a -Wuninitialized warning, which isn't entirely correct, but at least suggests a problem:

49974.cc: In function 'int main()':
49974.cc:10:12: warning: '<anonymous>.X::i' is used uninitialized [-Wuninitialized]
   10 |   return x.i;
      |            ^
49974.cc:9:15: note: '<anonymous>' declared here
    9 |   X&& x = f(X{});
      |               ^

(the note is new in GCC 11).

With -fsanitize=address we get a nice stack-use-after-scope error:

==3088273==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fff976ed520 at pc 0x000000401100 bp 0x7fff976ed4f0 sp 0x7fff976ed4e8
READ of size 4 at 0x7fff976ed520 thread T0
    #0 0x4010ff in main /tmp/49974.cc:10
    #1 0x7fd157d181a2 in __libc_start_main ../csu/libc-start.c:308
    #2 0x40116d in _start (/tmp/a.out+0x40116d)