public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Joseph S. Myers" <joseph@codesourcery.com>
To: Marek Polacek <polacek@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH] Implement -fsanitize=float-cast-overflow
Date: Tue, 13 May 2014 18:11:00 -0000	[thread overview]
Message-ID: <Pine.LNX.4.64.1405131745570.23277@digraph.polyomino.org.uk> (raw)
In-Reply-To: <20140513170801.GG2663@redhat.com>

On Tue, 13 May 2014, Marek Polacek wrote:

> Here's an attempt to add the -fsanitize=float-cast-overflow
> instrumentation.  It should issue a runtime error when a floating-point
> to integer type conversion overflows.  Eventually it should instrument

As with divide-by-zero, this should not be part of -fsanitize=undefined 
because under Annex F this is not undefined behavior (instead it raises 
"invalid" and returns an unspecified value, C11 F.4).

> even floating-point to floating-point conversions to detect e.g.
> (float)1e39 overflow, but I'd like to settle first on float to int
> before implementing that.

I think that should be a separate option.  If the type has infinities, 
it's not undefined even in the absence of Annex F (because infinities 
count as part of the range of the type).  And of course overflow depends 
on the rounding mode when the type of the result is a floating-point type.

(However, conversions of integers to floating point can probably count the 
same as conversions of floating point to floating point if you add such an 
option.  __int128 to float can overflow.)

It would be a good idea for the testcases to cover conversions to 
__int128 / unsigned __int128, where supported.

What do you do for overflowing conversions to bit-fields?  I think the 
correct rule is:

* For C, if the floating-point value, truncated toward 0, is outside the 
range of a signed or unsigned type of the specified number of bits, then 
you should get the (invalid, unspecified value (this isn't actually 
implemented in GCC)), and get a runtime error for the new option.

* For C++, bit-fields don't count as separate types, so it should act as 
converting to the declared type and then converting from that to the 
bit-field (as a modulo operation).

Thus, for an unsigned:1 bit-field, for example, values outside the 
interval (-1, 2) would produce the error for C, but only those outside 
(-1, 0x1p32) would do so for C++ (presuming 32-bit int).

> +  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));

It looks to me like this will first round the max value to the 
floating-point type, then add 1 to the rounded value and round again.  
Which I think is in fact safe at least for IEEE binary floating-point 
types, but that isn't immediately obvious.

Possible issues:

* Does the folding of the addition occur in all cases for IBM long double?

* Is this correct for decimal floating point?  There, the overflow 
condition (value >= max+1) should be using a value of (max+1) rounded 
upward rather than to-nearest, if max+1 isn't exactly representable (and 
in general it isn't - powers of two 0x1p24 and above aren't representable 
in decimal32, 0x1p54 and above in decimal64, 0x1p113 and above in 
decimal128, so you just need to find a case where the double-rounding 
computation you have produces the wrong value).

* Likewise, (value <= min-1) for both binary and decimal floating point - 
you need to round once, away from 0.  For float converted to signed int, 
the relevant condition is values < -0x1p31 - 1, i.e. <= 0x1.000002p31f 
once you allow for which values are representable as float, which is not 
min-1 (min-1 rounds to -0x1p31, but a conversion of that to signed int is 
fully defined with no exceptions).

-- 
Joseph S. Myers
joseph@codesourcery.com

  parent reply	other threads:[~2014-05-13 18:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 17:08 Marek Polacek
2014-05-13 17:38 ` Jakub Jelinek
2014-05-13 18:11 ` Joseph S. Myers [this message]
2014-05-14 11:38   ` Jakub Jelinek
2014-05-14 17:37     ` Joseph S. Myers
2014-05-15 19:09       ` Jakub Jelinek
2014-05-15 21:29         ` Joseph S. Myers
2014-05-16  7:37           ` Jakub Jelinek
2014-05-16 15:54             ` Joseph S. Myers
2014-05-20 20:33             ` [PATCH] Implement -fsanitize=float-cast-overflow (take 2) Marek Polacek
2014-05-20 21:50               ` Joseph S. Myers
2014-05-21 12:51                 ` Marek Polacek
2014-05-21 14:46                   ` Jakub Jelinek
2014-05-21 18:52                     ` Jakub Jelinek
2014-05-22  8:06                       ` [PATCH] Fix LTO decimal ICE Jakub Jelinek
2014-05-21 17:50                   ` [PATCH] Implement -fsanitize=float-cast-overflow (take 2) Joseph S. Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.1405131745570.23277@digraph.polyomino.org.uk \
    --to=joseph@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=polacek@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).