From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 4252A3856DE2 for ; Wed, 27 Jul 2022 09:33:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4252A3856DE2 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 2A52437E24; Wed, 27 Jul 2022 09:33:48 +0000 (UTC) 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 22AD42C141; Wed, 27 Jul 2022 09:33:48 +0000 (UTC) Date: Wed, 27 Jul 2022 09:33:47 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org, Jason Merrill Subject: Re: [PATCH] gimple, internal-fn: Add IFN_TRAP and use it for __builtin_unreachable [PR106099] 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.0 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jul 2022 09:33:51 -0000 On Wed, 27 Jul 2022, Jakub Jelinek wrote: > Hi! > > __builtin_unreachable and __ubsan_handle_builtin_unreachable don't > use vops, they are marked const/leaf/noreturn/nothrow/cold. > But __builtin_trap uses vops, isn't const, just leaf/noreturn/nothrow/cold. > This is I believe so that when users explicitly use __builtin_trap in their > sources they get stores visible at the trap side. > -fsanitize=unreachable -fsanitize-undefined-trap-on-error used to transform > __builtin_unreachable to __builtin_trap even in the past, but the sanopt pass > has TODO_update_ssa, so it worked fine. > > Now that gimple_build_builtin_unreachable can build a __builtin_trap call > right away, we can run into problems that whenever we need it we would need > to either manually or through TODO_update* ensure the vops being updated. > > Though, as it is originally __builtin_unreachable which is just implemented > as trap, I think for this case it is fine to avoid vops. For this the > patch introduces IFN_TRAP, which has ECF_* flags like __builtin_unreachable > and is expanded as __builtin_trap. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I think for the sake of sanitizing unreachable as trap this is OK but it seems this isn't actually what is done. I still think the original change fiddling with the unreachable decl is wrong. Likewise unrolling shouldn't use gimple_build_builtin_unreachable - it isn't sanitizing anything but telling the middle-end to DCE a path. IMHO only few select places where the middle-end builds unreachable () should be using this function (which means it probably shouldn't exist), like path isolation which IIRC uses a trap anyway. > 2022-07-27 Jakub Jelinek > > PR tree-optimization/106099 > * internal-fn.def (TRAP): New internal fn. > * internal-fn.h (expand_TRAP): Declare. > * internal-fn.cc (expand_TRAP): Define. > * gimple.cc (gimple_build_builtin_unreachable): For BUILT_IN_TRAP, > use internal fn rather than builtin. > > * gcc.dg/ubsan/pr106099.c: New test. > > --- gcc/internal-fn.def.jj 2022-07-26 10:32:23.886269144 +0200 > +++ gcc/internal-fn.def 2022-07-26 11:40:41.799927048 +0200 > @@ -456,6 +456,10 @@ 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, NULL) > + > #undef DEF_INTERNAL_INT_FN > #undef DEF_INTERNAL_FLT_FN > #undef DEF_INTERNAL_FLT_FLOATN_FN > --- gcc/internal-fn.h.jj 2022-06-16 10:56:28.945385251 +0200 > +++ gcc/internal-fn.h 2022-07-26 11:45:50.483837472 +0200 > @@ -242,6 +242,7 @@ extern void expand_internal_call (intern > extern void expand_PHI (internal_fn, gcall *); > extern void expand_SHUFFLEVECTOR (internal_fn, gcall *); > extern void expand_SPACESHIP (internal_fn, gcall *); > +extern void expand_TRAP (internal_fn, gcall *); > > extern bool vectorized_internal_fn_supported_p (internal_fn, tree); > > --- gcc/internal-fn.cc.jj 2022-07-26 10:32:23.885269157 +0200 > +++ gcc/internal-fn.cc 2022-07-26 11:42:02.611856420 +0200 > @@ -4494,3 +4494,9 @@ expand_SPACESHIP (internal_fn, gcall *st > if (!rtx_equal_p (target, ops[0].value)) > emit_move_insn (target, ops[0].value); > } > + > +void > +expand_TRAP (internal_fn, gcall *) > +{ > + expand_builtin_trap (); > +} > --- gcc/gimple.cc.jj 2022-06-27 11:18:02.680058429 +0200 > +++ gcc/gimple.cc 2022-07-26 11:57:17.049760135 +0200 > @@ -430,7 +430,16 @@ gimple_build_builtin_unreachable (locati > { > tree data = NULL_TREE; > tree fn = sanitize_unreachable_fn (&data, loc); > - gcall *g = gimple_build_call (fn, data != NULL_TREE, data); > + 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); > + } > gimple_set_location (g, loc); > return g; > } > --- gcc/testsuite/gcc.dg/ubsan/pr106099.c.jj 2022-07-26 12:22:26.248156163 +0200 > +++ gcc/testsuite/gcc.dg/ubsan/pr106099.c 2022-07-26 11:34:25.660909186 +0200 > @@ -0,0 +1,10 @@ > +/* PR tree-optimization/106099 */ > +/* { dg-do compile } */ > +/* { dg-options "-O -fsanitize=unreachable -fsanitize-undefined-trap-on-error -fno-tree-ccp -fno-tree-dominator-opts" } */ > + > +void > +foo (void) > +{ > + for (unsigned i = 0; i == 0; i++) > + ; > +} > > 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)