From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4851C3856DD6 for ; Wed, 27 Jul 2022 10:18:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4851C3856DD6 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-56-e45BkySiMJ-RO8DA4nCWwA-1; Wed, 27 Jul 2022 06:18:00 -0400 X-MC-Unique: e45BkySiMJ-RO8DA4nCWwA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A32CD1824604; Wed, 27 Jul 2022 10:17:59 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 49A331121314; Wed, 27 Jul 2022 10:17:59 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 26RAHul52208800 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 27 Jul 2022 12:17:56 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 26RAHtUS2208799; Wed, 27 Jul 2022 12:17:55 +0200 Date: Wed, 27 Jul 2022 12:17:55 +0200 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org, Jason Merrill , Jan Hubicka Subject: Re: [PATCH] gimple, internal-fn: Add IFN_TRAP and use it for __builtin_unreachable [PR106099] Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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 10:18:05 -0000 On Wed, Jul 27, 2022 at 10:09:34AM +0000, Richard Biener wrote: > > We chose to sanitize not just explicit user __builtin_unreachable calls, > > but also the internally generated __builtin_unreachable calls (with the > > one exception of fall through to end of C++ function returning non-void, > > which had before a separate sanitizer) and we've been doing it since 2013 > > when ubsan was added. > > Even for the internally generated unreachable calls like those from > > devirtualization or other reasons like ivcanon/unrolling, having the > > possibility to get some runtime diagnostics or trap can be useful over > > just falling through to random following code. > > So at least for the unrolling use the intent is to have the > unreachable () fully elided by later passes. Honza can correct me > if I'm wrong. Using __builtin_trap from the start until sanopt > may prevent some of that from happening, keeping dead conditions > live, no? That is true. I guess changing the sanopt gate back and building __builtin_unreachable only in the ivcanon/unrolling case is possible too. Without or with this patch then, the advantage of the patch would be that we wouldn't need to recompute vops if we replace unreachables with traps during the sanopt pass. > > > Previously we'd always emit __builtin_unreachable, then perhaps in some > > cases could e.g. optimize it away (say if there is a guarding condition > > around the implicitly added unreachable turning the condition into VRP > > info and optimizing the conditional away), otherwise the sanopt pass > > would turn those __builtin_unreachable calls into __builtin_trap. > > With the recent changes, we don't run the sanopt pass when only > > doing -fsanitize=unreachable (or -funrechable-traps) though, so we need > > to emit the trap/__ubsan_handle_unreachable/__builtin_unreachable right > > away. > > Why did the recent changes not just replace __builtin_unreachable > at RTL expansion time? Was the intent really to force the paths > to be kept live? I can see that for user or frontend generated > unreachables but not so much for some of the middle-end ones. It is easier on GIMPLE and perhaps allows e.g. sharing the data for __ubsan_handle_unreachable calls. sanopt pass is quite late anyway. Jakub