[Bug c++/108165] New: -Wdangling-reference false positive

[Bug c++/108165] New: -Wdangling-reference false positive

[romain.geissler at amadeus dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

            Bug ID: 108165
           Summary: -Wdangling-reference false positive
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: romain.geissler at amadeus dot com
  Target Milestone: ---

Hi,

The following snippet issues a wrong -Wdangling-reference warning when compiled
with -Wall with current gcc trunk:

<<END_OF_FILE
struct A {};

struct B
{
    B(int) {}
};

const A& f(const A& a, const B&) {
    return a;
}

const A& g(const A& a) {
    const A& result = f(a, 42);

    return result;
}
END_OF_FILE

The warning is:

<source>: In function 'const A& g(const A&)':
<source>:13:14: warning: possibly dangling reference to a temporary
[-Wdangling-reference]
   13 |     const A& result = f(a, 42);
      |              ^~~~~~
<source>:13:24: note: the temporary was destroyed at the end of the full
expression 'f((* & a), B(42))'
   13 |     const A& result = f(a, 42);
      |                       ~^~~~~~~
ASM generation compiler returned: 0
<source>: In function 'const A& g(const A&)':
<source>:13:14: warning: possibly dangling reference to a temporary
[-Wdangling-reference]
   13 |     const A& result = f(a, 42);
      |              ^~~~~~
<source>:13:24: note: the temporary was destroyed at the end of the full
expression 'f((* & a), B(42))'
   13 |     const A& result = f(a, 42);
      |                       ~^~~~~~~

[Bug c++/108165] -Wdangling-reference false positive

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So at the warning is not flow sensitive at all and does not take into account
the definition of f; only the call location of f is taken into account.

In this case, the call site of f has a temporary and the warning is saying
possibly because a temporary is made in the call arguments of f.

[Bug c++/108165] -Wdangling-reference false positive

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
or is it because of the different types? GCC does not look into that either and
does not look into if they are castable either ...

[Bug c++/108165] -Wdangling-reference false positive

[romain.geissler at amadeus dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #3 from Romain Geissler <romain.geissler at amadeus dot com> ---
In my real life case B was std::string and used a "string literal" at call
site, and I guess using the implicit conversion from const char[] to
std::string is something that might happen in many call sites in big code
bases.

Is it expected that -Wdangling-reference doesn't take into account the
definition of f ? The problem of dangling reference in general needs function
definitions to be effective, otherwise I fear there might be quite some false
positives.

[Bug c++/108165] -Wdangling-reference false positive

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-12-19
     Ever confirmed|0                           |1

[Bug c++/108165] -Wdangling-reference false positive

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Romain Geissler from comment #3)
> In my real life case B was std::string and used a "string literal" at call
> site, and I guess using the implicit conversion from const char[] to
> std::string is something that might happen in many call sites in big code
> bases.

And in general that is not safe.

const std::string& f(const std::string& s) { return s; }
const std::string& s = f(""); // BUG

Warning here would be entirely correct. A temporary string is created from the
string literal, then a reference to that temporary is returned, and bound to
another reference.
This is a dangling reference, and a serious bug, and exactly what the new
warning is designed to diagnose.

> Is it expected that -Wdangling-reference doesn't take into account the
> definition of f ?

Yes.

> The problem of dangling reference in general needs
> function definitions to be effective, otherwise I fear there might be quite
> some false positives.

Yes, but not in a case like f(const std::string&) above. The warning is correct
in most real cases.

The situation for the code in comment 0 is different though, there is no A
temporary. The temporary is the second argument of type B, and that isn't
returned. This seems like a bug in the implementation of the warning's
heuristics, not a problem with the design of the warning.

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Sorry, I'm not sure if the false positive in comment 0 can be fixed.  I can't
simply compare the type of the temporary argument and the return type, because
we may be returning a subobject of the temporary argument, which is still
dangerous.

[Bug c++/108165] -Wdangling-reference false positive

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #5)
> Sorry, I'm not sure if the false positive in comment 0 can be fixed.  I
> can't simply compare the type of the temporary argument and the return type,
> because we may be returning a subobject of the temporary argument, which is
> still dangerous.

Well, you could e.g. walk recursively all the TYPE_FIELDS of the type of the
temporary
and compare to the type the return type references.  If the temporary has some
subobject of that type or if the temporary itself has that type, warn,
otherwise not warn.
There would still be false positives and false negatives, but those are without
actually looking at the definition unavoidable.

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Sure, I could (lookup_member?).  It's still just guessing but maybe it would be
worth it.  Let me try...

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #8 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(For std::any et al I guess we also have to look for void*.)

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #9 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
But even that won't work for Wdangling-reference6.C where the argtype is int
but the rettype is std::pair<const int&, const int&>.

Really, all I could do is to warn only when all the arguments to the function
returning a reference are temporaries, e.g. don't warn on

f(a, a);
f(a, 42);
f(42, a);

only on

f(42, 42);

[Bug c++/108165] -Wdangling-reference false positive

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #10 from Martin Liška <marxin at gcc dot gnu.org> ---
@Marek, is there any progress on this?

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #11 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
No, because as Comment 9 says, there's no good way to suppress the warning. 
I'm currently leaning towards closing the BZ and suggesting adding a #pragma to
disable the warning.

[Bug c++/108165] -Wdangling-reference false positive

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #12 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #11)
> No, because as Comment 9 says, there's no good way to suppress the warning. 
> I'm currently leaning towards closing the BZ and suggesting adding a #pragma
> to disable the warning.

Another solution would be the removal of the warning from -Wall?

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #13 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I would really not like to do that, the false positives rate is pretty low.

[Bug c++/108165] -Wdangling-reference false positive

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

--- Comment #14 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #13)
> I would really not like to do that, the false positives rate is pretty low.

You right! I noticed the warning for about 3 packages of 3300 we have in a
testing CI.

[Bug c++/108165] -Wdangling-reference false positive

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #15 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I'm sorry but I think I'm going to close this PR without attempting to deal
with the false positive in the compiler.  Please add a #pragma around the
function that provokes this warning.  Sorry again.

[Bug c++/108165] -Wdangling-reference false positive

[mrsam@courier-mta.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108165

Sam Varshavchik <mrsam@courier-mta.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mrsam@courier-mta.com

--- Comment #16 from Sam Varshavchik <mrsam@courier-mta.com> ---
*** Bug 109538 has been marked as a duplicate of this bug. ***