From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3141 invoked by alias); 15 May 2014 19:09:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 3121 invoked by uid 89); 15 May 2014 19:09:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 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; Thu, 15 May 2014 19:09:38 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4FJ9Z9g002477 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 15 May 2014 15:09:36 -0400 Received: from tucnak.zalov.cz (ovpn-116-116.ams2.redhat.com [10.36.116.116]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4FJ9XrR008485 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 15 May 2014 15:09:35 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.8/8.14.7) with ESMTP id s4FJ9VWL010983; Thu, 15 May 2014 21:09:32 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.8/8.14.8/Submit) id s4FJ9Tm2010982; Thu, 15 May 2014 21:09:29 +0200 Date: Thu, 15 May 2014 19:09:00 -0000 From: Jakub Jelinek To: "Joseph S. Myers" Cc: Marek Polacek , GCC Patches Subject: Re: [PATCH] Implement -fsanitize=float-cast-overflow Message-ID: <20140515190929.GQ10386@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20140513170801.GG2663@redhat.com> <20140514113839.GE10386@tucnak.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2014-05/txt/msg01220.txt.bz2 On Wed, May 14, 2014 at 05:37:25PM +0000, Joseph S. Myers wrote: > > with s/max/min/;s/dconst1/dconstm1/; and, after the real_convert > > do inexact = real_arithmetic (&newminval, MINUS_EXPR, &minval, &dconst1); > > if !inexact just min = build_real (expr_type, newminval); and be done with > > it (the question is if for IBM double double this will DTRT for > > LONG_LONG_MIN, which I think should be that the high double will contain > > (double) LONG_LONG_MIN and the low double -1.0). For inexact > > (which should be the same thing as if result of real_arithmetic + real_convert > > is the same as original minval) we need to subtract more than one, dunno if > > we should just compute it from the REAL_EXP and precision, or just keep > > subtracing powers of two until after real_convert it is no longer bitwise > > identical to original minval. We don't have anything close to > > real_nextafter nor real_convert variant that can round for arbitrary > > rounding modes. > > Any preferences how to implement this? > > In the inexact case but where the power of 2 is representable, you could > always handle it as < min rather than <= min-1 - although computing the > actual nextafter value based on the precision of the floating-point type > shouldn't be hard and would allow <= to be used everywhere. Here is incremental implementation for the binary floating point formats (and, it seems also the C/C++ bitfields are handled properly already by Marek's patch). I've tried struct S { int i : 17; } s; void f1 (float x) { s.i = x; } long long f2 (float x) { return x; } __int128_t f3 (long double x) { return x; } __int128_t f4 (float x) { return x; } __uint128_t f5 (float x) { return x; } long long f6 (long double x) { return x; } on both x86_64 and ppc64 -mlong-double-128 and manually inspected the numbers and they looked ok, for ppc64 f6 was using: u<= { 9223372036854775808.0 + 0.0 } u>= {-9223372036854775808.0 + -1.0 } and f3 u<= { 170141183460469231731687303715884105728.0 + 0.0 } u>= { -170141183460469231731687303715884105728.0 + -4194304.0 } which I believe is correct. Of course the above needs to be transformed into two real testcases that will actually test that valid in-range values don't complain and out of range do, will leave that to Marek. > > For _Decimal*, no idea unfortunately, perhaps for the first iteration > > ubsan should ignore decimal to int conversions. > > Yes, that seems reasonable. (Computing the exact max+1 or min-1 as an > MPFR value and then using mpfr_snprintf (then decimal_real_from_string) > would be one way of converting to decimal with a controlled rounding > direction.) If prec < HOST_BITS_PER_WIDE_INT, then we can just snprintf normally, otherwise yes, we could e.g. use char *buf = XALLOCAVEC (char, prec / 2); mpfr_t m; mpfr_init2 (m, prec + 2); mpfr_set_ui_2exp (m, 1, prec - !uns_p); mpfr_snprintf (buf, prec / 2, "%f", m); // buf should be TYPE_MAX_VALUE + 1.0 here? if (!uns_p) { mpfr_set_si_2exp (m, -1, prec - 1); mpfr_sub_ui (m, m, 1, GMP_RNDN); mpfr_snprintf (buf, prec / 2, "%f", m); // buf should be TYPE_MIN_VALUE - 1.0 here? } But I think we can't use decimal_real_from_string, we'd need a variant of that function that would allow specification of the rounding mode and which of the 3 _DecimalN types it is, and supposedly decNumber dn; decContext set; decContextDefault (&set, DEC_INIT_DECIMAL{32,64,128}); set.traps = 0; set.round = {DEC_ROUND_CEILING,DEC_ROUND_FLOOR}; decNumberFromString (&dn, s, &set); then if not DEC_INIT_DECIMAL128 supposedly convert there and back to DEC_INIT_DECIMAL128 and then convert to REAL_FORMAT. But my _Decimal knowledge is very limited, so I'll leave that to the DFP folks (unless Marek is interested). --- gcc/convert.c +++ gcc/convert.c @@ -851,6 +851,8 @@ expr = save_expr (expr); tree check = ubsan_instrument_float_cast (loc, type, expr); expr = build1 (FIX_TRUNC_EXPR, type, expr); + if (check == NULL) + return expr; return fold_build2 (COMPOUND_EXPR, TREE_TYPE (expr), check, expr); } else --- gcc/ubsan.c +++ gcc/ubsan.c @@ -903,17 +903,62 @@ ubsan_instrument_float_cast (location_t loc, tree type, tree expr) { tree expr_type = TREE_TYPE (expr); - tree t, tt, fn; + tree t, tt, fn, min, max; + enum machine_mode mode = TYPE_MODE (expr_type); + int prec = TYPE_PRECISION (type); + bool uns_p = TYPE_UNSIGNED (type); - tree min = TYPE_MIN_VALUE (type); - tree max = TYPE_MAX_VALUE (type); - /* Add/subtract 1.0 so we can avoid truncating the value of EXPR. */ - min = fold_build2 (MINUS_EXPR, expr_type, - build_real_from_int_cst (expr_type, min), - build_one_cst (expr_type)); - max = fold_build2 (PLUS_EXPR, expr_type, - build_real_from_int_cst (expr_type, max), - build_one_cst (expr_type)); + /* Float to integer conversion first truncates toward zero, so + even signed char c = 127.875f; is not problematic. + Therefore, we should complain only if EXPR is unordered or smaller + or equal than TYPE_MIN_VALUE - 1.0 or greater or equal than + TYPE_MAX_VALUE + 1.0. */ + if (REAL_MODE_FORMAT (mode)->b == 2) + { + /* For maximum, TYPE_MAX_VALUE might not be representable + in EXPR_TYPE, e.g. if TYPE is 64-bit long long and + EXPR_TYPE is IEEE single float, but TYPE_MAX_VALUE + 1.0 is + either representable or infinity. */ + REAL_VALUE_TYPE maxval = dconst1; + SET_REAL_EXP (&maxval, REAL_EXP (&maxval) + prec - !uns_p); + real_convert (&maxval, mode, &maxval); + max = build_real (expr_type, maxval); + + /* For unsigned, assume -1.0 is always representable. */ + if (uns_p) + min = build_minus_one_cst (expr_type); + else + { + /* TYPE_MIN_VALUE is generally representable (or -inf), + but TYPE_MIN_VALUE - 1.0 might not be. */ + REAL_VALUE_TYPE minval = dconstm1, minval2; + SET_REAL_EXP (&minval, REAL_EXP (&minval) + prec - 1); + real_convert (&minval, mode, &minval); + real_arithmetic (&minval2, MINUS_EXPR, &minval, &dconst1); + real_convert (&minval2, mode, &minval2); + if (real_compare (EQ_EXPR, &minval, &minval2) + && !real_isinf (&minval)) + { + /* If TYPE_MIN_VALUE - 1.0 is not representable and + rounds to TYPE_MIN_VALUE, we need to subtract + more. As REAL_MODE_FORMAT (mode)->p is the number + of base digits, we want to subtract a number that + will be 1 << (REAL_MODE_FORMAT (mode)->p - 1) + times smaller than minval. */ + minval2 = dconst1; + gcc_assert (prec > REAL_MODE_FORMAT (mode)->p); + SET_REAL_EXP (&minval2, + REAL_EXP (&minval2) + prec - 1 + - REAL_MODE_FORMAT (mode)->p + 1); + real_arithmetic (&minval2, MINUS_EXPR, &minval, &minval2); + real_convert (&minval2, mode, &minval2); + } + min = build_real (expr_type, minval2); + } + } + /* Only binary floating point supported right now. */ + else + return NULL_TREE; if (flag_sanitize_undefined_trap_on_error) fn = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); Jakub