* [PATCH] coroutines: Only set parm copy guard vars if we have exceptions [PR 102454].
@ 2021-09-27 19:38 Iain Sandoe
2021-09-28 17:48 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Iain Sandoe @ 2021-09-27 19:38 UTC (permalink / raw)
To: gcc-patches
For coroutines, we make copies of the original function arguments into
the coroutine frame. Normally, these are destroyed on the proper exit
from the coroutine when the frame is destroyed.
However, if an exception is thrown before the first suspend point is
reached, the cleanup has to happen in the ramp function. These cleanups
are guarded such that they are only applied to any param copies actually
made.
The ICE is caused by an attempt to set the guard variable when there are
no exceptions enabled (the guard var is not created in this case).
Fixed by checking for flag_exceptions in this case too.
While touching this code paths, also clean up the synthetic names used
when a function parm is unnamed.
tested on x86_64-darwin,
OK for master?
thanks
Iain
Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
PR c++/102454
gcc/cp/ChangeLog:
* coroutines.cc (analyze_fn_parms): Clean up synthetic names for
unnamed function params.
(morph_fn_to_coro): Do not try to set a guard variable for param
DTORs in the ramp, unless we have exceptions active.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr102454.C: New test.
---
gcc/cp/coroutines.cc | 26 ++++++++-------
gcc/testsuite/g++.dg/coroutines/pr102454.C | 38 ++++++++++++++++++++++
2 files changed, 52 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr102454.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index fbd5c49533f..c761e769c12 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3829,13 +3829,12 @@ analyze_fn_parms (tree orig)
if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (parm.frame_type))
{
- char *buf = xasprintf ("_Coro_%s_live", IDENTIFIER_POINTER (name));
- parm.guard_var = build_lang_decl (VAR_DECL, get_identifier (buf),
- boolean_type_node);
- free (buf);
- DECL_ARTIFICIAL (parm.guard_var) = true;
- DECL_CONTEXT (parm.guard_var) = orig;
- DECL_INITIAL (parm.guard_var) = boolean_false_node;
+ char *buf = xasprintf ("%s%s_live", DECL_NAME (arg) ? "_Coro_" : "",
+ IDENTIFIER_POINTER (name));
+ parm.guard_var
+ = coro_build_artificial_var (UNKNOWN_LOCATION, get_identifier (buf),
+ boolean_type_node, orig,
+ boolean_false_node);
parm.trivial_dtor = false;
}
else
@@ -4843,11 +4842,14 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
NULL, parm.frame_type,
LOOKUP_NORMAL,
tf_warning_or_error);
- /* This var is now live. */
- r = build_modify_expr (fn_start, parm.guard_var,
- boolean_type_node, INIT_EXPR, fn_start,
- boolean_true_node, boolean_type_node);
- finish_expr_stmt (r);
+ if (flag_exceptions)
+ {
+ /* This var is now live. */
+ r = build_modify_expr (fn_start, parm.guard_var,
+ boolean_type_node, INIT_EXPR, fn_start,
+ boolean_true_node, boolean_type_node);
+ finish_expr_stmt (r);
+ }
}
}
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr102454.C b/gcc/testsuite/g++.dg/coroutines/pr102454.C
new file mode 100644
index 00000000000..41aeda7b973
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr102454.C
@@ -0,0 +1,38 @@
+// { dg-additional-options "-fno-exceptions" }
+
+#include <coroutine>
+#include <string>
+
+template <typename T>
+struct looper {
+ struct promise_type {
+ auto get_return_object () { return handle_type::from_promise (*this); }
+ auto initial_suspend () { return suspend_always_prt {}; }
+ auto final_suspend () noexcept { return suspend_always_prt {}; }
+ void return_value (T);
+ void unhandled_exception ();
+ };
+
+ using handle_type = std::coroutine_handle<promise_type>;
+
+ looper (handle_type);
+
+ struct suspend_always_prt {
+ bool await_ready () noexcept;
+ void await_suspend (handle_type) noexcept;
+ void await_resume () noexcept;
+ };
+};
+
+template <typename T>
+looper<T>
+with_ctorable_state (T)
+{
+ co_return T ();
+}
+
+auto
+foo ()
+{
+ return with_ctorable_state<std::string>;
+}
--
2.24.3 (Apple Git-128)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] coroutines: Only set parm copy guard vars if we have exceptions [PR 102454].
2021-09-27 19:38 [PATCH] coroutines: Only set parm copy guard vars if we have exceptions [PR 102454] Iain Sandoe
@ 2021-09-28 17:48 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2021-09-28 17:48 UTC (permalink / raw)
To: iain, gcc-patches
On 9/27/21 15:38, Iain Sandoe wrote:
> For coroutines, we make copies of the original function arguments into
> the coroutine frame. Normally, these are destroyed on the proper exit
> from the coroutine when the frame is destroyed.
>
> However, if an exception is thrown before the first suspend point is
> reached, the cleanup has to happen in the ramp function. These cleanups
> are guarded such that they are only applied to any param copies actually
> made.
>
> The ICE is caused by an attempt to set the guard variable when there are
> no exceptions enabled (the guard var is not created in this case).
>
> Fixed by checking for flag_exceptions in this case too.
>
> While touching this code paths, also clean up the synthetic names used
> when a function parm is unnamed.
>
> tested on x86_64-darwin,
> OK for master?
OK.
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>
> PR c++/102454
>
> gcc/cp/ChangeLog:
>
> * coroutines.cc (analyze_fn_parms): Clean up synthetic names for
> unnamed function params.
> (morph_fn_to_coro): Do not try to set a guard variable for param
> DTORs in the ramp, unless we have exceptions active.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/coroutines/pr102454.C: New test.
> ---
> gcc/cp/coroutines.cc | 26 ++++++++-------
> gcc/testsuite/g++.dg/coroutines/pr102454.C | 38 ++++++++++++++++++++++
> 2 files changed, 52 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr102454.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index fbd5c49533f..c761e769c12 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -3829,13 +3829,12 @@ analyze_fn_parms (tree orig)
>
> if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (parm.frame_type))
> {
> - char *buf = xasprintf ("_Coro_%s_live", IDENTIFIER_POINTER (name));
> - parm.guard_var = build_lang_decl (VAR_DECL, get_identifier (buf),
> - boolean_type_node);
> - free (buf);
> - DECL_ARTIFICIAL (parm.guard_var) = true;
> - DECL_CONTEXT (parm.guard_var) = orig;
> - DECL_INITIAL (parm.guard_var) = boolean_false_node;
> + char *buf = xasprintf ("%s%s_live", DECL_NAME (arg) ? "_Coro_" : "",
> + IDENTIFIER_POINTER (name));
> + parm.guard_var
> + = coro_build_artificial_var (UNKNOWN_LOCATION, get_identifier (buf),
> + boolean_type_node, orig,
> + boolean_false_node);
> parm.trivial_dtor = false;
> }
> else
> @@ -4843,11 +4842,14 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
> NULL, parm.frame_type,
> LOOKUP_NORMAL,
> tf_warning_or_error);
> - /* This var is now live. */
> - r = build_modify_expr (fn_start, parm.guard_var,
> - boolean_type_node, INIT_EXPR, fn_start,
> - boolean_true_node, boolean_type_node);
> - finish_expr_stmt (r);
> + if (flag_exceptions)
> + {
> + /* This var is now live. */
> + r = build_modify_expr (fn_start, parm.guard_var,
> + boolean_type_node, INIT_EXPR, fn_start,
> + boolean_true_node, boolean_type_node);
> + finish_expr_stmt (r);
> + }
> }
> }
> }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr102454.C b/gcc/testsuite/g++.dg/coroutines/pr102454.C
> new file mode 100644
> index 00000000000..41aeda7b973
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr102454.C
> @@ -0,0 +1,38 @@
> +// { dg-additional-options "-fno-exceptions" }
> +
> +#include <coroutine>
> +#include <string>
> +
> +template <typename T>
> +struct looper {
> + struct promise_type {
> + auto get_return_object () { return handle_type::from_promise (*this); }
> + auto initial_suspend () { return suspend_always_prt {}; }
> + auto final_suspend () noexcept { return suspend_always_prt {}; }
> + void return_value (T);
> + void unhandled_exception ();
> + };
> +
> + using handle_type = std::coroutine_handle<promise_type>;
> +
> + looper (handle_type);
> +
> + struct suspend_always_prt {
> + bool await_ready () noexcept;
> + void await_suspend (handle_type) noexcept;
> + void await_resume () noexcept;
> + };
> +};
> +
> +template <typename T>
> +looper<T>
> +with_ctorable_state (T)
> +{
> + co_return T ();
> +}
> +
> +auto
> +foo ()
> +{
> + return with_ctorable_state<std::string>;
> +}
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-28 17:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 19:38 [PATCH] coroutines: Only set parm copy guard vars if we have exceptions [PR 102454] Iain Sandoe
2021-09-28 17:48 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).