From patchwork Tue Apr 29 09:34:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 343735 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 2DC151400FE for ; Tue, 29 Apr 2014 19:35:06 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=ZW7X3/BXmsVblE7Cq RIehsKEV43VIlzyCqOB8EZdpmcx8MSU06rJTloB5fEn5tuB5Oq8TdRgh59LACnRO 1wq96VABijm7IfxI9z6N9bHwrNVh66dDFGWnY+9vDb5lWRKjkNVrm0/Mbp9H1WIR vH4gb0Zrtn9kx/yRbaWpn7tMN8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=EO3jnmDuz7N75gmWK9Ggito 04QU=; b=YZ0gN59BGGyp5dWr1rJMgfayACvg0kTi5yDIcC66QY3kMm5FKO2OX7i vfxqKXxBISEoXShuWT6rp68JlzAijGCeCNHVFBWOKwPSGHT5ytAWEH18RaD3QncC OHDn3XIzEhCRK/I5UVivEwZEnEjd589WlsWyGsDIH3206Y16iNNo= Received: (qmail 21693 invoked by alias); 29 Apr 2014 09:35:00 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 21675 invoked by uid 89); 29 Apr 2014 09:34:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Apr 2014 09:34:58 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s3T9Ytmq028648 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 29 Apr 2014 05:34:55 -0400 Received: from redhat.com (ovpn-116-53.ams2.redhat.com [10.36.116.53]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s3T9YoLj022555 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Tue, 29 Apr 2014 05:34:53 -0400 Date: Tue, 29 Apr 2014 11:34:50 +0200 From: Marek Polacek To: Jakub Jelinek Cc: Tobias Burnus , Marc Glisse , GCC Patches , "Joseph S. Myers" Subject: Re: [PATCH] Implement -fsanitize=float-divide-by-zero Message-ID: <20140429093450.GA11802@redhat.com> References: <20140429082758.GA9773@physik.fu-berlin.de> <20140429084606.GO1817@tucnak.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140429084606.GO1817@tucnak.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) On Tue, Apr 29, 2014 at 10:46:06AM +0200, Jakub Jelinek wrote: > On Tue, Apr 29, 2014 at 10:27:58AM +0200, Tobias Burnus wrote: > > Thus, I think Fortran users would also prefer not to have > > -fsanitize=undefined implying trapping dividing by zero. > > > > Thus, I wonder whether it wouldn't be more useful to provide a command-line option > > to make the floating-point exceptions signalling - and to remind the users to link > > libbacktrace to get the trace. > > That can be done independently with a different option, but -fsanitize=... > IMNSHO definitely shouldn't do anything with floating point control > registers. > > To my surprise, the wording in C99 or C++11 make even floating point > division by zero undefined behavior, but I think generally at least for IEEE > floating point semantics it is well defined, thus I think we shouldn't > include it in -fsanitize=undefined. Thanks for comments. The following is thus implementation of -fsanitize=float-divide-by-zero that is not enabled by default. For this to work well I had to uglify builtins.def and gcc.c a little bit (hopefully this will be the sole ubsan option that is not included in SANITIZE_UNDEFINED). Ran ubsan testsuite (-m32/-m64) + bootstrap-ubsan on x86_64-linux, ok now? 2014-04-29 Marek Polacek * gcc.c (sanitize_spec_function): Handle SANITIZE_FLOAT_DIVIDE. * builtins.def: Initialize builtins even for SANITIZE_FLOAT_DIVIDE. * flag-types.h (enum sanitize_code): Add SANITIZE_FLOAT_DIVIDE. * opts.c (common_handle_option): Add -fsanitize=float-divide-by-zero. c-family/ * c-ubsan.c (ubsan_instrument_division): Handle REAL_TYPEs. Perform INT_MIN / -1 sanitization only for integer types. c/ * c-typeck.c (build_binary_op): Call ubsan_instrument_division also when SANITIZE_FLOAT_DIVIDE is on. cp/ * typeck.c (cp_build_binary_op): Call ubsan_instrument_division even when SANITIZE_FLOAT_DIVIDE is on. Set doing_div_or_mod even for non-integer types. testsuite/ * c-c++-common/ubsan/div-by-zero-5.c: Fix formatting. * c-c++-common/ubsan/float-div-by-zero-1.c: New test. Marek diff --git gcc/builtins.def gcc/builtins.def index 5b902d8..d400ecb 100644 --- gcc/builtins.def +++ gcc/builtins.def @@ -176,7 +176,7 @@ along with GCC; see the file COPYING3. If not see DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE, \ true, true, true, ATTRS, true, \ (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \ - | SANITIZE_UNDEFINED))) + | SANITIZE_UNDEFINED | SANITIZE_FLOAT_DIVIDE))) #undef DEF_CILKPLUS_BUILTIN #define DEF_CILKPLUS_BUILTIN(ENUM, NAME, TYPE, ATTRS) \ diff --git gcc/c-family/c-ubsan.c gcc/c-family/c-ubsan.c index e4f6f32..a039792 100644 --- gcc/c-family/c-ubsan.c +++ gcc/c-family/c-ubsan.c @@ -46,15 +46,21 @@ ubsan_instrument_division (location_t loc, tree op0, tree op1) gcc_assert (TYPE_MAIN_VARIANT (TREE_TYPE (op0)) == TYPE_MAIN_VARIANT (TREE_TYPE (op1))); - /* TODO: REAL_TYPE is not supported yet. */ - if (TREE_CODE (type) != INTEGER_TYPE) + if (TREE_CODE (type) == INTEGER_TYPE + && (flag_sanitize & SANITIZE_DIVIDE)) + t = fold_build2 (EQ_EXPR, boolean_type_node, + op1, build_int_cst (type, 0)); + else if (TREE_CODE (type) == REAL_TYPE + && (flag_sanitize & SANITIZE_FLOAT_DIVIDE)) + t = fold_build2 (EQ_EXPR, boolean_type_node, + op1, build_real (type, dconst0)); + else return NULL_TREE; - t = fold_build2 (EQ_EXPR, boolean_type_node, - op1, build_int_cst (type, 0)); - /* We check INT_MIN / -1 only for signed types. */ - if (!TYPE_UNSIGNED (type)) + if (TREE_CODE (type) == INTEGER_TYPE + && (flag_sanitize & SANITIZE_DIVIDE) + && !TYPE_UNSIGNED (type)) { tree x; tt = fold_build2 (EQ_EXPR, boolean_type_node, op1, diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 62c72df..8df544b 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -10995,7 +10995,8 @@ build_binary_op (location_t location, enum tree_code code, return error_mark_node; } - if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE)) + if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE + | SANITIZE_FLOAT_DIVIDE)) && current_function_decl != 0 && !lookup_attribute ("no_sanitize_undefined", DECL_ATTRIBUTES (current_function_decl)) @@ -11006,7 +11007,8 @@ build_binary_op (location_t location, enum tree_code code, op1 = c_save_expr (op1); op0 = c_fully_fold (op0, false, NULL); op1 = c_fully_fold (op1, false, NULL); - if (doing_div_or_mod && (flag_sanitize & SANITIZE_DIVIDE)) + if (doing_div_or_mod && (flag_sanitize & (SANITIZE_DIVIDE + | SANITIZE_FLOAT_DIVIDE))) instrument_expr = ubsan_instrument_division (location, op0, op1); else if (doing_shift && (flag_sanitize & SANITIZE_SHIFT)) instrument_expr = ubsan_instrument_shift (location, code, op0, op1); diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 9a80727..99b4ce6 100644 --- gcc/cp/typeck.c +++ gcc/cp/typeck.c @@ -4112,10 +4112,7 @@ cp_build_binary_op (location_t location, enum tree_code tcode0 = code0, tcode1 = code1; tree cop1 = fold_non_dependent_expr_sfinae (op1, tf_none); cop1 = maybe_constant_value (cop1); - - if (tcode0 == INTEGER_TYPE) - doing_div_or_mod = true; - + doing_div_or_mod = true; warn_for_div_by_zero (location, cop1); if (tcode0 == COMPLEX_TYPE || tcode0 == VECTOR_TYPE) @@ -4155,9 +4152,7 @@ cp_build_binary_op (location_t location, { tree cop1 = fold_non_dependent_expr_sfinae (op1, tf_none); cop1 = maybe_constant_value (cop1); - - if (code0 == INTEGER_TYPE) - doing_div_or_mod = true; + doing_div_or_mod = true; warn_for_div_by_zero (location, cop1); } @@ -4904,7 +4899,8 @@ cp_build_binary_op (location_t location, if (build_type == NULL_TREE) build_type = result_type; - if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE)) + if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE + | SANITIZE_FLOAT_DIVIDE)) && !processing_template_decl && current_function_decl != 0 && !lookup_attribute ("no_sanitize_undefined", @@ -4918,7 +4914,8 @@ cp_build_binary_op (location_t location, tf_none)); op1 = maybe_constant_value (fold_non_dependent_expr_sfinae (op1, tf_none)); - if (doing_div_or_mod && (flag_sanitize & SANITIZE_DIVIDE)) + if (doing_div_or_mod && (flag_sanitize & (SANITIZE_DIVIDE + | SANITIZE_FLOAT_DIVIDE))) { /* For diagnostics we want to use the promoted types without shorten_binary_op. So convert the arguments to the diff --git gcc/flag-types.h gcc/flag-types.h index fc3261b..caf4039 100644 --- gcc/flag-types.h +++ gcc/flag-types.h @@ -228,6 +228,7 @@ enum sanitize_code { SANITIZE_SI_OVERFLOW = 1 << 9, SANITIZE_BOOL = 1 << 10, SANITIZE_ENUM = 1 << 11, + SANITIZE_FLOAT_DIVIDE = 1 << 12, SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN | SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM diff --git gcc/gcc.c gcc/gcc.c index e5130d1..7bea6d7 100644 --- gcc/gcc.c +++ gcc/gcc.c @@ -8170,7 +8170,7 @@ sanitize_spec_function (int argc, const char **argv) if (strcmp (argv[0], "thread") == 0) return (flag_sanitize & SANITIZE_THREAD) ? "" : NULL; if (strcmp (argv[0], "undefined") == 0) - return ((flag_sanitize & SANITIZE_UNDEFINED) + return ((flag_sanitize & (SANITIZE_UNDEFINED | SANITIZE_FLOAT_DIVIDE)) && !flag_sanitize_undefined_trap_on_error) ? "" : NULL; if (strcmp (argv[0], "leak") == 0) return ((flag_sanitize diff --git gcc/opts.c gcc/opts.c index 1873b96..3c214f0 100644 --- gcc/opts.c +++ gcc/opts.c @@ -1461,6 +1461,8 @@ common_handle_option (struct gcc_options *opts, sizeof "signed-integer-overflow" -1 }, { "bool", SANITIZE_BOOL, sizeof "bool" - 1 }, { "enum", SANITIZE_ENUM, sizeof "enum" - 1 }, + { "float-divide-by-zero", SANITIZE_FLOAT_DIVIDE, + sizeof "float-divide-by-zero" - 1 }, { NULL, 0, 0 } }; const char *comma; diff --git gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c index 7a28bac..bb391c5 100644 --- gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c +++ gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c @@ -1,4 +1,4 @@ -/* { dg-do compile} */ +/* { dg-do compile } */ /* { dg-options "-fsanitize=integer-divide-by-zero" } */ void diff --git gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-1.c gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-1.c index e69de29..30bc090 100644 --- gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-1.c +++ gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-1.c @@ -0,0 +1,25 @@ +/* { dg-do run } */ +/* { dg-options "-fsanitize=float-divide-by-zero" } */ + +int +main (void) +{ + volatile float a = 1.3f; + volatile double b = 0.0; + volatile int c = 4; + + a / b; + a / 0.0; + 2.7f / b; + 3.6 / (b = 0.0, b); + c / b; + b / c; + + return 0; +} + +/* { dg-output "division by zero\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*division by zero\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*division by zero\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*division by zero\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*division by zero\[^\n\r]*" } */