Re: [PATCH] coroutines : Remove throwing_cleanup marks from the ramp [PR95822].
Jason Merrill wrote:
On 2/26/21 4:36 PM, Iain Sandoe wrote:
Jason Merrill wrote:
On 2/24/21 3:06 PM, Iain Sandoe wrote:
So (a) the 'throwing_cleanup' is no longer correct for the ramp and
(b) we do not need to transfer it to the actor which only contains
void returns.
OK, but I wonder if there are other things that should also be reset.
That would be believable but, absent problem reports, have you any advice
on how/what to audit?
Looking at language_function, I guess it would make sense to clear
returns_abnormally and infinite_loop{,s}, maybe bindings.
thanks, I’ll take a look.
Since you're completely replacing the function body, almost none of the
information we previously recorded about it in cfun is accurate.
Yes, and maybe no longer accurate about the body when transferred to the
actor function [like the throwing_cleanup] (but it’s worth considering the
other cases anyway).
But it may be harmless.
I’d be inclined to null stuff out if it’s definitively unneeded - so will
check some more.
thanks
Iain
Re: [PATCH] coroutines : Remove throwing_cleanup marks from the ramp [PR95822].
On 2/26/21 4:36 PM, Iain Sandoe wrote:
Jason Merrill wrote:
On 2/24/21 3:06 PM, Iain Sandoe wrote:
The FE contains a mechanism for cleaning up return expressions if a
function throws during the execution of cleanups prior to the return.
If the original function has a return value with a non-trivial DTOR
and the body contains a var with a DTOR that might throw, the function
decl is marked "throwing_cleanup".
However, we do not [in the coroutine ramp function, which is
synthesised], use any body var types with DTORs that might throw.
The original body [which will then contain the type with the throwing
DTOR] is transformed into the actor function which only contains void
returns, and is also wrapped in a try-catch block.
So (a) the 'throwing_cleanup' is no longer correct for the ramp and
(b) we do not need to transfer it to the actor which only contains
void returns.
this is an ICE-on-valid,
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / 10.x ?
OK, but I wonder if there are other things that should also be reset.
That would be believable but, absent problem reports, have you any advice
on how/what to audit?
Looking at language_function, I guess it would make sense to clear
returns_abnormally and infinite_loop{,s}, maybe bindings. Since you're
completely replacing the function body, almost none of the information
we previously recorded about it in cfun is accurate. But it may be
harmless.
Jason
Re: [PATCH] coroutines : Remove throwing_cleanup marks from the ramp [PR95822].
Jason Merrill wrote: On 2/24/21 3:06 PM, Iain Sandoe wrote: The FE contains a mechanism for cleaning up return expressions if a function throws during the execution of cleanups prior to the return. If the original function has a return value with a non-trivial DTOR and the body contains a var with a DTOR that might throw, the function decl is marked "throwing_cleanup". However, we do not [in the coroutine ramp function, which is synthesised], use any body var types with DTORs that might throw. The original body [which will then contain the type with the throwing DTOR] is transformed into the actor function which only contains void returns, and is also wrapped in a try-catch block. So (a) the 'throwing_cleanup' is no longer correct for the ramp and (b) we do not need to transfer it to the actor which only contains void returns. this is an ICE-on-valid, tested on x86_64-darwin, x86_64-linux-gnu, OK for master / 10.x ? OK, but I wonder if there are other things that should also be reset. That would be believable but, absent problem reports, have you any advice on how/what to audit? thanks Iain
Re: [PATCH] coroutines : Remove throwing_cleanup marks from the ramp [PR95822].
On 2/24/21 3:06 PM, Iain Sandoe wrote:
Hi,
The FE contains a mechanism for cleaning up return expressions if a
function throws during the execution of cleanups prior to the return.
If the original function has a return value with a non-trivial DTOR
and the body contains a var with a DTOR that might throw, the function
decl is marked "throwing_cleanup".
However, we do not [in the coroutine ramp function, which is
synthesised], use any body var types with DTORs that might throw.
The original body [which will then contain the type with the throwing
DTOR] is transformed into the actor function which only contains void
returns, and is also wrapped in a try-catch block.
So (a) the 'throwing_cleanup' is no longer correct for the ramp and
(b) we do not need to transfer it to the actor which only contains
void returns.
this is an ICE-on-valid,
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / 10.x ?
OK, but I wonder if there are other things that should also be reset.
thanks
Iain
gcc/cp/ChangeLog:
PR c++/95822
* coroutines.cc (morph_fn_to_coro): Unconditionally remove any
set throwing_cleanup marker.
gcc/testsuite/ChangeLog:
PR c++/95822
* g++.dg/coroutines/pr95822.C: New test.
---
gcc/cp/coroutines.cc | 11 +
gcc/testsuite/g++.dg/coroutines/pr95822.C | 29 +++
2 files changed, 40 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95822.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index abfe8d08192..19d2ca3e23e 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4029,6 +4029,17 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
*destroyer)
TREE_OPERAND (body_start, 0) = push_stmt_list ();
}
+ /* If the original function has a return value with a non-trivial DTOR
+ and the body contains a var with a DTOR that might throw, the decl is
+ marked "throwing_cleanup".
+ We do not [in the ramp, which is synthesised here], use any body var
+ types with DTORs that might throw.
+ The original body is transformed into the actor function which only
+ contains void returns, and is also wrapped in a try-catch block.
+ So (a) the 'throwing_cleanup' is not correct for the ramp and (b) we do
+ not need to transfer it to the actor which only contains void returns. */
+ cp_function_chain->throwing_cleanup = false;
+
/* Create the coro frame type, as far as it can be known at this stage.
1. Types we already know. */
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95822.C b/gcc/testsuite/g++.dg/coroutines/pr95822.C
new file mode 100644
index 000..f6284aa417e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95822.C
@@ -0,0 +1,29 @@
+#include
+
+struct task {
+ struct promise_type {
+auto initial_suspend() noexcept { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+void return_void() {}
+task get_return_object() { return task{}; }
+void unhandled_exception() noexcept {}
+ };
+
+ ~task() noexcept {}
+
+ bool await_ready() const noexcept { return false; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ void await_resume() noexcept {}
+};
+
+struct Error {
+ Error() { };
+ ~Error() noexcept(false) {}
+};
+
+task g();
+
+task f() {
+ Error error;
+ co_await g();
+}
[PATCH] coroutines : Remove throwing_cleanup marks from the ramp [PR95822].
Hi,
The FE contains a mechanism for cleaning up return expressions if a
function throws during the execution of cleanups prior to the return.
If the original function has a return value with a non-trivial DTOR
and the body contains a var with a DTOR that might throw, the function
decl is marked "throwing_cleanup".
However, we do not [in the coroutine ramp function, which is
synthesised], use any body var types with DTORs that might throw.
The original body [which will then contain the type with the throwing
DTOR] is transformed into the actor function which only contains void
returns, and is also wrapped in a try-catch block.
So (a) the 'throwing_cleanup' is no longer correct for the ramp and
(b) we do not need to transfer it to the actor which only contains
void returns.
this is an ICE-on-valid,
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / 10.x ?
thanks
Iain
gcc/cp/ChangeLog:
PR c++/95822
* coroutines.cc (morph_fn_to_coro): Unconditionally remove any
set throwing_cleanup marker.
gcc/testsuite/ChangeLog:
PR c++/95822
* g++.dg/coroutines/pr95822.C: New test.
---
gcc/cp/coroutines.cc | 11 +
gcc/testsuite/g++.dg/coroutines/pr95822.C | 29 +++
2 files changed, 40 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95822.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index abfe8d08192..19d2ca3e23e 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4029,6 +4029,17 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
*destroyer)
TREE_OPERAND (body_start, 0) = push_stmt_list ();
}
+ /* If the original function has a return value with a non-trivial DTOR
+ and the body contains a var with a DTOR that might throw, the decl is
+ marked "throwing_cleanup".
+ We do not [in the ramp, which is synthesised here], use any body var
+ types with DTORs that might throw.
+ The original body is transformed into the actor function which only
+ contains void returns, and is also wrapped in a try-catch block.
+ So (a) the 'throwing_cleanup' is not correct for the ramp and (b) we do
+ not need to transfer it to the actor which only contains void returns. */
+ cp_function_chain->throwing_cleanup = false;
+
/* Create the coro frame type, as far as it can be known at this stage.
1. Types we already know. */
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95822.C
b/gcc/testsuite/g++.dg/coroutines/pr95822.C
new file mode 100644
index 000..f6284aa417e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95822.C
@@ -0,0 +1,29 @@
+#include
+
+struct task {
+ struct promise_type {
+auto initial_suspend() noexcept { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+void return_void() {}
+task get_return_object() { return task{}; }
+void unhandled_exception() noexcept {}
+ };
+
+ ~task() noexcept {}
+
+ bool await_ready() const noexcept { return false; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ void await_resume() noexcept {}
+};
+
+struct Error {
+ Error() { };
+ ~Error() noexcept(false) {}
+};
+
+task g();
+
+task f() {
+ Error error;
+ co_await g();
+}
--
2.24.1
