Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug in float.fromhex #89117

Closed
pgimeno mannequin opened this issue Aug 19, 2021 · 9 comments
Closed

Bug in float.fromhex #89117

pgimeno mannequin opened this issue Aug 19, 2021 · 9 comments
Assignees
@mdickinson
Labels
3.9 only security fixes stdlib Python modules in the Lib dir

Comments

@pgimeno
Copy link
Mannequin

pgimeno mannequin commented Aug 19, 2021

BPO 44954
Nosy @mdickinson, @miss-islington
PRs
  • bpo-44954: Fix wrong result in float.fromhex corner case #27834
  • [3.10] bpo-44954: Fix wrong result in float.fromhex corner case (GH-27834) #27854
  • [3.9] bpo-44954: Fix wrong result in float.fromhex corner case (GH-27834) #27855
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/mdickinson'
closed_at = <Date 2021-08-20.22:49:17.075>
created_at = <Date 2021-08-19.11:34:02.430>
labels = ['library', '3.9']
title = 'Bug in float.fromhex'
updated_at = <Date 2021-08-20.22:49:17.075>
user = 'https://bugs.python.org/pgimeno'

    bugs.python.org fields:

    activity = <Date 2021-08-20.22:49:17.075>
actor = 'terry.reedy'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2021-08-20.22:49:17.075>
closer = 'terry.reedy'
components = ['Library (Lib)']
creation = <Date 2021-08-19.11:34:02.430>
creator = 'pgimeno'
dependencies = []
files = []
hgrepos = []
issue_num = 44954
keywords = ['patch']
message_count = 9.0
messages = ['399909', '399910', '399913', '399960', '399961', '399962', '399983', '399987', '399991']
nosy_count = 3.0
nosy_names = ['mark.dickinson', 'pgimeno', 'miss-islington']
pr_nums = ['27834', '27854', '27855']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue44954'
versions = ['Python 3.9']
    @pgimeno
    Copy link
    Mannequin Author

    pgimeno mannequin commented Aug 19, 2021 

    >>> float.fromhex('0x0.8p-1074')
0.0
>>> float.fromhex('0x.8p-1074')
5e-324

    One of them is obviously wrong. It's the second one, because:

    • The smallest denormal is 0x1p-1074
    • Therefore, 0x0.8p-1074 is a tie for rounding purposes.
    • The digit in the last place is even because the number is zero, and there is a tie, which implies rounding down.

    @pgimeno pgimeno mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir labels Aug 19, 2021
    @mdickinson
    Copy link
    Member

    Thanks for the report! I can reproduce the issue, and agree with your analysis.

    @mdickinson mdickinson added 3.10 only security fixes 3.11 only security fixes labels Aug 19, 2021
    @mdickinson mdickinson self-assigned this Aug 19, 2021
    @mdickinson mdickinson added 3.10 only security fixes 3.11 only security fixes labels Aug 19, 2021
    @mdickinson mdickinson self-assigned this Aug 19, 2021
    @mdickinson
    Copy link
    Member

    The bug is in this line:

    cpython/Objects/floatobject.c

    Line 1467 in 3db42fc

    (half_eps == 8 && (HEX_DIGIT(key_digit+1) & 1) != 0))

    which reads:

            (half_eps == 8 && (HEX_DIGIT(key_digit+1) & 1) != 0))

    In the buggy case, key_digit=0 and the HEX_DIGIT macro is trying to retrieve the value of the second-to-least significant hex digit in the coefficient, to decide whether it's odd or not. For the "0x0.8" case it retrieves the "0". For the "0x.8" case, it retrieves the "x" and tries to interpret it as a hex digit.

    Even worse, if we exclude the "0x" prefix, as in float.fromhex(".8p-1074"), HEX_DIGIT(1) is accessing memory that doesn't belong to the string.

    @mdickinson
    Copy link
    Member

    New changeset 60b93d9 by Mark Dickinson in branch 'main':
    bpo-44954: Fix wrong result in float.fromhex corner case (GH-27834)
    60b93d9

    @mdickinson
    Copy link
    Member

    New changeset 7ef0673 by Miss Islington (bot) in branch '3.9':
    bpo-44954: Fix wrong result in float.fromhex corner case (GH-27834) (GH-27855)
    7ef0673

    @mdickinson
    Copy link
    Member

    Fixed in the main branch and in 3.9; 3.10 is very close to release, so the backport PR for 3.10 may have to wait for 3.10.1 (which wouldn't really be a problem, given that this bug has apparently lain unnoticed since Python 2.7). That's Pablo's call, of course.

    @pedro Thanks again for the report! Just out of curiosity, how did you manage to find this?

    @miss-islington
    Copy link
    Contributor

    New changeset 838b0e9 by Miss Islington (bot) in branch '3.10':
    bpo-44954: Fix wrong result in float.fromhex corner case (GH-27834)
    838b0e9

    @mdickinson
    Copy link
    Member

    All fixed! Closing.

    @mdickinson mdickinson added the type-bug An unexpected behavior, bug, or error label Aug 20, 2021
    @mdickinson mdickinson added the type-bug An unexpected behavior, bug, or error label Aug 20, 2021
    @pgimeno
    Copy link
    Mannequin Author

    pgimeno mannequin commented Aug 20, 2021

    @pedro Thanks again for the report! Just out of curiosity, how did you manage to find this?

    I'm writing a C strtod implementation and I was adding corner cases to the unit testing (now published here: https://codeberg.org/pgimeno/ACSL/src/branch/master/tests/test-strtod.c). Sometimes I get confused by whether to add one or subtract one to the exponent as digits are moved, and since I have an interactive Python shell always open as a calculator, I used it to make sure that the test I was writing had the correct exponent. But the result I got was not the one I expected, and upon verification, I soon became convinced that it was a bug, so I dug a bit more (at first I didn't notice it only happened if the leading zero was left out) and finally submitted the bug.

    Thanks for the quick fix, by the way.

    @pgimeno pgimeno mannequin removed 3.10 only security fixes 3.11 only security fixes labels Aug 20, 2021
    @pgimeno pgimeno mannequin reopened this Aug 20, 2021
    @pgimeno pgimeno mannequin removed type-bug An unexpected behavior, bug, or error 3.10 only security fixes 3.11 only security fixes labels Aug 20, 2021
    @pgimeno pgimeno mannequin reopened this Aug 20, 2021
    @pgimeno pgimeno mannequin removed the type-bug An unexpected behavior, bug, or error label Aug 20, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @mdickinson mdickinson

    Labels
    3.9 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @mdickinson @terryjreedy @miss-islington