Wrong return value from PyLong_AsUnsignedLongLongMask on PyErr_BadInternalCall

[ztane]

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]

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

[vstinner]

-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

[ztane]

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]

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

[vstinner]

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]

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]

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

[vstinner]

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]

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]

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]

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]

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