[C++] Fix -Wreturn-local-addr handling of structured bindings (PR c++/87582)

Message ID 20181011125237.GZ11625@tucnak
State New
Headers show
Series
  • [C++] Fix -Wreturn-local-addr handling of structured bindings (PR c++/87582)
Related show

Commit Message

Jakub Jelinek Oct. 11, 2018, 12:52 p.m.
Hi!

Except for std::tuple* structured bindings, the VAR_DECLs we create for the
identifiers aren't actually variables, but placeholders with
DECL_VALUE_EXPR.  If the structured binding is not a reference, it is still
an automatic variable and so -Wreturn-local-addr should warn on those,
but if it is a reference, then it depends if it references an automatic
variable or something else.

The following patch handles it by recursing for references on the
initializer of the structured binding.  Note we don't just emit incorrect
warning without this patch, but the caller replaces return something;
with return (something, 0); if maybe_warn_about_returning_address_of_local
returns true, so it is also invalid at runtime.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
release branches?

2018-10-11  Jakub Jelinek  <jakub@redhat.com>

	PR c++/87582
	* typeck.c (maybe_warn_about_returning_address_of_local): If
	whats_returned is a structured binding identifier and the structured
	binding is a reference, recurse on its initializer.

	* g++.dg/cpp1z/decomp48.C: New test.


	Jakub

Comments

Jason Merrill Oct. 11, 2018, 1:37 p.m. | #1
OK.
On Thu, Oct 11, 2018 at 8:52 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> Except for std::tuple* structured bindings, the VAR_DECLs we create for the
> identifiers aren't actually variables, but placeholders with
> DECL_VALUE_EXPR.  If the structured binding is not a reference, it is still
> an automatic variable and so -Wreturn-local-addr should warn on those,
> but if it is a reference, then it depends if it references an automatic
> variable or something else.
>
> The following patch handles it by recursing for references on the
> initializer of the structured binding.  Note we don't just emit incorrect
> warning without this patch, but the caller replaces return something;
> with return (something, 0); if maybe_warn_about_returning_address_of_local
> returns true, so it is also invalid at runtime.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> release branches?
>
> 2018-10-11  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/87582
>         * typeck.c (maybe_warn_about_returning_address_of_local): If
>         whats_returned is a structured binding identifier and the structured
>         binding is a reference, recurse on its initializer.
>
>         * g++.dg/cpp1z/decomp48.C: New test.
>
> --- gcc/cp/typeck.c.jj  2018-09-13 09:27:31.547765011 +0200
> +++ gcc/cp/typeck.c     2018-10-11 10:06:36.820295475 +0200
> @@ -9096,6 +9096,22 @@ maybe_warn_about_returning_address_of_lo
>        && !(TREE_STATIC (whats_returned)
>            || TREE_PUBLIC (whats_returned)))
>      {
> +      if (VAR_P (whats_returned)
> +         && DECL_DECOMPOSITION_P (whats_returned)
> +         && DECL_DECOMP_BASE (whats_returned)
> +         && DECL_HAS_VALUE_EXPR_P (whats_returned))
> +       {
> +         /* When returning address of a structured binding, if the structured
> +            binding is not a reference, continue normally, if it is a
> +            reference, recurse on the initializer of the structured
> +            binding.  */
> +         tree base = DECL_DECOMP_BASE (whats_returned);
> +         if (TYPE_REF_P (TREE_TYPE (base)))
> +           {
> +             tree init = DECL_INITIAL (base);
> +             return maybe_warn_about_returning_address_of_local (init);
> +           }
> +       }
>        bool w = false;
>        auto_diagnostic_group d;
>        if (TYPE_REF_P (valtype))
> --- gcc/testsuite/g++.dg/cpp1z/decomp48.C.jj    2018-10-11 10:30:09.255651339 +0200
> +++ gcc/testsuite/g++.dg/cpp1z/decomp48.C       2018-10-11 11:00:23.210283412 +0200
> @@ -0,0 +1,134 @@
> +// PR c++/87582
> +// { dg-do run { target c++11 } }
> +// { dg-options "-Wreturn-local-addr" }
> +
> +struct S { int s, t; };
> +S v {1, 2};
> +int a[3] = {1, 2, 3};
> +
> +int &
> +f1 ()
> +{
> +  auto& [s, t] = v;    // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-bogus "reference to local variable '.' returned" }
> +}
> +
> +int &
> +f2 ()
> +{
> +  S v {1, 2};
> +  auto& [s, t] = v;    // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-warning "reference to local variable 'v' returned" }
> +}
> +
> +int &
> +f3 ()
> +{
> +  auto& [s, t, u] = a; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-bogus "reference to local variable '.' returned" }
> +}
> +
> +int &
> +f4 ()
> +{
> +  int a[3] = {1, 2, 3};
> +  auto& [s, t, u] = a; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-warning "reference to local variable 'a' returned" }
> +}
> +
> +int &
> +f5 ()
> +{
> +  auto [s, t] = v;     // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-warning "reference to local variable 's' returned" }
> +}
> +
> +int &
> +f6 ()
> +{
> +  S v {1, 2};
> +  auto [s, t] = v;     // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-warning "reference to local variable 's' returned" }
> +}
> +
> +int &
> +f7 ()
> +{
> +  auto [s, t, u] = a;  // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-warning "reference to local variable 's' returned" }
> +}
> +
> +int &
> +f8 ()
> +{
> +  int a[3] = {1, 2, 3};
> +  auto [s, t, u] = a;  // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return s;            // { dg-warning "reference to local variable 's' returned" }
> +}
> +
> +int *
> +f9 ()
> +{
> +  auto& [s, t] = v;    // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-bogus "address of local variable '.' returned" }
> +}
> +
> +int *
> +f10 ()
> +{
> +  S v {1, 2};
> +  auto& [s, t] = v;    // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-warning "address of local variable 'v' returned" }
> +}
> +
> +int *
> +f11 ()
> +{
> +  auto& [s, t, u] = a; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-bogus "address of local variable '.' returned" }
> +}
> +
> +int *
> +f12 ()
> +{
> +  int a[3] = {1, 2, 3};
> +  auto& [s, t, u] = a; // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-warning "address of local variable 'a' returned" }
> +}
> +
> +int *
> +f13 ()
> +{
> +  auto [s, t] = v;     // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-warning "address of local variable 's' returned" }
> +}
> +
> +int *
> +f14 ()
> +{
> +  S v {1, 2};
> +  auto [s, t] = v;     // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-warning "address of local variable 's' returned" }
> +}
> +
> +int *
> +f15 ()
> +{
> +  auto [s, t, u] = a;  // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-warning "address of local variable 's' returned" }
> +}
> +
> +int *
> +f16 ()
> +{
> +  int a[3] = {1, 2, 3};
> +  auto [s, t, u] = a;  // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return &s;           // { dg-warning "address of local variable 's' returned" }
> +}
> +
> +int
> +main ()
> +{
> +  if (&f1 () != &v.s || &f3 () != &a[0] || f9 () != &v.s || f11 () != &a[0])
> +    __builtin_abort ();
> +}
>
>         Jakub

Patch

--- gcc/cp/typeck.c.jj	2018-09-13 09:27:31.547765011 +0200
+++ gcc/cp/typeck.c	2018-10-11 10:06:36.820295475 +0200
@@ -9096,6 +9096,22 @@  maybe_warn_about_returning_address_of_lo
       && !(TREE_STATIC (whats_returned)
 	   || TREE_PUBLIC (whats_returned)))
     {
+      if (VAR_P (whats_returned)
+	  && DECL_DECOMPOSITION_P (whats_returned)
+	  && DECL_DECOMP_BASE (whats_returned)
+	  && DECL_HAS_VALUE_EXPR_P (whats_returned))
+	{
+	  /* When returning address of a structured binding, if the structured
+	     binding is not a reference, continue normally, if it is a
+	     reference, recurse on the initializer of the structured
+	     binding.  */
+	  tree base = DECL_DECOMP_BASE (whats_returned);
+	  if (TYPE_REF_P (TREE_TYPE (base)))
+	    {
+	      tree init = DECL_INITIAL (base);
+	      return maybe_warn_about_returning_address_of_local (init);
+	    }
+	}
       bool w = false;
       auto_diagnostic_group d;
       if (TYPE_REF_P (valtype))
--- gcc/testsuite/g++.dg/cpp1z/decomp48.C.jj	2018-10-11 10:30:09.255651339 +0200
+++ gcc/testsuite/g++.dg/cpp1z/decomp48.C	2018-10-11 11:00:23.210283412 +0200
@@ -0,0 +1,134 @@ 
+// PR c++/87582
+// { dg-do run { target c++11 } }
+// { dg-options "-Wreturn-local-addr" }
+
+struct S { int s, t; };
+S v {1, 2};
+int a[3] = {1, 2, 3};
+
+int &
+f1 ()
+{
+  auto& [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-bogus "reference to local variable '.' returned" }
+}
+
+int &
+f2 ()
+{
+  S v {1, 2};
+  auto& [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-warning "reference to local variable 'v' returned" }
+}
+
+int &
+f3 ()
+{
+  auto& [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-bogus "reference to local variable '.' returned" }
+}
+
+int &
+f4 ()
+{
+  int a[3] = {1, 2, 3};
+  auto& [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-warning "reference to local variable 'a' returned" }
+}
+
+int &
+f5 ()
+{
+  auto [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-warning "reference to local variable 's' returned" }
+}
+
+int &
+f6 ()
+{
+  S v {1, 2};
+  auto [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-warning "reference to local variable 's' returned" }
+}
+
+int &
+f7 ()
+{
+  auto [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-warning "reference to local variable 's' returned" }
+}
+
+int &
+f8 ()
+{
+  int a[3] = {1, 2, 3};
+  auto [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return s;		// { dg-warning "reference to local variable 's' returned" }
+}
+
+int *
+f9 ()
+{
+  auto& [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-bogus "address of local variable '.' returned" }
+}
+
+int *
+f10 ()
+{
+  S v {1, 2};
+  auto& [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-warning "address of local variable 'v' returned" }
+}
+
+int *
+f11 ()
+{
+  auto& [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-bogus "address of local variable '.' returned" }
+}
+
+int *
+f12 ()
+{
+  int a[3] = {1, 2, 3};
+  auto& [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-warning "address of local variable 'a' returned" }
+}
+
+int *
+f13 ()
+{
+  auto [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-warning "address of local variable 's' returned" }
+}
+
+int *
+f14 ()
+{
+  S v {1, 2};
+  auto [s, t] = v;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-warning "address of local variable 's' returned" }
+}
+
+int *
+f15 ()
+{
+  auto [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-warning "address of local variable 's' returned" }
+}
+
+int *
+f16 ()
+{
+  int a[3] = {1, 2, 3};
+  auto [s, t, u] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return &s;		// { dg-warning "address of local variable 's' returned" }
+}
+
+int
+main ()
+{
+  if (&f1 () != &v.s || &f3 () != &a[0] || f9 () != &v.s || f11 () != &a[0])
+    __builtin_abort ();
+}