c++, coroutines: Handle allocation fail returns [PR121219].

The current implementation was returning the result of the g_r_o_o_a_f
call independently of the return expressions for 'normal' cases.

This prevents the NVRO that we need to guarantee copy elision for the
ramp return values - when these are initialised from a temporary of the
same type.

The solution here reorders the code so that the regular return expression
appears before the allocation-failed case.  Ensure that the g_r_o and
associated code appears in a distinct scope.  These steps are to meet the
constaints of NRV.

	PR c++/121219

gcc/cp/ChangeLog:

	* coroutines.cc
	(cp_coroutine_transform::build_ramp_function): Reorder the return
	expressions for the 'normal' and 'allocation failed' cases so that
	NRV constraints are met.

gcc/testsuite/ChangeLog:

	* g++.dg/coroutines/torture/pr121219.C: New test.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
(cherry picked from commit a2775feb7c7de9f21f79052e2b6a752a3eb08f07)

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 306424c22b021e0fd0b187e41424c87e7fd1f309..3fe22ac98024caaf85f65322d6cd09825e7c86ee 100644 (file)
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -5066,6 +5066,8 @@ cp_coroutine_transform::build_ramp_function ()
      check the returned pointer and call the func if it's null.
      Otherwise, no check, and we fail for noexcept/fno-exceptions cases.  */
 
+  tree grooaf_if_stmt = NULL_TREE;
+  tree alloc_ok_scope = NULL_TREE;
   if (grooaf)
     {
       /* [dcl.fct.def.coroutine] / 10 (part 3)
@@ -5073,20 +5075,11 @@ cp_coroutine_transform::build_ramp_function ()
         control to the caller of the coroutine and the return value is
         obtained by a call to T::get_return_object_on_allocation_failure(),
         where T is the promise type.  */
-      tree if_stmt = begin_if_stmt ();
       tree cond = build1 (CONVERT_EXPR, frame_ptr_type, nullptr_node);
-      cond = build2 (EQ_EXPR, boolean_type_node, coro_fp, cond);
-      finish_if_stmt_cond (cond, if_stmt);
-      r = NULL_TREE;
-      if (void_ramp_p)
-       /* Execute the get-return-object-on-alloc-fail call...  */
-       finish_expr_stmt (grooaf);
-      else
-       /* Get the fallback return object.  */
-       r = grooaf;
-      finish_return_stmt (r);
-      finish_then_clause (if_stmt);
-      finish_if_stmt (if_stmt);
+      cond = build2 (NE_EXPR, boolean_type_node, coro_fp, cond);
+      grooaf_if_stmt = begin_if_stmt ();
+      finish_if_stmt_cond (cond, grooaf_if_stmt);
+      alloc_ok_scope = begin_compound_stmt (BCS_NORMAL);
     }
 
   /* Dereference the frame pointer, to use in member access code.  */
@@ -5311,7 +5304,6 @@ cp_coroutine_transform::build_ramp_function ()
         a temp which is then used to intialize the return object, including
         NVRO.  */
 
-      /* Temporary var to hold the g_r_o across the function body.  */
       coro_gro
        = coro_build_and_push_artificial_var (loc, "_Coro_gro", gro_type,
                                              orig_fn_decl, NULL_TREE);
@@ -5344,9 +5336,28 @@ cp_coroutine_transform::build_ramp_function ()
   /* The ramp is done, we just need the return statement, which we build from
      the return object we constructed before we called the actor.  */
 
+  /* This is our 'normal' exit.  */
   r = void_ramp_p ? NULL_TREE : convert_from_reference (coro_gro);
   finish_return_stmt (r);
 
+  if (grooaf)
+    {
+      finish_compound_stmt (alloc_ok_scope);
+      finish_then_clause (grooaf_if_stmt);
+
+      begin_else_clause (grooaf_if_stmt);
+      /* We come here if the frame allocation failed.  */
+      r = NULL_TREE;
+      if (void_ramp_p)
+       /* Execute the get-return-object-on-alloc-fail call...  */
+       finish_expr_stmt (grooaf);
+      else
+       /* Get the fallback return object.  */
+       r = grooaf;
+      finish_return_stmt (r);
+      finish_if_stmt (grooaf_if_stmt);
+    }
+
   finish_compound_stmt (ramp_fnbody);
   return true;
 }

diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr121219.C b/gcc/testsuite/g++.dg/coroutines/torture/pr121219.C
new file mode 100644 (file)
index 0000000..d1e7cb1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/pr121219.C
@@ -0,0 +1,149 @@
+// PR c++/121219
+// { dg-do run }
+
+#include <coroutine>
+#ifdef OUTPUT
+#include <iostream>
+#endif
+#include <stdexcept>
+
+struct Task {
+    struct promise_type;
+    using handle_type = std::coroutine_handle<promise_type>;
+
+    struct promise_type {
+        Task* task_;
+        int result_;
+
+        static void* operator new(std::size_t size) noexcept {
+            void* p = ::operator new(size, std::nothrow);
+#ifdef OUTPUT
+            std::cerr << "operator new (no arg) " << size << " -> " << p << std::endl;  
+#endif
+            return p;
+        }
+        static void operator delete(void* ptr) noexcept {
+            return ::operator delete(ptr, std::nothrow);
+        }
+#if 1 // change to 0 to fix crash
+        static Task get_return_object_on_allocation_failure() noexcept {
+#ifdef OUTPUT
+            std::cerr << "get_return_object_on_allocation_failure" << std::endl;
+#endif
+            return Task(nullptr);
+        }
+#endif
+
+        auto get_return_object() { 
+#ifdef OUTPUT
+            std::cerr << "get_return_object" << std::endl;
+#endif
+            return Task{handle_type::from_promise(*this)}; 
+        }
+
+        auto initial_suspend() { 
+#ifdef OUTPUT
+            std::cerr << "initial_suspend" << std::endl;
+#endif
+            return std::suspend_always{}; 
+        }
+
+        auto final_suspend() noexcept { 
+#ifdef OUTPUT
+            std::cerr << "final_suspend" << std::endl;
+#endif
+            return std::suspend_never{};  // Coroutine auto-destructs
+        }
+
+        ~promise_type() {
+            if (task_) {
+#ifdef OUTPUT
+                std::cerr << "promise_type destructor: Clearing Task handle" << std::endl;
+#endif
+                task_->h_ = nullptr;
+            }
+        }
+
+        void unhandled_exception() { 
+#ifdef OUTPUT
+            std::cerr << "unhandled_exception" << std::endl;
+#endif
+            std::terminate(); 
+        }
+
+        void return_value(int value) { 
+#ifdef OUTPUT
+            std::cerr << "return_value: " << value << std::endl;
+#endif
+            result_ = value;
+            if (task_) {
+                task_->result_ = value;
+                task_->completed_ = true;
+            }
+        }
+    };
+
+    handle_type h_;
+    int result_;
+    bool completed_ = false;
+
+    Task(handle_type h) : h_(h) {
+#ifdef OUTPUT
+        std::cerr << "Task constructor" << std::endl;
+#endif
+        if (h_) {
+            h_.promise().task_ = this;  // Link promise to Task
+        }
+    }
+
+    ~Task() { 
+#ifdef OUTPUT
+        std::cerr << "~Task destructor" << std::endl;
+#endif
+        // Only destroy handle if still valid (coroutine not completed)
+        if (h_) {
+#ifdef OUTPUT
+            std::cerr << "Destroying coroutine handle" << std::endl;
+#endif
+            h_.destroy(); 
+        }
+    }
+
+    bool done() const { 
+        return completed_ || !h_ || h_.done(); 
+    }
+
+    void resume() { 
+#ifdef OUTPUT
+        std::cerr << "Resuming task" << std::endl;
+#endif
+        if (h_) h_.resume(); 
+    }
+
+    int result() const {
+        if (!done()) throw std::runtime_error("Result not available");
+        return result_;
+    }
+};
+
+Task my_coroutine() {
+#ifdef OUTPUT
+    std::cerr << "Inside my_coroutine" << std::endl;
+#endif
+   co_return 42;
+}
+
+int main() {
+    auto task = my_coroutine();
+    while (!task.done()) {
+#ifdef OUTPUT
+        std::cerr << "Resuming task in main" << std::endl;
+#endif
+        task.resume();
+    }
+#ifdef OUTPUT
+    std::cerr << "Task completed in main, printing result" << std::endl;
+#endif
+    if (task.result() != 42)
+      __builtin_abort ();
+}