classification
Title: Wrong return value from PyLong_AsUnsignedLongLongMask on PyErr_BadInternalCall
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.9, Python 3.8, Python 3.7, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, eric.smith, mark.dickinson, vstinner, ztane
Priority: normal Keywords: patch

Created on 2019-06-06 06:52 by ztane, last changed 2019-06-07 16:23 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 13860 merged ZackerySpytz, 2019-06-06 10:16
PR 13891 merged ZackerySpytz, 2019-06-07 12:37
PR 13896 merged ZackerySpytz, 2019-06-07 14:46
PR 13898 merged ZackerySpytz, 2019-06-07 16:07
Messages (12)
msg344789 - (view) Author: Antti Haapala (ztane) * Date: 2019-06-06 06:52
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`
msg344790 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-06 06:54
-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
msg344791 - (view) Author: Antti Haapala (ztane) * Date: 2019-06-06 07:19
Victor, as a friendly reminder, (unsigned long)-1 is not necessarily the same number as (unsigned long long)-1. The documentation means the latter.
msg344802 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2019-06-06 11:43
As ztane says, the issue isn't the -1, it's the type of the cast. This looks like a legitimate issue to me.
msg344806 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-06 12:22
> 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?
msg344809 - (view) Author: Antti Haapala (ztane) * Date: 2019-06-06 12:29
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
msg344818 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-06 14:26
Python 2.7 - 3.9 are affected. Python 3.6 no longer accept bugfixes.
msg344874 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-06 20:39
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
msg344943 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-07 14:23
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
msg344953 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-07 15:41
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
msg344961 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-07 16:23
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
msg344962 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-07 16:23
Thanks Antti Haapala for the bug report and thanks Zackery Spytz for the fix!
History
Date User Action Args
2019-06-07 16:23:38vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg344962

stage: patch review -> resolved
2019-06-07 16:23:01vstinnersetmessages: + msg344961
2019-06-07 16:07:19ZackerySpytzsetpull_requests: + pull_request13773
2019-06-07 15:41:32vstinnersetmessages: + msg344953
2019-06-07 14:46:42ZackerySpytzsetpull_requests: + pull_request13771
2019-06-07 14:23:01vstinnersetmessages: + msg344943
2019-06-07 12:37:05ZackerySpytzsetkeywords: + patch
pull_requests: + pull_request13765
2019-06-06 20:39:40vstinnersetmessages: + msg344874
2019-06-06 17:05:58ZackerySpytzsetnosy: + ZackerySpytz

versions: - Python 3.6
2019-06-06 14:26:02vstinnersetmessages: + msg344818
versions: - Python 3.5
2019-06-06 13:09:09mark.dickinsonsetnosy: + mark.dickinson
2019-06-06 12:29:41ztanesetmessages: + msg344809
2019-06-06 12:22:33vstinnersetmessages: + msg344806
2019-06-06 11:43:08eric.smithsetstatus: closed -> open

nosy: + eric.smith
messages: + msg344802

resolution: not a bug -> (no value)
stage: resolved -> patch review
2019-06-06 10:16:48ZackerySpytzsetpull_requests: + pull_request13734
2019-06-06 07:19:18ztanesetmessages: + msg344791
2019-06-06 06:54:48vstinnersetstatus: open -> closed

nosy: + vstinner
messages: + msg344790

resolution: not a bug
stage: resolved
2019-06-06 06:52:41ztanecreate