Issue 37170: Wrong return value from PyLong_AsUnsignedLongLongMask on PyErr_BadInternalCall

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: https://github.com/python/cpython/blob/cb65202520e7959196a2df8215692de155bf0cc8/Objects/longobject.c#L1379

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: https://github.com/python/cpython/blob/cb65202520e7959196a2df8215692de155bf0cc8/Objects/longobject.c#L1407

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: https://github.com/python/cpython/blob/20093b3adf6b06930fe994527670dfb3aee40cc7/Objects/longobject.c#L1025

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

-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

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

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

> 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?

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_Check`s or similar, as they probably should, but the other is part of public API

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

New changeset dc2476500d91082f0c907772c83a044bf49af279 by Victor Stinner (Zackery Spytz) in branch 'master':
bpo-37170: Fix the cast on error in PyLong_AsUnsignedLongLongMask() (GH-13860)
https://github.com/python/cpython/commit/dc2476500d91082f0c907772c83a044bf49af279

New changeset dd492d9c352eb0fa2bc48ea9acc47e47a7fab8a0 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)
https://github.com/python/cpython/commit/dd492d9c352eb0fa2bc48ea9acc47e47a7fab8a0

New changeset e36ed475ea429f7cc80a4d65f80b44686a74b246 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)
https://github.com/python/cpython/commit/e36ed475ea429f7cc80a4d65f80b44686a74b246

New changeset 2bfc2dc214445550521074f428245b502d215eac 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)
https://github.com/python/cpython/commit/2bfc2dc214445550521074f428245b502d215eac

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

stage: patch review -> resolved

versions: - Python 3.6

nosy: + eric.smith
messages: + msg344802

resolution: not a bug -> (no value)
stage: resolved -> patch review

nosy: + vstinner
messages: + msg344790

resolution: not a bug
stage: resolved