]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Reject tail calls that read from an escaped RESULT_DECL [PR90313]
authorRichard Sandiford <richard.sandiford@arm.com>
Fri, 9 Aug 2019 09:37:55 +0000 (09:37 +0000)
committerRichard Sandiford <richard.sandiford@arm.com>
Tue, 18 Feb 2020 12:26:02 +0000 (12:26 +0000)
In this PR we have two return paths from a function "map".  The common
code sets <result> to the value returned by one path, while the other
path does:

   <retval> = map (&<retval>, ...);

We treated this call as tail recursion, losing the copy semantics
on the value returned by the recursive call.

We'd correctly reject the same thing for variables:

   local = map (&local, ...);

The problem is that RESULT_DECLs didn't get the same treatment.

2020-02-18  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
Backport from mainline
2019-08-09  Richard Sandiford  <richard.sandiford@arm.com>

PR middle-end/90313
* tree-tailcall.c (find_tail_calls): Reject calls that might
read from an escaped RESULT_DECL.

gcc/testsuite/
PR middle-end/90313
* g++.dg/torture/pr90313.cc: New test.

gcc/ChangeLog
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/torture/pr90313.cc [new file with mode: 0644]
gcc/tree-tailcall.c

index 3aca4c50e4ad4c150c7d25c35047ad28f506ff8c..874de008609f63477afc668e70f4bab96c395b05 100644 (file)
@@ -1,3 +1,12 @@
+2020-02-18  Richard Sandiford  <richard.sandiford@arm.com>
+
+       Backport from mainline
+       2019-08-09  Richard Sandiford  <richard.sandiford@arm.com>
+
+       PR middle-end/90313
+       * tree-tailcall.c (find_tail_calls): Reject calls that might
+       read from an escaped RESULT_DECL.
+
 2020-02-18  Richard Sandiford  <richard.sandiford@arm.com>
 
        Backport from mainline
index 81926efcbbdb5bc9b01fe2a5f1588b6caaf80fb3..d02ad44ceff2be252dba5fff6a444bec20da93bb 100644 (file)
@@ -1,3 +1,8 @@
+2020-02-18  Richard Sandiford  <richard.sandiford@arm.com>
+
+       PR middle-end/90313
+       * g++.dg/torture/pr90313.cc: New test.
+
 2020-02-18  Mark Eggleston <markeggleston@gcc.gnu.org>
 
        Back-ported from mainline
diff --git a/gcc/testsuite/g++.dg/torture/pr90313.cc b/gcc/testsuite/g++.dg/torture/pr90313.cc
new file mode 100644 (file)
index 0000000..d9f183a
--- /dev/null
@@ -0,0 +1,33 @@
+// { dg-do run }
+
+#include <stddef.h>
+
+namespace std {
+  template<typename T, size_t N> struct array {
+    T elems[N];
+    const T &operator[](size_t i) const { return elems[i]; }
+  };
+}
+
+using Coordinates = std::array<double, 3>;
+
+Coordinates map(const Coordinates &c, size_t level)
+{
+  Coordinates result{ c[1], c[2], c[0] };
+
+  if (level != 0)
+    result = map (result, level - 1);
+
+  return result;
+}
+
+int main()
+{
+  Coordinates vecOfCoordinates = { 1.0, 2.0, 3.0 };
+
+  auto result = map(vecOfCoordinates, 1);
+  if (result[0] != 3 || result[1] != 1 || result[2] != 2)
+    __builtin_abort ();
+
+  return 0;
+}
index e0265b22dd5f85c91a004fbb813ffd89d311a2ec..255575f1198aeeb7504d68af490adc15c57b62ce 100644 (file)
@@ -479,6 +479,35 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
       && !stmt_can_throw_external (cfun, stmt))
     return;
 
+  /* If the function returns a value, then at present, the tail call
+     must return the same type of value.  There is conceptually a copy
+     between the object returned by the tail call candidate and the
+     object returned by CFUN itself.
+
+     This means that if we have:
+
+        lhs = f (&<retval>);    // f reads from <retval>
+                                // (lhs is usually also <retval>)
+
+     there is a copy between the temporary object returned by f and lhs,
+     meaning that any use of <retval> in f occurs before the assignment
+     to lhs begins.  Thus the <retval> that is live on entry to the call
+     to f is really an independent local variable V that happens to be
+     stored in the RESULT_DECL rather than a local VAR_DECL.
+
+     Turning this into a tail call would remove the copy and make the
+     lifetimes of the return value and V overlap.  The same applies to
+     tail recursion, since if f can read from <retval>, we have to assume
+     that CFUN might already have written to <retval> before the call.
+
+     The problem doesn't apply when <retval> is passed by value, but that
+     isn't a case we handle anyway.  */
+  tree result_decl = DECL_RESULT (cfun->decl);
+  if (result_decl
+      && may_be_aliased (result_decl)
+      && ref_maybe_used_by_stmt_p (call, result_decl))
+    return;
+
   /* We found the call, check whether it is suitable.  */
   tail_recursion = false;
   func = gimple_call_fndecl (call);