c++, mingw: Fix up types of dtor hooks to __cxa_{,thread_}atexit/__cxa_throw on mingw ia32 [PR114968]

  Hi!

__cxa_atexit/__cxa_thread_atexit/__cxa_throw functions accept function
pointers to usually directly destructors rather than wrappers around
them.
Now, mingw ia32 uses implicitly __attribute__((thiscall)) calling
conventions for METHOD_TYPE (where the this pointer is passed in %ecx
register, the rest on the stack), so these functions use:
in config/os/mingw32/os_defines.h:
 #if defined (__i386__)
 #define _GLIBCXX_CDTOR_CALLABI __thiscall
 #endif
in libsupc++/cxxabi.h
__cxa_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void*) _GLIBCXX_NOTHROW;
__cxa_thread_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void *) _GLIBCXX_NOTHROW;
__cxa_throw(void*, std::type_info*, void (_GLIBCXX_CDTOR_CALLABI *) (void *))
__attribute__((__noreturn__));

Now, mingw for some weird reason uses
 #define TARGET_CXX_USE_ATEXIT_FOR_CXA_ATEXIT hook_bool_void_true
so it never actually uses __cxa_atexit, but does use __cxa_thread_atexit
and __cxa_throw.  Recent changes for modules result in more detailed
__cxa_*atexit/__cxa_throw prototypes precreated by the compiler, and if
that happens and one also includes <cxxabi.h>, the compiler complains about
mismatches in the prototypes.

One thing is the missing thiscall attribute on the FUNCTION_TYPE, the
other problem is that all of atexit/__cxa_atexit/__cxa_thread_atexit
get function pointer types gets created by a single function,
get_atexit_fn_ptr_type (), which creates it depending on if atexit
or __cxa_atexit will be used as either void(*)(void) or void(*)(void *),
but when using atexit and __cxa_thread_atexit it uses the wrong function
type for __cxa_thread_atexit.

The following patch adds a target hook to add the thiscall attribute to the
function pointers, and splits the get_atexit_fn_ptr_type () function into
get_atexit_fn_ptr_type () and get_cxa_atexit_fn_ptr_type (), the former always
creates shared void(*)(void) type, the latter creates either
void(*)(void*) (on most targets) or void(__attribute__((thiscall))*)(void*)
(on mingw ia32).  So that we don't waiste another GTY global tree for it,
because cleanup_type used for the same purpose for __cxa_throw should be
the same, the code changes it to use that type too.

In register_dtor_fn then based on the decision whether to use atexit,
__cxa_atexit or __cxa_thread_atexit it picks the right function pointer
type, and also if it decides to emit a __tcf_* wrapper for the cleanup,
uses that type for that wrapper so that it agrees on calling convention.

Bootstrapped/regtested on x86_64-linux and i686-linux and Liu Hao tested
it on mingw32, ok for trunk?

2024-05-09  Jakub Jelinek  <jakub@redhat.com>

	PR target/114968
gcc/
	* target.def (use_atexit_for_cxa_atexit): Remove spurious space
	from comment.
	(adjust_cdtor_callabi_fntype): New cxx target hook.
	* targhooks.h (default_cxx_adjust_cdtor_callabi_fntype): Declare.
	* targhooks.cc (default_cxx_adjust_cdtor_callabi_fntype): New
	function.
	* doc/tm.texi.in (TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Add.
	* doc/tm.texi: Regenerate.
	* config/i386/i386.cc (ix86_cxx_adjust_cdtor_callabi_fntype): New
	function.
	(TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Redefine.
gcc/cp/
	* cp-tree.h (atexit_fn_ptr_type_node, cleanup_type): Adjust macro
	comments.
	(get_cxa_atexit_fn_ptr_type): Declare.
	* decl.cc (get_atexit_fn_ptr_type): Adjust function comment, only
	build type for atexit argument.
	(get_cxa_atexit_fn_ptr_type): New function.
	(get_atexit_node): Call get_cxa_atexit_fn_ptr_type rather than
	get_atexit_fn_ptr_type when using __cxa_atexit.
	(get_thread_atexit_node): Call get_cxa_atexit_fn_ptr_type
	rather than get_atexit_fn_ptr_type.
	(start_cleanup_fn): Add fntype argument, don't call
	get_atexit_fn_ptr_type for it.
	(register_dtor_fn): Adjust start_cleanup_fn caller, use
	get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type
	when ob_parm is true.
	* except.cc (build_throw): Use get_cxa_atexit_fn_ptr_type ().


	Jakub
  

On 5/9/24 05:01, Jakub Jelinek wrote:
> Hi!
> 
> __cxa_atexit/__cxa_thread_atexit/__cxa_throw functions accept function
> pointers to usually directly destructors rather than wrappers around
> them.
> Now, mingw ia32 uses implicitly __attribute__((thiscall)) calling
> conventions for METHOD_TYPE (where the this pointer is passed in %ecx
> register, the rest on the stack), so these functions use:
> in config/os/mingw32/os_defines.h:
>   #if defined (__i386__)
>   #define _GLIBCXX_CDTOR_CALLABI __thiscall
>   #endif
> in libsupc++/cxxabi.h
> __cxa_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void*) _GLIBCXX_NOTHROW;
> __cxa_thread_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void *) _GLIBCXX_NOTHROW;
> __cxa_throw(void*, std::type_info*, void (_GLIBCXX_CDTOR_CALLABI *) (void *))
> __attribute__((__noreturn__));
> 
> Now, mingw for some weird reason uses
>   #define TARGET_CXX_USE_ATEXIT_FOR_CXA_ATEXIT hook_bool_void_true
> so it never actually uses __cxa_atexit, but does use __cxa_thread_atexit
> and __cxa_throw.  Recent changes for modules result in more detailed
> __cxa_*atexit/__cxa_throw prototypes precreated by the compiler, and if
> that happens and one also includes <cxxabi.h>, the compiler complains about
> mismatches in the prototypes.
> 
> One thing is the missing thiscall attribute on the FUNCTION_TYPE, the
> other problem is that all of atexit/__cxa_atexit/__cxa_thread_atexit
> get function pointer types gets created by a single function,
> get_atexit_fn_ptr_type (), which creates it depending on if atexit
> or __cxa_atexit will be used as either void(*)(void) or void(*)(void *),
> but when using atexit and __cxa_thread_atexit it uses the wrong function
> type for __cxa_thread_atexit.
> 
> The following patch adds a target hook to add the thiscall attribute to the
> function pointers, and splits the get_atexit_fn_ptr_type () function into
> get_atexit_fn_ptr_type () and get_cxa_atexit_fn_ptr_type (), the former always
> creates shared void(*)(void) type, the latter creates either
> void(*)(void*) (on most targets) or void(__attribute__((thiscall))*)(void*)
> (on mingw ia32).  So that we don't waiste another GTY global tree for it,
> because cleanup_type used for the same purpose for __cxa_throw should be
> the same, the code changes it to use that type too.
> 
> In register_dtor_fn then based on the decision whether to use atexit,
> __cxa_atexit or __cxa_thread_atexit it picks the right function pointer
> type, and also if it decides to emit a __tcf_* wrapper for the cleanup,
> uses that type for that wrapper so that it agrees on calling convention.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux and Liu Hao tested
> it on mingw32, ok for trunk?
> 
> 2024-05-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/114968
> gcc/
> 	* target.def (use_atexit_for_cxa_atexit): Remove spurious space
> 	from comment.
> 	(adjust_cdtor_callabi_fntype): New cxx target hook.
> 	* targhooks.h (default_cxx_adjust_cdtor_callabi_fntype): Declare.
> 	* targhooks.cc (default_cxx_adjust_cdtor_callabi_fntype): New
> 	function.
> 	* doc/tm.texi.in (TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Add.
> 	* doc/tm.texi: Regenerate.
> 	* config/i386/i386.cc (ix86_cxx_adjust_cdtor_callabi_fntype): New
> 	function.
> 	(TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Redefine.
> gcc/cp/
> 	* cp-tree.h (atexit_fn_ptr_type_node, cleanup_type): Adjust macro
> 	comments.
> 	(get_cxa_atexit_fn_ptr_type): Declare.
> 	* decl.cc (get_atexit_fn_ptr_type): Adjust function comment, only
> 	build type for atexit argument.
> 	(get_cxa_atexit_fn_ptr_type): New function.
> 	(get_atexit_node): Call get_cxa_atexit_fn_ptr_type rather than
> 	get_atexit_fn_ptr_type when using __cxa_atexit.
> 	(get_thread_atexit_node): Call get_cxa_atexit_fn_ptr_type
> 	rather than get_atexit_fn_ptr_type.
> 	(start_cleanup_fn): Add fntype argument, don't call
> 	get_atexit_fn_ptr_type for it.
> 	(register_dtor_fn): Adjust start_cleanup_fn caller, use
> 	get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type
> 	when ob_parm is true.
> 	* except.cc (build_throw): Use get_cxa_atexit_fn_ptr_type ().
> 
> --- gcc/target.def.jj	2024-05-07 21:28:46.554394913 +0200
> +++ gcc/target.def	2024-05-08 11:19:39.290798568 +0200
> @@ -6498,7 +6498,7 @@ is in effect.  The default is to return
>    hook_bool_void_false)
>   
>   /* Returns true if target may use atexit in the same manner as
> -   __cxa_atexit  to register static destructors.  */
> +   __cxa_atexit to register static destructors.  */
>   DEFHOOK
>   (use_atexit_for_cxa_atexit,
>    "This hook returns true if the target @code{atexit} function can be used\n\
> @@ -6509,6 +6509,17 @@ unloaded. The default is to return false
>    bool, (void),
>    hook_bool_void_false)
>   
> +/* Returns modified FUNCTION_TYPE for cdtor callabi.  */
> +DEFHOOK
> +(adjust_cdtor_callabi_fntype,
> + "This hook returns a possibly modified @code{FUNCTION_TYPE} for arguments\n\
> +to @code{__cxa_atexit}, @code{__cxa_thread_atexit} or @code{__cxa_throw}\n\
> +function pointers.  ABIs like mingw32 require special attributes to be added\n\
> +to function types pointed to by arguments of these functions.\n\
> +The default is to return the passed argument unmodified.",
> + tree, (tree fntype),
> + default_cxx_adjust_cdtor_callabi_fntype)
> +
>   DEFHOOK
>   (adjust_class_at_definition,
>   "@var{type} is a C++ class (i.e., RECORD_TYPE or UNION_TYPE) that has just\n\
> --- gcc/targhooks.h.jj	2024-05-07 21:28:46.554394913 +0200
> +++ gcc/targhooks.h	2024-05-08 11:19:39.290798568 +0200
> @@ -65,6 +65,7 @@ extern machine_mode default_mode_for_suf
>   
>   extern tree default_cxx_guard_type (void);
>   extern tree default_cxx_get_cookie_size (tree);
> +extern tree default_cxx_adjust_cdtor_callabi_fntype (tree);
>   
>   extern bool hook_pass_by_reference_must_pass_in_stack
>     (cumulative_args_t, const function_arg_info &);
> --- gcc/targhooks.cc.jj	2024-05-07 21:28:46.554394913 +0200
> +++ gcc/targhooks.cc	2024-05-08 11:19:39.290798568 +0200
> @@ -329,6 +329,14 @@ default_cxx_get_cookie_size (tree type)
>     return cookie_size;
>   }
>   
> +/* Returns modified FUNCTION_TYPE for cdtor callabi.  */
> +
> +tree
> +default_cxx_adjust_cdtor_callabi_fntype (tree fntype)
> +{
> +  return fntype;
> +}
> +
>   /* Return true if a parameter must be passed by reference.  This version
>      of the TARGET_PASS_BY_REFERENCE hook uses just MUST_PASS_IN_STACK.  */
>   
> --- gcc/doc/tm.texi.in.jj	2024-05-07 21:28:46.553394927 +0200
> +++ gcc/doc/tm.texi.in	2024-05-08 11:19:39.291798555 +0200
> @@ -7223,6 +7223,8 @@ floating-point support; they are not inc
>   
>   @hook TARGET_CXX_USE_ATEXIT_FOR_CXA_ATEXIT
>   
> +@hook TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE
> +
>   @hook TARGET_CXX_ADJUST_CLASS_AT_DEFINITION
>   
>   @hook TARGET_CXX_DECL_MANGLING_CONTEXT
> --- gcc/doc/tm.texi.jj	2024-05-07 21:28:46.523395332 +0200
> +++ gcc/doc/tm.texi	2024-05-08 11:19:39.293798528 +0200
> @@ -11117,6 +11117,14 @@ shared libraries are run in the correct
>   unloaded. The default is to return false.
>   @end deftypefn
>   
> +@deftypefn {Target Hook} tree TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE (tree @var{fntype})
> +This hook returns a possibly modified @code{FUNCTION_TYPE} for arguments
> +to @code{__cxa_atexit}, @code{__cxa_thread_atexit} or @code{__cxa_throw}
> +function pointers.  ABIs like mingw32 require special attributes to be added
> +to function types pointed to by arguments of these functions.
> +The default is to return the passed argument unmodified.
> +@end deftypefn
> +
>   @deftypefn {Target Hook} void TARGET_CXX_ADJUST_CLASS_AT_DEFINITION (tree @var{type})
>   @var{type} is a C++ class (i.e., RECORD_TYPE or UNION_TYPE) that has just
>   been defined.  Use this hook to make adjustments to the class (eg, tweak
> --- gcc/config/i386/i386.cc.jj	2024-05-08 10:16:53.959824814 +0200
> +++ gcc/config/i386/i386.cc	2024-05-08 11:19:39.288798595 +0200
> @@ -25799,6 +25799,20 @@ ix86_bitint_type_info (int n, struct bit
>     return true;
>   }
>   
> +/* Returns modified FUNCTION_TYPE for cdtor callabi.  */
> +tree
> +ix86_cxx_adjust_cdtor_callabi_fntype (tree fntype)
> +{
> +  if (TARGET_64BIT
> +      || TARGET_RTD
> +      || ix86_function_type_abi (fntype) != MS_ABI)
> +    return fntype;
> +  /* For 32-bit MS ABI add thiscall attribute.  */
> +  tree attribs = tree_cons (get_identifier ("thiscall"), NULL_TREE,
> +			    TYPE_ATTRIBUTES (fntype));
> +  return build_type_attribute_variant (fntype, attribs);
> +}
> +
>   /* Implement PUSH_ROUNDING.  On 386, we have pushw instruction that
>      decrements by exactly 2 no matter what the position was, there is no pushb.
>   
> @@ -26410,6 +26424,8 @@ static const scoped_attribute_specs *con
>   #define TARGET_C_EXCESS_PRECISION ix86_get_excess_precision
>   #undef TARGET_C_BITINT_TYPE_INFO
>   #define TARGET_C_BITINT_TYPE_INFO ix86_bitint_type_info
> +#undef TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE
> +#define TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE ix86_cxx_adjust_cdtor_callabi_fntype
>   #undef TARGET_PROMOTE_PROTOTYPES
>   #define TARGET_PROMOTE_PROTOTYPES hook_bool_const_tree_true
>   #undef TARGET_PUSH_ARGUMENT
> --- gcc/cp/cp-tree.h.jj	2024-05-07 08:47:36.092835331 +0200
> +++ gcc/cp/cp-tree.h	2024-05-08 11:46:46.767087686 +0200
> @@ -368,8 +368,7 @@ extern GTY(()) tree cp_global_trees[CPTI
>   #define throw_fn			cp_global_trees[CPTI_THROW_FN]
>   #define rethrow_fn			cp_global_trees[CPTI_RETHROW_FN]
>   
> -/* The type of the function-pointer argument to "__cxa_atexit" (or
> -   "std::atexit", if "__cxa_atexit" is not being used).  */
> +/* The type of the function-pointer argument to "std::atexit".  */
>   #define atexit_fn_ptr_type_node         cp_global_trees[CPTI_ATEXIT_FN_PTR_TYPE]
>   
>   /* A pointer to `std::atexit'.  */
> @@ -384,7 +383,8 @@ extern GTY(()) tree cp_global_trees[CPTI
>   /* The declaration of the dynamic_cast runtime.  */
>   #define dynamic_cast_node		cp_global_trees[CPTI_DCAST]
>   
> -/* The type of a destructor.  */
> +/* The type of a destructor, passed to __cxa_atexit, __cxa_thread_atexit
> +   or __cxa_throw.  */
>   #define cleanup_type			cp_global_trees[CPTI_CLEANUP_TYPE]
>   
>   /* The type of the vtt parameter passed to subobject constructors and
> @@ -7067,6 +7067,7 @@ extern tree check_default_argument		(tre
>   extern int wrapup_namespace_globals		();
>   extern tree create_implicit_typedef		(tree, tree);
>   extern int local_variable_p			(const_tree);
> +extern tree get_cxa_atexit_fn_ptr_type		();
>   extern tree register_dtor_fn			(tree);
>   extern tmpl_spec_kind current_tmpl_spec_kind	(int);
>   extern tree cxx_builtin_function		(tree decl);
> --- gcc/cp/decl.cc.jj	2024-05-07 21:28:50.230345231 +0200
> +++ gcc/cp/decl.cc	2024-05-08 11:57:57.655143259 +0200
> @@ -93,7 +93,7 @@ static void record_key_method_defined (t
>   static tree create_array_type_for_decl (tree, tree, tree, location_t);
>   static tree get_atexit_node (void);
>   static tree get_dso_handle_node (void);
> -static tree start_cleanup_fn (void);
> +static tree start_cleanup_fn (tree);
>   static void end_cleanup_fn (void);
>   static tree cp_make_fname_decl (location_t, tree, int);
>   static void initialize_predefined_identifiers (void);
> @@ -9678,34 +9678,39 @@ declare_global_var (tree name, tree type
>     return decl;
>   }
>   
> -/* Returns the type for the argument to "__cxa_atexit" (or "atexit",
> -   if "__cxa_atexit" is not being used) corresponding to the function
> +/* Returns the type for the argument to "atexit" corresponding to the function
>      to be called when the program exits.  */
>   
>   static tree
> -get_atexit_fn_ptr_type (void)
> +get_atexit_fn_ptr_type ()
>   {
> -  tree fn_type;
> -
>     if (!atexit_fn_ptr_type_node)
>       {
> -      tree arg_type;
> -      if (flag_use_cxa_atexit
> -	  && !targetm.cxx.use_atexit_for_cxa_atexit ())
> -	/* The parameter to "__cxa_atexit" is "void (*)(void *)".  */
> -	arg_type = ptr_type_node;
> -      else
> -	/* The parameter to "atexit" is "void (*)(void)".  */
> -	arg_type = NULL_TREE;
> -
> -      fn_type = build_function_type_list (void_type_node,
> -					  arg_type, NULL_TREE);
> +      tree fn_type = build_function_type_list (void_type_node, NULL_TREE);
>         atexit_fn_ptr_type_node = build_pointer_type (fn_type);
>       }
>   
>     return atexit_fn_ptr_type_node;
>   }
>   
> +/* Returns the type for the argument to "__cxa_atexit", "__cxa_thread_atexit"
> +   or "__cxa_throw" corresponding to the destructor to be called when the
> +   program exits.  */
> +
> +tree
> +get_cxa_atexit_fn_ptr_type ()
> +{
> +  if (!cleanup_type)
> +    {
> +      tree fntype = build_function_type_list (void_type_node,
> +					      ptr_type_node, NULL_TREE);
> +      fntype = targetm.cxx.adjust_cdtor_callabi_fntype (fntype);
> +      cleanup_type = build_pointer_type (fntype);
> +    }
> +
> +  return cleanup_type;
> +}
> +
>   /* Returns a pointer to the `atexit' function.  Note that if
>      FLAG_USE_CXA_ATEXIT is nonzero, then this will actually be the new
>      `__cxa_atexit' function specified in the IA64 C++ ABI.  */
> @@ -9736,7 +9741,7 @@ get_atexit_node (void)
>         use_aeabi_atexit = targetm.cxx.use_aeabi_atexit ();
>         /* First, build the pointer-to-function type for the first
>   	 argument.  */
> -      fn_ptr_type = get_atexit_fn_ptr_type ();
> +      fn_ptr_type = get_cxa_atexit_fn_ptr_type ();
>         /* Then, build the rest of the argument types.  */
>         argtype2 = ptr_type_node;
>         if (use_aeabi_atexit)
> @@ -9819,7 +9824,7 @@ get_thread_atexit_node (void)
>   
>        int __cxa_thread_atexit (void (*)(void *), void *, void *) */
>     tree fn_type = build_function_type_list (integer_type_node,
> -					   get_atexit_fn_ptr_type (),
> +					   get_cxa_atexit_fn_ptr_type (),
>   					   ptr_type_node, ptr_type_node,
>   					   NULL_TREE);
>   
> @@ -9866,7 +9871,7 @@ get_dso_handle_node (void)
>   static GTY(()) int start_cleanup_cnt;
>   
>   static tree
> -start_cleanup_fn (void)
> +start_cleanup_fn (tree fntype)
>   {
>     char name[32];
>   
> @@ -9878,7 +9883,6 @@ start_cleanup_fn (void)
>     /* Build the name of the function.  */
>     sprintf (name, "__tcf_%d", start_cleanup_cnt++);
>     /* Build the function declaration.  */
> -  tree fntype = TREE_TYPE (get_atexit_fn_ptr_type ());
>     tree fndecl = build_lang_decl (FUNCTION_DECL, get_identifier (name), fntype);
>     DECL_CONTEXT (fndecl) = FROB_CONTEXT (current_namespace);
>     /* It's a function with internal linkage, generated by the
> @@ -9968,8 +9972,10 @@ register_dtor_fn (tree decl)
>         build_cleanup (decl);
>     
>         /* Now start the function.  */
> -      cleanup = start_cleanup_fn ();
> -
> +      cleanup = start_cleanup_fn (TREE_TYPE (ob_parm
> +					     ? get_cxa_atexit_fn_ptr_type ()
> +					     : get_atexit_fn_ptr_type ()));

I think I'd rather pass ob_parm to start_cleanup_fn, where it can also 
replace the flag_use_cxa_atexit check in that function.

> +
>         /* Now, recompute the cleanup.  It may contain SAVE_EXPRs that refer
>   	 to the original function, rather than the anonymous one.  That
>   	 will make the back end think that nested functions are in use,
> @@ -9998,7 +10004,8 @@ register_dtor_fn (tree decl)
>       {
>         /* We must convert CLEANUP to the type that "__cxa_atexit"
>   	 expects.  */
> -      cleanup = build_nop (get_atexit_fn_ptr_type (), cleanup);
> +      cleanup = build_nop (ob_parm ? get_cxa_atexit_fn_ptr_type ()
> +				   : get_atexit_fn_ptr_type (), cleanup);

If we're (now) using the correct type to build the cleanup fn, this 
conversion should be unnecessary.

Jason