[v3] Fix __builtin_nested_func_ptr_{created, deleted} symbol versions [PR113402]

  In order to regularise the two new builtins as extension library types
the scope of this patch has grown w.r.t "just rename".

Tested on x86_64-darwin21 (default heap trampolines) and x86_64 Linux and
other Darwin platforms that are default executable stack.

How does this look now?
thanks
Iain

--- 8< ---

The symbols for the functions supporting heap-based trampolines were
exported at an incorrect symbol version, the following patch fixes that.

As requested in the PR, this also renames __builtin_nested_func_ptr* to
__gcc_nested_func_ptr*.  In carrying our the rename, we move the builtins
to use DEF_EXT_LIB_BUILTIN.

	PR libgcc/113402

gcc/ChangeLog:

	* builtins.cc (expand_builtin): Handle BUILT_IN_NESTED_PTR_CREATED
	and BUILT_IN_NESTED_PTR_DELETED.
	* builtins.def (BUILT_IN_NESTED_PTR_CREATED,
	BUILT_IN_NESTED_PTR_DELETED): Make these builtins LIB-EXT and
	rename the library fallbacks to __gcc_nested_func_ptr_created and
	__gcc_nested_func_ptr_deleted.
	* doc/invoke.texi: Rename these to __gcc_nested_func_ptr_created
	and __gcc_nested_func_ptr_deleted.
	* tree-nested.cc (finalize_nesting_tree_1): Use builtin_explicit for
	BUILT_IN_NESTED_PTR_CREATED and BUILT_IN_NESTED_PTR_DELETED.
	* tree.cc (build_common_builtin_nodes): Build the
	BUILT_IN_NESTED_PTR_CREATED and BUILT_IN_NESTED_PTR_DELETED local
	builtins only for non-explicit.

libgcc/ChangeLog:

	* config/aarch64/heap-trampoline.c: Rename
	__builtin_nested_func_ptr_created to __gcc_nested_func_ptr_created and
	__builtin_nested_func_ptr_deleted to __gcc_nested_func_ptr_deleted.
	* config/i386/heap-trampoline.c: Likewise.
	* libgcc2.h: Likewise.
	* libgcc-std.ver.in (GCC_7.0.0): Likewise and then move
	__gcc_nested_func_ptr_created and
	__gcc_nested_func_ptr_deleted from this symbol version to ...
	(GCC_14.0.0): ... this one.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
Co-authored-by: Jakub Jelinek  <jakub@redhat.com>
---
 gcc/builtins.cc                         |  5 ++++
 gcc/builtins.def                        |  4 ++--
 gcc/doc/invoke.texi                     |  4 ++--
 gcc/tree-nested.cc                      |  4 ++--
 gcc/tree.cc                             | 31 ++++++++++++++-----------
 libgcc/config/aarch64/heap-trampoline.c |  8 +++----
 libgcc/config/i386/heap-trampoline.c    |  8 +++----
 libgcc/libgcc-std.ver.in                |  5 ++--
 libgcc/libgcc2.h                        |  4 ++--
 9 files changed, 41 insertions(+), 32 deletions(-)
  

On Thu, Jan 18, 2024 at 02:59:23PM +0000, Iain Sandoe wrote:
> --- a/gcc/builtins.cc
> +++ b/gcc/builtins.cc
> @@ -8416,6 +8416,11 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
>      case BUILT_IN_ADJUST_DESCRIPTOR:
>        return expand_builtin_adjust_descriptor (exp);
>  
> +    case BUILT_IN_NESTED_PTR_CREATED:
> +    case BUILT_IN_NESTED_PTR_DELETED:

Unsure if it is ok to have the BUILT_IN_ names so different from the actual
functions, if they shouldn't be
BUILT_IN_GCC_NESTED_FUNC_PTR_{CREATED,DELETED} instead.
The missing __ is what happens even with BUILT_IN_CLEAR_CACHE / __clear_cache.

> +      break; /* At present, no expansion, just call the function.  */
> +
> +

Just one empty newline, not 2.

>      case BUILT_IN_FORK:
>      case BUILT_IN_EXECL:
>      case BUILT_IN_EXECV:
> diff --git a/gcc/builtins.def b/gcc/builtins.def
> index 4d97ca0eec9..fd040eb8d80 100644
> --- a/gcc/builtins.def
> +++ b/gcc/builtins.def
> @@ -1084,8 +1084,8 @@ DEF_BUILTIN_STUB (BUILT_IN_ADJUST_TRAMPOLINE, "__builtin_adjust_trampoline")
>  DEF_BUILTIN_STUB (BUILT_IN_INIT_DESCRIPTOR, "__builtin_init_descriptor")
>  DEF_BUILTIN_STUB (BUILT_IN_ADJUST_DESCRIPTOR, "__builtin_adjust_descriptor")
>  DEF_BUILTIN_STUB (BUILT_IN_NONLOCAL_GOTO, "__builtin_nonlocal_goto")
> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, "__builtin_nested_func_ptr_created")
> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, "__builtin_nested_func_ptr_deleted")
> +DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_CREATED, "__gcc_nested_func_ptr_created", BT_FN_VOID_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
> +DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_DELETED, "__gcc_nested_func_ptr_deleted", BT_FN_VOID, ATTR_NOTHROW_LIST)

See above.

Otherwise LGTM.

	Jakub
  

> On 18 Jan 2024, at 15:05, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Thu, Jan 18, 2024 at 02:59:23PM +0000, Iain Sandoe wrote:
>> --- a/gcc/builtins.cc
>> +++ b/gcc/builtins.cc
>> @@ -8416,6 +8416,11 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
>>     case BUILT_IN_ADJUST_DESCRIPTOR:
>>       return expand_builtin_adjust_descriptor (exp);
>> 
>> +    case BUILT_IN_NESTED_PTR_CREATED:
>> +    case BUILT_IN_NESTED_PTR_DELETED:
> 
> Unsure if it is ok to have the BUILT_IN_ names so different from the actual
> functions, if they shouldn't be
> BUILT_IN_GCC_NESTED_FUNC_PTR_{CREATED,DELETED} instead.
> The missing __ is what happens even with BUILT_IN_CLEAR_CACHE / __clear_cache.
> 
>> +      break; /* At present, no expansion, just call the function.  */
>> +
>> +
> 
> Just one empty newline, not 2.
> 
>>     case BUILT_IN_FORK:
>>     case BUILT_IN_EXECL:
>>     case BUILT_IN_EXECV:
>> diff --git a/gcc/builtins.def b/gcc/builtins.def
>> index 4d97ca0eec9..fd040eb8d80 100644
>> --- a/gcc/builtins.def
>> +++ b/gcc/builtins.def
>> @@ -1084,8 +1084,8 @@ DEF_BUILTIN_STUB (BUILT_IN_ADJUST_TRAMPOLINE, "__builtin_adjust_trampoline")
>> DEF_BUILTIN_STUB (BUILT_IN_INIT_DESCRIPTOR, "__builtin_init_descriptor")
>> DEF_BUILTIN_STUB (BUILT_IN_ADJUST_DESCRIPTOR, "__builtin_adjust_descriptor")
>> DEF_BUILTIN_STUB (BUILT_IN_NONLOCAL_GOTO, "__builtin_nonlocal_goto")
>> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, "__builtin_nested_func_ptr_created")
>> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, "__builtin_nested_func_ptr_deleted")
>> +DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_CREATED, "__gcc_nested_func_ptr_created", BT_FN_VOID_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
>> +DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_DELETED, "__gcc_nested_func_ptr_deleted", BT_FN_VOID, ATTR_NOTHROW_LIST)
> 
> See above.
> 
> Otherwise LGTM

This is what I pushed, thanks
Iain
From 837827f8f2542c36ba5944b4da0a76ea6a64b08b Mon Sep 17 00:00:00 2001
From: Iain Sandoe <iain@sandoe.co.uk>
Date: Tue, 16 Jan 2024 10:21:14 +0000
Subject: [PATCH] Fix __builtin_nested_func_ptr_{created,deleted} symbol
 versions [PR113402]

The symbols for the functions supporting heap-based trampolines were
exported at an incorrect symbol version, the following patch fixes that.

As requested in the PR, this also renames __builtin_nested_func_ptr* to
__gcc_nested_func_ptr*.  In carrying our the rename, we move the builtins
to use DEF_EXT_LIB_BUILTIN.

	PR libgcc/113402

gcc/ChangeLog:

	* builtins.cc (expand_builtin): Handle BUILT_IN_GCC_NESTED_PTR_CREATED
	and BUILT_IN_GCC_NESTED_PTR_DELETED.
	* builtins.def (BUILT_IN_GCC_NESTED_PTR_CREATED,
	BUILT_IN_GCC_NESTED_PTR_DELETED): Make these builtins LIB-EXT and
	rename the library fallbacks to __gcc_nested_func_ptr_created and
	__gcc_nested_func_ptr_deleted.
	* doc/invoke.texi: Rename these to __gcc_nested_func_ptr_created
	and __gcc_nested_func_ptr_deleted.
	* tree-nested.cc (finalize_nesting_tree_1): Use builtin_explicit for
	BUILT_IN_GCC_NESTED_PTR_CREATED and BUILT_IN_GCC_NESTED_PTR_DELETED.
	* tree.cc (build_common_builtin_nodes): Build the
	BUILT_IN_GCC_NESTED_PTR_CREATED and BUILT_IN_GCC_NESTED_PTR_DELETED local
	builtins only for non-explicit.

libgcc/ChangeLog:

	* config/aarch64/heap-trampoline.c: Rename
	__builtin_nested_func_ptr_created to __gcc_nested_func_ptr_created and
	__builtin_nested_func_ptr_deleted to __gcc_nested_func_ptr_deleted.
	* config/i386/heap-trampoline.c: Likewise.
	* libgcc2.h: Likewise.
	* libgcc-std.ver.in (GCC_7.0.0): Likewise and then move
	__gcc_nested_func_ptr_created and
	__gcc_nested_func_ptr_deleted from this symbol version to ...
	(GCC_14.0.0): ... this one.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
Co-authored-by: Jakub Jelinek  <jakub@redhat.com>
---
 gcc/builtins.cc                         |  4 ++++
 gcc/builtins.def                        |  4 ++--
 gcc/doc/invoke.texi                     |  4 ++--
 gcc/tree-nested.cc                      |  4 ++--
 gcc/tree.cc                             | 31 ++++++++++++++-----------
 libgcc/config/aarch64/heap-trampoline.c |  8 +++----
 libgcc/config/i386/heap-trampoline.c    |  8 +++----
 libgcc/libgcc-std.ver.in                |  5 ++--
 libgcc/libgcc2.h                        |  4 ++--
 9 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 09f2354f114..a0bd82c7981 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -8416,6 +8416,10 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
     case BUILT_IN_ADJUST_DESCRIPTOR:
       return expand_builtin_adjust_descriptor (exp);
 
+    case BUILT_IN_GCC_NESTED_PTR_CREATED:
+    case BUILT_IN_GCC_NESTED_PTR_DELETED:
+      break; /* At present, no expansion, just call the function.  */
+
     case BUILT_IN_FORK:
     case BUILT_IN_EXECL:
     case BUILT_IN_EXECV:
diff --git a/gcc/builtins.def b/gcc/builtins.def
index 4d97ca0eec9..f6f3e104f6a 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -1084,8 +1084,8 @@ DEF_BUILTIN_STUB (BUILT_IN_ADJUST_TRAMPOLINE, "__builtin_adjust_trampoline")
 DEF_BUILTIN_STUB (BUILT_IN_INIT_DESCRIPTOR, "__builtin_init_descriptor")
 DEF_BUILTIN_STUB (BUILT_IN_ADJUST_DESCRIPTOR, "__builtin_adjust_descriptor")
 DEF_BUILTIN_STUB (BUILT_IN_NONLOCAL_GOTO, "__builtin_nonlocal_goto")
-DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, "__builtin_nested_func_ptr_created")
-DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, "__builtin_nested_func_ptr_deleted")
+DEF_EXT_LIB_BUILTIN (BUILT_IN_GCC_NESTED_PTR_CREATED, "__gcc_nested_func_ptr_created", BT_FN_VOID_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
+DEF_EXT_LIB_BUILTIN (BUILT_IN_GCC_NESTED_PTR_DELETED, "__gcc_nested_func_ptr_deleted", BT_FN_VOID, ATTR_NOTHROW_LIST)
 
 /* Implementing __builtin_setjmp.  */
 DEF_BUILTIN_STUB (BUILT_IN_SETJMP_SETUP, "__builtin_setjmp_setup")
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f2dee977b00..819a75dfe94 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -19523,8 +19523,8 @@ for nested functions.
 By default, trampolines are generated on stack.  However, certain platforms
 (such as the Apple M1) do not permit an executable stack.  Compiling with
 @option{-ftrampoline-impl=heap} generate calls to
-@code{__builtin_nested_func_ptr_created} and
-@code{__builtin_nested_func_ptr_deleted} in order to allocate and
+@code{__gcc_nested_func_ptr_created} and
+@code{__gcc_nested_func_ptr_deleted} in order to allocate and
 deallocate trampoline space on the executable heap.  These functions are
 implemented in libgcc, and will only be provided on specific targets:
 x86_64 Darwin, x86_64 and aarch64 Linux.  @emph{PLEASE NOTE}: Heap
diff --git a/gcc/tree-nested.cc b/gcc/tree-nested.cc
index 96718a66d01..c8f07f78185 100644
--- a/gcc/tree-nested.cc
+++ b/gcc/tree-nested.cc
@@ -3557,13 +3557,13 @@ finalize_nesting_tree_1 (struct nesting_info *root)
 			  root->frame_decl, field, NULL_TREE);
 	      arg3 = build_addr (x);
 
-	      x = builtin_decl_implicit (BUILT_IN_NESTED_PTR_CREATED);
+	      x = builtin_decl_explicit (BUILT_IN_GCC_NESTED_PTR_CREATED);
 	      stmt = gimple_build_call (x, 3, arg1, arg2, arg3);
 	      gimple_seq_add_stmt (&stmt_list, stmt);
 
 	      /* This call to delete the nested function trampoline is added to
 		 the cleanup list, and called when we exit the current scope.  */
-	      x = builtin_decl_implicit (BUILT_IN_NESTED_PTR_DELETED);
+	      x = builtin_decl_explicit (BUILT_IN_GCC_NESTED_PTR_DELETED);
 	      stmt = gimple_build_call (x, 0);
 	      gimple_seq_add_stmt (&cleanup_list, stmt);
 	    }
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 8aee3ef18d8..3dff8c51083 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -9929,20 +9929,25 @@ build_common_builtin_nodes (void)
 
   tree ptr_ptr_type_node = build_pointer_type (ptr_type_node);
 
-  ftype = build_function_type_list (void_type_node,
-				    ptr_type_node, // void *chain
-				    ptr_type_node, // void *func
-				    ptr_ptr_type_node, // void **dst
-				    NULL_TREE);
-  local_define_builtin ("__builtin_nested_func_ptr_created", ftype,
-			BUILT_IN_NESTED_PTR_CREATED,
-			"__builtin_nested_func_ptr_created", ECF_NOTHROW);
+  if (!builtin_decl_explicit_p (BUILT_IN_GCC_NESTED_PTR_CREATED))
+    {
+      ftype = build_function_type_list (void_type_node,
+					ptr_type_node, // void *chain
+					ptr_type_node, // void *func
+					ptr_ptr_type_node, // void **dst
+					NULL_TREE);
+      local_define_builtin ("__builtin___gcc_nested_func_ptr_created", ftype,
+			    BUILT_IN_GCC_NESTED_PTR_CREATED,
+			    "__gcc_nested_func_ptr_created", ECF_NOTHROW);
+    }
 
-  ftype = build_function_type_list (void_type_node,
-				    NULL_TREE);
-  local_define_builtin ("__builtin_nested_func_ptr_deleted", ftype,
-			BUILT_IN_NESTED_PTR_DELETED,
-			"__builtin_nested_func_ptr_deleted", ECF_NOTHROW);
+  if (!builtin_decl_explicit_p (BUILT_IN_GCC_NESTED_PTR_DELETED))
+    {
+      ftype = build_function_type_list (void_type_node, NULL_TREE);
+      local_define_builtin ("__builtin___gcc_nested_func_ptr_deleted", ftype,
+			    BUILT_IN_GCC_NESTED_PTR_DELETED,
+			    "__gcc_nested_func_ptr_deleted", ECF_NOTHROW);
+    }
 
   ftype = build_function_type_list (void_type_node,
 				    ptr_type_node, ptr_type_node, NULL_TREE);
diff --git a/libgcc/config/aarch64/heap-trampoline.c b/libgcc/config/aarch64/heap-trampoline.c
index f22233987ca..2041fe6aa39 100644
--- a/libgcc/config/aarch64/heap-trampoline.c
+++ b/libgcc/config/aarch64/heap-trampoline.c
@@ -20,8 +20,8 @@ int get_trampolines_per_page (void);
 struct tramp_ctrl_data *allocate_tramp_ctrl (struct tramp_ctrl_data *parent);
 void *allocate_trampoline_page (void);
 
-void __builtin_nested_func_ptr_created (void *chain, void *func, void **dst);
-void __builtin_nested_func_ptr_deleted (void);
+void __gcc_nested_func_ptr_created (void *chain, void *func, void **dst);
+void __gcc_nested_func_ptr_deleted (void);
 
 #if defined(__gnu_linux__)
 static const uint32_t aarch64_trampoline_insns[] = {
@@ -108,7 +108,7 @@ allocate_tramp_ctrl (struct tramp_ctrl_data *parent)
 }
 
 void
-__builtin_nested_func_ptr_created (void *chain, void *func, void **dst)
+__gcc_nested_func_ptr_created (void *chain, void *func, void **dst)
 {
   if (tramp_ctrl_curr == NULL)
     {
@@ -155,7 +155,7 @@ __builtin_nested_func_ptr_created (void *chain, void *func, void **dst)
 }
 
 void
-__builtin_nested_func_ptr_deleted (void)
+__gcc_nested_func_ptr_deleted (void)
 {
   if (tramp_ctrl_curr == NULL)
     abort ();
diff --git a/libgcc/config/i386/heap-trampoline.c b/libgcc/config/i386/heap-trampoline.c
index 4b9f4365868..726cf55277a 100644
--- a/libgcc/config/i386/heap-trampoline.c
+++ b/libgcc/config/i386/heap-trampoline.c
@@ -20,8 +20,8 @@ int get_trampolines_per_page (void);
 struct tramp_ctrl_data *allocate_tramp_ctrl (struct tramp_ctrl_data *parent);
 void *allocate_trampoline_page (void);
 
-void __builtin_nested_func_ptr_created (void *chain, void *func, void **dst);
-void __builtin_nested_func_ptr_deleted (void);
+void __gcc_nested_func_ptr_created (void *chain, void *func, void **dst);
+void __gcc_nested_func_ptr_deleted (void);
 
 static const uint8_t trampoline_insns[] = {
   /* movabs $<chain>,%r11  */
@@ -108,7 +108,7 @@ allocate_tramp_ctrl (struct tramp_ctrl_data *parent)
 }
 
 void
-__builtin_nested_func_ptr_created (void *chain, void *func, void **dst)
+__gcc_nested_func_ptr_created (void *chain, void *func, void **dst)
 {
   if (tramp_ctrl_curr == NULL)
     {
@@ -155,7 +155,7 @@ __builtin_nested_func_ptr_created (void *chain, void *func, void **dst)
 }
 
 void
-__builtin_nested_func_ptr_deleted (void)
+__gcc_nested_func_ptr_deleted (void)
 {
   if (tramp_ctrl_curr == NULL)
     abort ();
diff --git a/libgcc/libgcc-std.ver.in b/libgcc/libgcc-std.ver.in
index a81c5a1142c..ac8f661a08e 100644
--- a/libgcc/libgcc-std.ver.in
+++ b/libgcc/libgcc-std.ver.in
@@ -1943,9 +1943,6 @@ GCC_4.8.0 {
 GCC_7.0.0 {
   __PFX__divmoddi4
   __PFX__divmodti4
-
-  __builtin_nested_func_ptr_created
-  __builtin_nested_func_ptr_deleted
 }
 
 %inherit GCC_14.0.0 GCC_7.0.0
@@ -1960,4 +1957,6 @@ GCC_14.0.0 {
   __PFX__strub_enter
   __PFX__strub_update
   __PFX__strub_leave
+  __gcc_nested_func_ptr_created
+  __gcc_nested_func_ptr_deleted
 }
diff --git a/libgcc/libgcc2.h b/libgcc/libgcc2.h
index 5050456eada..0b67fab637e 100644
--- a/libgcc/libgcc2.h
+++ b/libgcc/libgcc2.h
@@ -29,8 +29,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #pragma GCC visibility push(default)
 #endif
 
-extern void __builtin_nested_func_ptr_created (void *, void *, void **);
-extern void __builtin_nested_func_ptr_deleted (void);
+extern void __gcc_nested_func_ptr_created (void *, void *, void **);
+extern void __gcc_nested_func_ptr_deleted (void);
 
 extern int __gcc_bcmp (const unsigned char *, const unsigned char *, size_t);
 extern void __clear_cache (void *, void *);