Skip to content

Conversation

@mdickinson
Copy link
Member

@mdickinson mdickinson commented Aug 19, 2021

This PR fixes a corner-case bug where the rounding logic in float.fromhex resulted in a value that was rounded the wrong way. Assuming IEEE 754 binary64 floating-point, the bug can only occur in the case where:

  • the exact value of the input is 2**-1075
  • the binary exponent is two more than an integer multiple of 4, so that the only nonzero hex digit in the significand is an 8
  • the 8 is not preceded by any zeros

Example bad inputs are '0x.8p-1074', '0x.80p-1074'and 0x8p-1078. In these cases, the rounding logic tries to interpret the x character as a hex digit, decides that as a hex digit x is odd, and so ends up rounding the value up to 2**-1074, when it should have been rounded down to 0.

The case of a string that doesn't start with 0x is worse: for an input like .8p-1074 or .80p-1074 or 8p-1078, the rounding logic examines the character before the beginning of the string to decide how to round.

This bug affects all Python versions back to 2.7; the fix should be backported to 3.9 and 3.10, but I don't think it's critical enough to go into 3.10.0.rc2 - it can wait for 3.10.1.

https://bugs.python.org/issue44954

# Regression test for a corner-case bug reported in b.p.o. 44954
self.identical(fromHex('0x.8p-1074'), 0.0)
self.identical(fromHex('0x.80p-1074'), 0.0)
self.identical(fromHex('0x.81p-1074'), TINY)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to find what is the minimal example for non-zero result, but seems it works for arbitrary number of zeroes between 8 and 1: 0x.800000000000000000000...0000000000000000001p-1074.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's reassuring, and as it should be: 0x.8p-1074 is the exact threshold for rounding, so if it's larger than 0x.8p-1074 by even the tiniest amount, it should round up.

@miss-islington
Copy link
Contributor

Thanks @mdickinson for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-27854 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 20, 2021
)

(cherry picked from commit 60b93d9)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
@bedevere-bot
Copy link

GH-27855 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 20, 2021
)

(cherry picked from commit 60b93d9)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
@mdickinson mdickinson deleted the fix-44954 branch August 20, 2021 10:42
mdickinson added a commit that referenced this pull request Aug 20, 2021
…H-27855)

(cherry picked from commit 60b93d9)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
miss-islington added a commit that referenced this pull request Aug 20, 2021
(cherry picked from commit 60b93d9)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants