classification
Title: demoting floating float values to unrepresentable types is undefined behavior
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, mark.dickinson, vstinner
Priority: normal Keywords:

Created on 2017-09-06 22:20 by benjamin.peterson, last changed 2017-09-12 06:08 by benjamin.peterson. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3396 merged benjamin.peterson, 2017-09-06 22:26
PR 3424 merged python-dev, 2017-09-07 18:14
PR 3486 merged benjamin.peterson, 2017-09-10 20:56
PR 3495 merged benjamin.peterson, 2017-09-11 06:52
Messages (10)
msg301528 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-06 22:20
According to C99, "When a finite value of real floating type is converted to an integer type other than _Bool, the  fractional  part  is  discarded  (i.e.,  the  value  is  truncated  toward  zero). If the value of the integral part cannot be represented by the integer type,the behavior is undefined." So, e.g., (long)x, where x is a double is in general undefined behavior without a range check. We have several cases in CPython where we need to fix this.
msg301602 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-07 18:14
New changeset a853a8ba7850381d49b284295dd6f0dc491dbe44 by Benjamin Peterson in branch 'master':
bpo-31373: fix undefined floating-point demotions (#3396)
https://github.com/python/cpython/commit/a853a8ba7850381d49b284295dd6f0dc491dbe44
msg301607 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-07 18:35
New changeset b03623227ed1264e3cac4e6bb4878d96b91aa484 by Benjamin Peterson (Miss Islington (bot)) in branch '3.6':
[3.6] fixes bpo-31373: fix undefined floating-point demotions (GH-3396) (#3424)
https://github.com/python/cpython/commit/b03623227ed1264e3cac4e6bb4878d96b91aa484
msg301798 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-09-10 04:43
There's a (to my mind) unfortunate change in behaviour here. Under normal IEEE 754 rules, some C double values larger than FLT_MAX still round to FLT_MAX under conversion to float.

Python 3.6:

>>> import struct
>>> x = 3.40282356e38
>>> struct.pack("<f", x)
b'\xff\xff\x7f\x7f'

Following the changes in this PR:

>>> import struct
>>> x = 3.40282356e38
>>> struct.pack("<f", x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: float too large to pack with f format

The original behaviour is correct with respect to IEEE 754; the new behaviour is not. I think this is a case where the cure is worse than the disease.
msg301799 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-10 05:34
I agree that's bad. Do you know a way to get the IEEE 754 rounding behavior without invoking C undefined behavior?
msg301808 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-09-10 10:58
> Do you know a way to get the IEEE 754 rounding behavior without invoking C undefined behavior?

One option is to hard-code the actual boundary, which is 2**128 * (1 - 2**-25) (as opposed to FLT_MAX, which is 2**128 * (1 - 2**-24)): values equal to or larger than 2**128 * (1 - 2**-25) in absolute value should raise. But that means assuming IEEE 754 and round-ties-to-even, which isn't an outrageous assumption but does make the solution feel a bit fragile.

An alternative would be to scale values in the range (FLT_MAX, 2.0 * FLT_MAX] by 0.5 before doing the conversion, something like this:

        if (fabs(x) > FLT_MAX && !Py_IS_INFINITY(x)) {
          double half_x = 0.5 * x;
          if (half_x > FLT_MAX) {
            goto Overflow;
          }
          float half_y = (float)half_x;
          if (half_y > 0.5 * FLT_MAX) {
            goto Overflow;
          }
        }
msg301831 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-10 20:57
I'm going to undo the changes to getargs.c and floatobject.c. I think the pytime.c change is still correct because the doubles are explicitly rounded before conversion (and the old code checked for error > 1.0).

It's hard to win here I think. The clang undefined behavior sanitizer uses FLT_MAX to make its determination:
runtime error: value 3.40282e+38 is outside the range of representable values of type 'float'

So to satisfy it and preserve the old behavior, we would presumably have to implement the rounding ourselves, which doesn't seem like fun.

Annex F of C99, which defines the requirements for C implementations using IEE754, says, "the conversions for floating types provide  the IEC 60559 conversions between floating-point precisions.". Those are, of course, fully defined. It seems like the undefined behavior sanitizer is being overzealous when the target supports IEEE754.
msg301840 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-09-10 23:34
> It's hard to win here I think.

Agreed.

> It seems like the undefined behavior sanitizer is being overzealous when the target supports IEEE754.

Also agreed.
msg301864 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-11 06:50
New changeset 2bb69a5b4e7f96cb35d1b28aa7b7b3974b351f59 by Benjamin Peterson in branch 'master':
bpo-31373: remove overly strict float range checks (#3486)
https://github.com/python/cpython/commit/2bb69a5b4e7f96cb35d1b28aa7b7b3974b351f59
msg301939 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-09-12 06:08
New changeset cb356c2ecc0528d47fee2b9f4b32da4fcfb48b3a by Benjamin Peterson in branch '3.6':
[3.6] bpo-31373: remove overly strict float range checks (GH-3486) (#3495)
https://github.com/python/cpython/commit/cb356c2ecc0528d47fee2b9f4b32da4fcfb48b3a
History
Date User Action Args
2018-05-19 02:03:20martin.panterlinkissue20941 superseder
2017-09-12 06:08:52benjamin.petersonsetmessages: + msg301939
2017-09-11 06:52:08benjamin.petersonsetpull_requests: + pull_request3488
2017-09-11 06:50:48benjamin.petersonsetmessages: + msg301864
2017-09-10 23:34:08mark.dickinsonsetmessages: + msg301840
2017-09-10 20:57:02benjamin.petersonsetmessages: + msg301831
2017-09-10 20:56:44benjamin.petersonsetpull_requests: + pull_request3476
2017-09-10 10:58:15mark.dickinsonsetmessages: + msg301808
2017-09-10 05:34:07benjamin.petersonsetmessages: + msg301799
2017-09-10 04:43:53mark.dickinsonsetmessages: + msg301798
2017-09-07 18:35:49benjamin.petersonsetstatus: open -> closed
resolution: fixed
stage: resolved
2017-09-07 18:35:05benjamin.petersonsetmessages: + msg301607
2017-09-07 18:14:14python-devsetpull_requests: + pull_request3421
2017-09-07 18:14:01benjamin.petersonsetmessages: + msg301602
2017-09-06 23:46:26rhettingersetnosy: + mark.dickinson
2017-09-06 23:17:38vstinnersetnosy: + vstinner
2017-09-06 22:26:56benjamin.petersonsetpull_requests: + pull_request3402
2017-09-06 22:20:42benjamin.petersoncreate