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.

Sign up for GitHub

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

Wrong return value from PyLong_AsUnsignedLongLongMask on PyErr_BadInternalCall #81351

Closed
ztane mannequin opened this issue Jun 6, 2019 · 12 comments
Closed

Wrong return value from PyLong_AsUnsignedLongLongMask on PyErr_BadInternalCall #81351

ztane mannequin opened this issue Jun 6, 2019 · 12 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@ztane
Copy link
Mannequin

ztane mannequin commented Jun 6, 2019

BPO 37170
Nosy @mdickinson, @vstinner, @ericvsmith, @ztane, @ZackerySpytz
PRs
  • bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() #13860
  • [3.8] bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860) #13891
  • [3.7] bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860) #13896
  • [2.7] bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860) #13898
    		•

    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 = None
closed_at = <Date 2019-06-07.16:23:38.350>
created_at = <Date 2019-06-06.06:52:41.683>
labels = ['interpreter-core', '3.8', 'type-bug', '3.7', '3.9']
title = 'Wrong return value from PyLong_AsUnsignedLongLongMask on PyErr_BadInternalCall'
updated_at = <Date 2019-06-07.16:23:38.349>
user = 'https://github.com/ztane'

    bugs.python.org fields:

    activity = <Date 2019-06-07.16:23:38.349>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-06-07.16:23:38.350>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2019-06-06.06:52:41.683>
creator = 'ztane'
dependencies = []
files = []
hgrepos = []
issue_num = 37170
keywords = ['patch']
message_count = 12.0
messages = ['344789', '344790', '344791', '344802', '344806', '344809', '344818', '344874', '344943', '344953', '344961', '344962']
nosy_count = 5.0
nosy_names = ['mark.dickinson', 'vstinner', 'eric.smith', 'ztane', 'ZackerySpytz']
pr_nums = ['13860', '13891', '13896', '13898']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37170'
versions = ['Python 2.7', 'Python 3.7', 'Python 3.8', 'Python 3.9']
    @ztane
    Copy link
    Mannequin Author

    ztane mannequin commented Jun 6, 2019

    Hi, while checking the longobject implementation for a Stack Overflow answer, I noticed that the functions _PyLong_AsUnsignedLongLongMask and PyLong_AsUnsignedLongLongMask erroneously return (unsigned long)-1 on error when bad internal call is thrown.

    First case:

    cpython/Objects/longobject.c

    Line 1379 in cb65202

    return (unsigned long) -1; 

    static unsigned long long
_PyLong_AsUnsignedLongLongMask(PyObject *vv)
{
    PyLongObject *v;
    unsigned long long x;
    Py_ssize_t i;
    int sign;

    if (vv == NULL || !PyLong_Check(vv)) {
        PyErr_BadInternalCall();
        return (unsigned long) -1; <<<<
    }

    Second case:

    cpython/Objects/longobject.c

    Line 1407 in cb65202

    return (unsigned long)-1;

    They seem to have been incorrect for quite some time, the other one blames back to the SVN era. The bug seems to be in 2.7 alike:

    cpython/Objects/longobject.c

    Line 1025 in 20093b3

    return (unsigned long) -1;

    The correct return value should of course be (unsigned long long)-1

    @ztane ztane mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jun 6, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Jun 6, 2019

    -1 is documented: https://docs.python.org/dev/c-api/long.html#c.PyLong_AsUnsignedLongLongMask

    It's not a bug but a deliberate behavior. See also the general behavior described at:
    https://docs.python.org/dev/c-api/long.html#integer-objects

    @vstinner vstinner closed this as completed Jun 6, 2019
    @ztane
    Copy link
    Mannequin Author

    ztane mannequin commented Jun 6, 2019

    Victor, as a friendly reminder, (unsigned long)-1 is not necessarily the same number as (unsigned long long)-1. The documentation means the latter.

    @ericvsmith
    Copy link
    Member

    As ztane says, the issue isn't the -1, it's the type of the cast. This looks like a legitimate issue to me.

    @ericvsmith ericvsmith reopened this Jun 6, 2019
    @ericvsmith ericvsmith removed the invalid label Jun 6, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Jun 6, 2019

    Victor, as a friendly reminder, (unsigned long)-1 is not necessarily the same number as (unsigned long long)-1. The documentation means the latter.

    Aaaah, I didn't notice that the function returns unsigned *long long*, sorry :-)

    Which platform is impacted? Windows 64-bit? Does it mean that we have zero unit test on this function? Or does it mean that (unsigned long)-1 is compiled correctly as (unsigned long long)-1 on Windows 64-bit? Is "long long" 8 bytes long and "long" 4 bytes long on Windows 64-bit?

    @ztane
    Copy link
    Mannequin Author

    ztane mannequin commented Jun 6, 2019

    Unsigned long long needs to be at least 64 bits wide, so it is probably all 32-bit platforms and 64-bit window at least. These functions are not used only in a few places within the CPython code and when they are they're guarded with PyLong_Checks or similar, as they probably should, but the other is part of public API

    @vstinner
    Copy link
    Member

    vstinner commented Jun 6, 2019

    Python 2.7 - 3.9 are affected. Python 3.6 no longer accept bugfixes.

    @vstinner
    Copy link
    Member

    vstinner commented Jun 6, 2019

    New changeset dc24765 by Victor Stinner (Zackery Spytz) in branch 'master':
    bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860)
    dc24765

    @vstinner
    Copy link
    Member

    vstinner commented Jun 7, 2019

    New changeset dd492d9 by Victor Stinner (Zackery Spytz) in branch '3.8':
    [3.8] bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860) (GH-13891)
    dd492d9

    @vstinner
    Copy link
    Member

    vstinner commented Jun 7, 2019

    New changeset e36ed47 by Victor Stinner (Zackery Spytz) in branch '3.7':
    [3.7] bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860) (GH-13896)
    e36ed47

    @vstinner
    Copy link
    Member

    vstinner commented Jun 7, 2019

    New changeset 2bfc2dc by Victor Stinner (Zackery Spytz) in branch '2.7':
    [2.7] bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860) (GH-13898)
    2bfc2dc

    @vstinner
    Copy link
    Member

    vstinner commented Jun 7, 2019

    Thanks Antti Haapala for the bug report and thanks Zackery Spytz for the fix!

    @vstinner vstinner closed this as completed Jun 7, 2019
    @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
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @ericvsmith