diff mbox series

coroutines: Adjust outlined function names [PR95520].

Message ID FB99B4E0-0234-47A4-9A6F-38C1E8A30C22@sandoe.co.uk
State New
Headers show
Series coroutines: Adjust outlined function names [PR95520]. | expand

Commit Message

Iain Sandoe July 9, 2021, 6:18 p.m. UTC
Hi,

The mechanism used to date for uniquing the coroutine helper
functions (actor, destroy) was over-complicating things and
leading to the noted PR and also difficulties in setting
breakpoints on these functions (so this will help PR99215 as
well).  The revised mangling matches the form used by clang.

OK for master & backports?
thanks
Iain

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

PR c++/95520 - [coroutines] __builtin_FUNCTION() returns mangled .actor instead of original function name

	PR c++/95520

gcc/cp/ChangeLog:

	* coroutines.cc (act_des_fn): Adjust coroutine
	helper function name mangling.

gcc/testsuite/ChangeLog:

	* g++.dg/coroutines/pr95520.C: New test.
---
 gcc/cp/coroutines.cc                      | 14 +++++++++--
 gcc/testsuite/g++.dg/coroutines/pr95520.C | 29 +++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95520.C

Comments

Jason Merrill July 9, 2021, 9:40 p.m. UTC | #1
On 7/9/21 2:18 PM, Iain Sandoe wrote:
> Hi,
> 
> The mechanism used to date for uniquing the coroutine helper
> functions (actor, destroy) was over-complicating things and
> leading to the noted PR and also difficulties in setting
> breakpoints on these functions (so this will help PR99215 as
> well).  The revised mangling matches the form used by clang.
> 
> OK for master & backports?
> thanks
> Iain
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> 
> PR c++/95520 - [coroutines] __builtin_FUNCTION() returns mangled .actor instead of original function name
> 
> 	PR c++/95520
> 
> gcc/cp/ChangeLog:
> 
> 	* coroutines.cc (act_des_fn): Adjust coroutine
> 	helper function name mangling.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/coroutines/pr95520.C: New test.
> ---
>   gcc/cp/coroutines.cc                      | 14 +++++++++--
>   gcc/testsuite/g++.dg/coroutines/pr95520.C | 29 +++++++++++++++++++++++
>   2 files changed, 41 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95520.C
> 
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 54ffdc8d062..1a3ab58e044 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -3985,9 +3985,19 @@ register_local_var_uses (tree *stmt, int *do_subtree, void *d)
>   static tree
>   act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
>   {
> -  tree fn_name = get_fn_local_identifier (orig, name);
> +  tree fn_name;
>     location_t loc = DECL_SOURCE_LOCATION (orig);
> -  tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
> +  tree fn = build_lang_decl (FUNCTION_DECL, DECL_NAME (orig), fn_type);
> +  if (tree da_name = DECL_ASSEMBLER_NAME (orig))
> +    {
> +      char *buf = xasprintf ("%s.%s", IDENTIFIER_POINTER (da_name), name);
> +      fn_name = get_identifier (buf);
> +      free (buf);
> +    }
> +  else
> +    fn_name = get_fn_local_identifier (orig, name);

How about handling this in write_encoding, along the lines of the 
devel/c++-contracts branch?

Speaking of which, I wonder if you also want to do something similar to 
what I did there to put the ramp/actor/destroyer functions into into the 
same comdat group.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 54ffdc8d062..1a3ab58e044 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3985,9 +3985,19 @@  register_local_var_uses (tree *stmt, int *do_subtree, void *d)
 static tree
 act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name)
 {
-  tree fn_name = get_fn_local_identifier (orig, name);
+  tree fn_name;
   location_t loc = DECL_SOURCE_LOCATION (orig);
-  tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
+  tree fn = build_lang_decl (FUNCTION_DECL, DECL_NAME (orig), fn_type);
+  if (tree da_name = DECL_ASSEMBLER_NAME (orig))
+    {
+      char *buf = xasprintf ("%s.%s", IDENTIFIER_POINTER (da_name), name);
+      fn_name = get_identifier (buf);
+      free (buf);
+    }
+  else
+    fn_name = get_fn_local_identifier (orig, name);
+
+  SET_DECL_ASSEMBLER_NAME (fn, fn_name);
   DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
   DECL_SOURCE_LOCATION (fn) = loc;
   DECL_ARTIFICIAL (fn) = true;
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95520.C b/gcc/testsuite/g++.dg/coroutines/pr95520.C
new file mode 100644
index 00000000000..4849b0789c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95520.C
@@ -0,0 +1,29 @@ 
+// { dg-do run }
+// { dg-output "coroutine name: MyFoo" }
+#include <coroutine>
+#include <cstdio>
+
+struct pt
+{
+    using handle_t = std::coroutine_handle<pt>;
+    auto get_return_object() noexcept { return handle_t::from_promise(*this); }
+
+    std::suspend_never initial_suspend () const noexcept { return {}; }
+    std::suspend_never final_suspend () const noexcept { return {}; }
+    void return_void() const noexcept {}
+    void unhandled_exception() const noexcept {}
+};
+
+template <> struct std::coroutine_traits<pt::handle_t>
+    { using promise_type = pt; };
+
+static pt::handle_t MyFoo ()
+{ 
+    printf ("coroutine name: %s\n", __builtin_FUNCTION());
+    co_return;
+}
+
+int main()
+{
+    MyFoo ();
+}