From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 17AD73858412 for ; Thu, 2 Feb 2023 09:19:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 17AD73858412 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id E8D5B20601; Thu, 2 Feb 2023 09:19:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1675329595; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=SZNmz/g+2AczBXflIYHFg6pXycfTX+dpotGvompJ2Gw=; b=Ag6FfXwpVbVpglOiKItwrBVDE4Ezzs/3tzW3RFgChDmWBkY18ncjwM6SiAn9/ufdISwJ0u sJpdkYvAINxW4XJsjVI3VdK2cq5udWOlOZt5KesuuZpnlIfoqAtgOAgMQBev5L5A5DnNAw 4fGml6AaGEGqxLVN53HdLABrhRs+0uk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1675329595; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=SZNmz/g+2AczBXflIYHFg6pXycfTX+dpotGvompJ2Gw=; b=a8FTqzP/WQ6bLPfPpPzWPwm01t0qiJk9+QrngJlABvoVrVb0g7PKrz1I4onoQib780r6U5 55+01Fqbid9189AA== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id BB0032C141; Thu, 2 Feb 2023 09:19:55 +0000 (UTC) Date: Thu, 2 Feb 2023 09:19:55 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Replace IFN_TRAP with BUILT_IN_UNREACHABLE_TRAP [PR107300] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 > > 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 SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)