From patchwork Wed Oct 20 13:22:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 46440 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DE7313858419 for ; Wed, 20 Oct 2021 13:22:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DE7313858419 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1634736160; bh=p/AZZ+tQBHFwhvkXcAqqNRhIrTW7/k2dC1wcEnxDePM=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=WbmdZsuiQ+X502Nz8A+xE4iMK/l33ZA2N8H+bU9rbRdNYgBbjtb3Dq4BXMep/Eqeu bdDv9QpDw+ccZIZgQ3CyAJcDQ/eUJIc4ykIF5mQMr9p7UhgAP2EaOt06bsVHDtlGWV 1MQdtdliDPekiZb3VgxGaQ4+sWCNNxnl2r1CvKO0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 1E2423858C3A for ; Wed, 20 Oct 2021 13:22:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1E2423858C3A Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 0EBFF1FD9E for ; Wed, 20 Oct 2021 13:22:11 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id EF67613B29 for ; Wed, 20 Oct 2021 13:22:10 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id SNdVOQIYcGEOeQAAMHmgww (envelope-from ) for ; Wed, 20 Oct 2021 13:22:10 +0000 Date: Wed, 20 Oct 2021 15:22:10 +0200 (CEST) To: gcc-patches@gcc.gnu.org Subject: [PATCH][RFC] Map -ftrapv to -fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error Message-ID: <20on4s19-o945-4686-o662-6n4q48246q92@fhfr.qr> MIME-Version: 1.0 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Richard Biener via Gcc-patches From: Richard Biener Reply-To: Richard Biener Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This maps -ftrapv to -fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error, effectively removing flag_trapv (or rather making it always false). This has implications on language support - while -ftrapv was formerly universally available the mapping restricts it to the C family of frontends. It also raises questions on mixing -ftrapv with -fsanitize flags, specifically with other recovery options for the undefined sanitizer since -fsanitize-undefined-trap-on-error cannot be restricted to the signed-integer-overflow part at the moment. To more closely map behavior we could add -fsanitize=trapv where with a single option we could also simply alias -ftrapv to that. Code quality wise a simple signed add compiles to movl %edi, %eax addl %esi, %eax jo .L5 ... .L5: ud2 compared to call __addvsi3 and it has less of the bugs -ftrapv has. The IL will not contain a PLUS_EXPR but a .UBSAN_CHECK_ADD internal function call which has rudimentary support throughout optimizers but is not recognized as possibly terminating the program so int foo (int i, int j, int *p, int k) { int tem = i + j; *p = 0; if (k) return tem; return 0; } will be optimized to perform the add only conditional and the possibly NULL *p dereference first (note the same happens with the "legacy" -ftrapv). The behavior with -fnon-call-exceptions is also different as the internal functions are marked as not throwing and as seen above the actual kind of trap can change (SIGILL vs. SIGABRT). One question is whether -ftrapv makes signed integer overflow well-defined (to trap) like -fwrapv makes it wrap. If so the the above behavior is ill-formed. Not sure how sanitizers position themselves with respect to this and whether the current behavior is OK there. The patch below instruments signed integer ops but leaves them undefined so the compiler still has to be careful as to not introduce new signed overflow (but at least that won't trap). Currently -fwrapv -fsanitize=signed-integer-overflow will not instrument any signed operations for example. I do consider the option to simply make -ftrapv do nothing but warn that people should use UBSAN - that wouldn't imply semantics are 1:1 the same (which they are not). Bootstrapped and tested on x86_64-unknown-linux-gnu, regresses FAIL: gcc.dg/vect/trapv-vect-reduc-4.c scan-tree-dump-times vect "Detected reduc tion\\\\." 3 FAIL: gcc.dg/vect/trapv-vect-reduc-4.c scan-tree-dump-times vect "using an in-or der \\\\(fold-left\\\\) reduction" 1 FAIL: gcc.dg/vect/trapv-vect-reduc-4.c scan-tree-dump-times vect "vectorized 3 l oops" 1 where the vectorizer doesn't know the UBSAN IFNs. 2021-10-20 Richard Biener * opts.c (common_handle_option): Handle -ftrapv like -fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error and do not set flag_trapv. --- gcc/opts.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/gcc/opts.c b/gcc/opts.c index 65fe192a198..909d2a031ff 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -3022,7 +3022,21 @@ common_handle_option (struct gcc_options *opts, case OPT_ftrapv: if (value) - opts->x_flag_wrapv = 0; + { + opts->x_flag_wrapv = 0; + opts->x_flag_sanitize + = parse_sanitizer_options ("signed-integer-overflow", + loc, code, opts->x_flag_sanitize, + value, false); + if (!opts_set->x_flag_sanitize_undefined_trap_on_error) + opts->x_flag_sanitize_undefined_trap_on_error = 1; + /* This keeps overflow undefined and not trap. Specifically + it does no longer allow to catch exceptions together with + -fnon-call-exceptions. It also makes -ftrapv cease to + work with non-C-family languages since ubsan only works for + those. */ + opts->x_flag_trapv = 0; + } break; case OPT_fstrict_overflow: