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 59025385E02E for ; Tue, 14 Jun 2022 11:44:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 59025385E02E 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-22-eEXV7K3kPgCsnLpl_I2w-Q-1; Tue, 14 Jun 2022 07:44:36 -0400 X-MC-Unique: eEXV7K3kPgCsnLpl_I2w-Q-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6C08E802E5B; Tue, 14 Jun 2022 11:44:36 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2A23A492CA2; Tue, 14 Jun 2022 11:44:36 +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 25EBiXST3483839 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 14 Jun 2022 13:44:33 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 25EBiXqb3483838; Tue, 14 Jun 2022 13:44:33 +0200 Date: Tue, 14 Jun 2022 13:44:32 +0200 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, richard.guenther@gmail.com, jwakely.gcc@gmail.com Subject: Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] Message-ID: Reply-To: Jakub Jelinek References: <20220613195313.3240547-1-jason@redhat.com> MIME-Version: 1.0 In-Reply-To: <20220613195313.3240547-1-jason@redhat.com> X-Scanned-By: MIMEDefang 2.85 on 10.11.54.9 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=-4.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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: Tue, 14 Jun 2022 11:44:39 -0000 On Mon, Jun 13, 2022 at 03:53:13PM -0400, Jason Merrill via Gcc-patches wrote: > When not optimizing, we can't do anything useful with unreachability in > terms of code performance, so we might as well improve debugging by turning > __builtin_unreachable into a trap. In the PR richi suggested introducing an > -funreachable-traps flag for this, but this functionality is already > implemented as -fsanitize=unreachable -fsanitize-undefined-trap-on-error, we > just need to set those flags by default. > > I think it also makes sense to do this when we're explicitly optimizing for > the debugging experience. > > I then needed to make options-save handle -fsanitize and > -fsanitize-undefined-trap-on-error; since -fsanitize is has custom parsing, > that meant handling it explicitly in the awk scripts. I also noticed we > weren't setting it in opts_set. > > Do we still want -funreachable-traps as an alias (or separate flag) for this > behavior that doesn't mention the sanitizer? I do not like doing it this way, -fsanitize-undefined-trap-on-error is (unfortunately for compatibility with llvm misdesign, users should have ways to control which of the enabled sanitizers should be handled which way, where the 3 ways are runtime diagnostics without abort, runtime diagnostics with abort and __builtin_trap ()) an all or nothing option which affects all the sanitizers. Your patch will e.g. silently enable the sanitization whenever just -fsanitize-undefined-trap-on-error is passed, but that can be passed even when users don't really expect any sanitization, just making sure that if it is enabled (say for selected TUs or functions), it doesn't need a runtime library to report it. Furthermore, handling it the UBSan way means we slow down the compiler (enable a bunch of extra passes, like sanopt, ubsan), which is undesirable e.g. for -O0 compilation speed. So, I think -funreachable-traps should be a separate flag and not an alias, enabled by default for -O0 and -Og, which would be handled elsewhere (I'd say e.g. in fold_builtin_0 and perhaps gimple_fold_builtin too to avoid allocating trees unnecessary) and would be done if flag_unreachable_traps && !sanitize_flag_p (SANITIZE_UNREACHABLE), just replacing that __builtin_unreachable call with __builtin_trap. For the function ends in fact under those conditions we could emit __builtin_trap right away instead of emitting __builtin_unreachable and waiting on folding it later to __builtin_trap. Jakub