Re: [PATCH] Replace IFN_TRAP with BUILT_IN_UNREACHABLE_TRAP [PR107300]

[Richard Biener <rguenther@suse.de>]

On Thu, 2 Feb 2023, Jakub Jelinek wrote:

> Hi!
> 
> For PR106099 I've added IFN_TRAP as an alternative to __builtin_trap
> meant for __builtin_unreachable purposes (e.g. with -funreachable-traps
> or some sanitizers) which doesn't need vops because __builtin_unreachable
> doesn't need them either.  This works in various cases, but unfortunately
> IPA likes to decide on the redirection to unreachable just by tweaking
> the cgraph edge to point to a different FUNCTION_DECL.  As internal
> functions don't have a decl, this causes problems like in the following
> testcase.
> 
> The following patch fixes it by removing IFN_TRAP again and replacing
> it with user inaccessible BUILT_IN_UNREACHABLE_TRAP, so that e.g.
> builtin_decl_unreachable can return it directly and we don't need to tweak
> it later in wherever we actually replace the call stmt.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2023-02-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR ipa/107300
> 	* builtins.def (BUILT_IN_UNREACHABLE_TRAP): New builtin.
> 	* internal-fn.def (TRAP): Remove.
> 	* internal-fn.cc (expand_TRAP): Remove.
> 	* tree.cc (build_common_builtin_nodes): Define
> 	BUILT_IN_UNREACHABLE_TRAP if not yet defined.
> 	(builtin_decl_unreachable): Use BUILT_IN_UNREACHABLE_TRAP
> 	instead of BUILT_IN_TRAP.
> 	* gimple.cc (gimple_build_builtin_unreachable): Remove
> 	emitting internal function for BUILT_IN_TRAP.
> 	* asan.cc (maybe_instrument_call): Handle BUILT_IN_UNREACHABLE_TRAP.
> 	* cgraph.cc (cgraph_edge::verify_corresponds_to_fndecl): Handle
> 	BUILT_IN_UNREACHABLE_TRAP instead of BUILT_IN_TRAP.
> 	* ipa-devirt.cc (possible_polymorphic_call_target_p): Handle
> 	BUILT_IN_UNREACHABLE_TRAP.
> 	* builtins.cc (expand_builtin, is_inexpensive_builtin): Likewise.
> 	* tree-cfg.cc (verify_gimple_call,
> 	pass_warn_function_return::execute): Likewise.
> 	* attribs.cc (decl_attributes): Don't report exclusions on
> 	BUILT_IN_UNREACHABLE_TRAP either.
> 
> 	* gcc.dg/pr107300.c: New test.
> 
> --- gcc/builtins.def.jj	2023-01-02 09:32:37.988059844 +0100
> +++ gcc/builtins.def	2023-02-01 19:19:15.382475912 +0100
> @@ -1048,6 +1048,7 @@ DEF_GCC_BUILTIN        (BUILT_IN_SETJMP,
>  DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRFMON, "strfmon", BT_FN_SSIZE_STRING_SIZE_CONST_STRING_VAR, ATTR_FORMAT_STRFMON_NOTHROW_3_4)
>  DEF_LIB_BUILTIN        (BUILT_IN_STRFTIME, "strftime", BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR, ATTR_FORMAT_STRFTIME_NOTHROW_3_0)
>  DEF_GCC_BUILTIN        (BUILT_IN_TRAP, "trap", BT_FN_VOID, ATTR_NORETURN_NOTHROW_LEAF_COLD_LIST)
> +DEF_GCC_BUILTIN        (BUILT_IN_UNREACHABLE_TRAP, "unreachable trap", BT_FN_VOID, ATTR_CONST_NORETURN_NOTHROW_LEAF_COLD_LIST)
>  DEF_GCC_BUILTIN        (BUILT_IN_UNREACHABLE, "unreachable", BT_FN_VOID, ATTR_CONST_NORETURN_NOTHROW_LEAF_COLD_LIST)
>  DEF_GCC_BUILTIN        (BUILT_IN_UNWIND_INIT, "unwind_init", BT_FN_VOID, ATTR_NULL)
>  DEF_GCC_BUILTIN        (BUILT_IN_UPDATE_SETJMP_BUF, "update_setjmp_buf", BT_FN_VOID_PTR, ATTR_NULL)
> --- gcc/internal-fn.def.jj	2023-01-02 09:32:38.771048530 +0100
> +++ gcc/internal-fn.def	2023-02-01 19:19:52.239936390 +0100
> @@ -457,11 +457,6 @@ DEF_INTERNAL_FN (SHUFFLEVECTOR, ECF_CONS
>  /* <=> optimization.  */
>  DEF_INTERNAL_FN (SPACESHIP, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
>  
> -/* __builtin_trap created from/for __builtin_unreachable.  */
> -DEF_INTERNAL_FN (TRAP, ECF_CONST | ECF_LEAF | ECF_NORETURN
> -		       | ECF_NOTHROW | ECF_COLD | ECF_LOOPING_CONST_OR_PURE,
> -		 NULL)
> -
>  /* [[assume (cond)]].  */
>  DEF_INTERNAL_FN (ASSUME, ECF_CONST | ECF_LEAF | ECF_NOTHROW
>  			 | ECF_LOOPING_CONST_OR_PURE, NULL)
> --- gcc/internal-fn.cc.jj	2023-01-02 09:32:22.206287869 +0100
> +++ gcc/internal-fn.cc	2023-02-01 19:16:52.939561023 +0100
> @@ -4518,12 +4518,6 @@ expand_SPACESHIP (internal_fn, gcall *st
>  }
>  
>  void
> -expand_TRAP (internal_fn, gcall *)
> -{
> -  expand_builtin_trap ();
> -}
> -
> -void
>  expand_ASSUME (internal_fn, gcall *)
>  {
>  }
> --- gcc/tree.cc.jj	2023-01-27 20:09:16.151971051 +0100
> +++ gcc/tree.cc	2023-02-01 20:37:01.287315982 +0100
> @@ -9758,6 +9758,7 @@ build_common_builtin_nodes (void)
>  
>    if (!builtin_decl_explicit_p (BUILT_IN_UNREACHABLE)
>        || !builtin_decl_explicit_p (BUILT_IN_TRAP)
> +      || !builtin_decl_explicit_p (BUILT_IN_UNREACHABLE_TRAP)
>        || !builtin_decl_explicit_p (BUILT_IN_ABORT))
>      {
>        ftype = build_function_type (void_type_node, void_list_node);
> @@ -9767,6 +9768,12 @@ build_common_builtin_nodes (void)
>  			      "__builtin_unreachable",
>  			      ECF_NOTHROW | ECF_LEAF | ECF_NORETURN
>  			      | ECF_CONST | ECF_COLD);
> +      if (!builtin_decl_explicit_p (BUILT_IN_UNREACHABLE_TRAP))
> +	local_define_builtin ("__builtin_unreachable trap", ftype,
> +			      BUILT_IN_UNREACHABLE_TRAP,
> +			      "__builtin_unreachable trap",
> +			      ECF_NOTHROW | ECF_LEAF | ECF_NORETURN
> +			      | ECF_CONST | ECF_COLD);
>        if (!builtin_decl_explicit_p (BUILT_IN_ABORT))
>  	local_define_builtin ("__builtin_abort", ftype, BUILT_IN_ABORT,
>  			      "abort",
> @@ -10908,7 +10915,7 @@ builtin_decl_unreachable ()
>    if (sanitize_flags_p (SANITIZE_UNREACHABLE)
>        ? (flag_sanitize_trap & SANITIZE_UNREACHABLE)
>        : flag_unreachable_traps)
> -    fncode = BUILT_IN_TRAP;
> +    fncode = BUILT_IN_UNREACHABLE_TRAP;
>    /* For non-trapping sanitize, we will rewrite __builtin_unreachable () later,
>       in the sanopt pass.  */
>  
> --- gcc/gimple.cc.jj	2023-01-02 09:32:44.443966564 +0100
> +++ gcc/gimple.cc	2023-02-01 19:20:45.634154800 +0100
> @@ -430,16 +430,7 @@ gimple_build_builtin_unreachable (locati
>  {
>    tree data = NULL_TREE;
>    tree fn = sanitize_unreachable_fn (&data, loc);
> -  gcall *g;
> -  if (DECL_FUNCTION_CODE (fn) != BUILT_IN_TRAP)
> -    g = gimple_build_call (fn, data != NULL_TREE, data);
> -  else
> -    {
> -      /* Instead of __builtin_trap use .TRAP, so that it doesn't
> -	 need vops.  */
> -      gcc_checking_assert (data == NULL_TREE);
> -      g = gimple_build_call_internal (IFN_TRAP, 0);
> -    }
> +  gcall *g = gimple_build_call (fn, data != NULL_TREE, data);
>    gimple_call_set_ctrl_altering (g, true);
>    gimple_set_location (g, loc);
>    return g;
> --- gcc/asan.cc.jj	2023-01-02 09:32:30.191172498 +0100
> +++ gcc/asan.cc	2023-02-01 20:40:02.132674664 +0100
> @@ -2951,6 +2951,7 @@ maybe_instrument_call (gimple_stmt_itera
>  	  switch (DECL_FUNCTION_CODE (callee))
>  	    {
>  	    case BUILT_IN_UNREACHABLE:
> +	    case BUILT_IN_UNREACHABLE_TRAP:
>  	    case BUILT_IN_TRAP:
>  	      /* Don't instrument these.  */
>  	      return false;
> --- gcc/cgraph.cc.jj	2023-01-30 10:57:03.433236251 +0100
> +++ gcc/cgraph.cc	2023-02-01 20:37:39.878752332 +0100
> @@ -3248,11 +3248,11 @@ cgraph_edge::verify_corresponds_to_fndec
>    node = node->ultimate_alias_target ();
>  
>    /* Optimizers can redirect unreachable calls or calls triggering undefined
> -     behavior to __builtin_unreachable or __builtin_trap.  */
> +     behavior to __builtin_unreachable or __builtin_unreachable trap.  */
>  
>    if (fndecl_built_in_p (callee->decl, BUILT_IN_NORMAL)
>        && (DECL_FUNCTION_CODE (callee->decl) == BUILT_IN_UNREACHABLE
> -	  || DECL_FUNCTION_CODE (callee->decl) == BUILT_IN_TRAP))
> +	  || DECL_FUNCTION_CODE (callee->decl) == BUILT_IN_UNREACHABLE_TRAP))
>      return false;
>  
>    if (callee->former_clone_of != node->decl
> --- gcc/ipa-devirt.cc.jj	2023-02-01 17:26:28.241878107 +0100
> +++ gcc/ipa-devirt.cc	2023-02-01 20:42:46.472274417 +0100
> @@ -3471,8 +3471,10 @@ possible_polymorphic_call_target_p (tree
>    unsigned int i;
>    bool final;
>  
> -  if (fndecl_built_in_p (n->decl, BUILT_IN_UNREACHABLE)
> -      || fndecl_built_in_p (n->decl, BUILT_IN_TRAP))
> +  if (fndecl_built_in_p (n->decl, BUILT_IN_NORMAL)
> +      && (DECL_FUNCTION_CODE (n->decl) == BUILT_IN_UNREACHABLE
> +	  || DECL_FUNCTION_CODE (n->decl) == BUILT_IN_TRAP
> +	  || DECL_FUNCTION_CODE (n->decl) == BUILT_IN_UNREACHABLE_TRAP))
>      return true;
>  
>    if (is_cxa_pure_virtual_p (n->decl))
> --- gcc/builtins.cc.jj	2023-01-02 09:32:20.735309123 +0100
> +++ gcc/builtins.cc	2023-02-01 20:39:28.727162564 +0100
> @@ -7844,6 +7844,7 @@ expand_builtin (tree exp, rtx target, rt
>        break;
>  
>      case BUILT_IN_TRAP:
> +    case BUILT_IN_UNREACHABLE_TRAP:
>        expand_builtin_trap ();
>        return const0_rtx;
>  
> @@ -11310,6 +11311,7 @@ is_inexpensive_builtin (tree decl)
>        case BUILT_IN_VA_ARG_PACK_LEN:
>        case BUILT_IN_VA_COPY:
>        case BUILT_IN_TRAP:
> +      case BUILT_IN_UNREACHABLE_TRAP:
>        case BUILT_IN_SAVEREGS:
>        case BUILT_IN_POPCOUNTL:
>        case BUILT_IN_POPCOUNTLL:
> --- gcc/tree-cfg.cc.jj	2023-01-02 09:32:27.879205904 +0100
> +++ gcc/tree-cfg.cc	2023-02-01 20:44:44.826545812 +0100
> @@ -3503,6 +3503,7 @@ verify_gimple_call (gcall *stmt)
>        switch (DECL_FUNCTION_CODE (fndecl))
>  	{
>  	case BUILT_IN_UNREACHABLE:
> +	case BUILT_IN_UNREACHABLE_TRAP:
>  	case BUILT_IN_TRAP:
>  	  if (gimple_call_num_args (stmt) > 0)
>  	    {
> @@ -9681,6 +9682,8 @@ pass_warn_function_return::execute (func
>  		  && ((LOCATION_LOCUS (gimple_location (last))
>  		       == BUILTINS_LOCATION
>  		       && (gimple_call_builtin_p (last, BUILT_IN_UNREACHABLE)
> +			   || gimple_call_builtin_p (last,
> +						     BUILT_IN_UNREACHABLE_TRAP)
>  			   || gimple_call_builtin_p (last, BUILT_IN_TRAP)))
>  		      || gimple_call_builtin_p (last, ubsan_missing_ret)))
>  		{
> --- gcc/attribs.cc.jj	2023-01-13 17:37:45.056485561 +0100
> +++ gcc/attribs.cc	2023-02-01 20:54:16.134198166 +0100
> @@ -846,6 +846,7 @@ decl_attributes (tree *node, tree attrib
>  	      || !DECL_P (*anode)
>  	      || DECL_BUILT_IN_CLASS (*anode) != BUILT_IN_NORMAL
>  	      || (DECL_FUNCTION_CODE (*anode) != BUILT_IN_UNREACHABLE
> +		  && DECL_FUNCTION_CODE (*anode) != BUILT_IN_UNREACHABLE_TRAP
>  		  && (DECL_FUNCTION_CODE (*anode)
>  		      != BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE)))
>  	    {
> --- gcc/testsuite/gcc.dg/pr107300.c.jj	2023-02-01 20:55:36.602021915 +0100
> +++ gcc/testsuite/gcc.dg/pr107300.c	2023-02-01 20:55:25.298187139 +0100
> @@ -0,0 +1,19 @@
> +/* PR ipa/107300 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fipa-cp-clone -funreachable-traps -fno-inline" } */
> +
> +void
> +bar (int x, int y)
> +{
> +  if (x)
> +    __builtin_unreachable ();
> +
> +  if (y)
> +    __builtin_abort ();
> +}
> +
> +void
> +foo (void)
> +{
> +  bar (0, 0);
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)