classification
Title: potential silent truncation in PyLong_AsVoidPtr
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Oren Milman, mark.dickinson, serhiy.storchaka, vstinner
Priority: normal Keywords:

Created on 2017-03-04 16:12 by Oren Milman, last changed 2017-03-27 12:15 by vstinner. This issue is now closed.

Messages (7)
msg288984 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-03-04 16:12
I am not sure whether such a platform exists, but on a platform where
SIZEOF_VOID_P < SIZEOF_LONG, PyLong_AsVoidPtr (which is in
Objects/longobject.c) is:
    long x;
    if (PyLong_Check(vv) && _PyLong_Sign(vv) < 0)
        x = PyLong_AsLong(vv);
    else
        x = PyLong_AsUnsignedLong(vv);
    if (x == -1 && PyErr_Occurred())
        return NULL;
    return (void *)x;
Thus, for example, 'PyLong_AsVoidPtr(PyLong_FromUnsignedLong(ULONG_MAX))'
would silently truncate ULONG_MAX, and return without an error.

An easy fix would be (mainly) to add to PyLong_AsVoidPtr
'Py_BUILD_ASSERT(SIZEOF_LONG <= SIZEOF_VOID_P);', but I am not sure we can
make that assumption.

Note that a compile time error is already raised:
    - by Objects/longobject.h, in case SIZEOF_VOID_P is different from
      SIZEOF_INT, SIZEOF_LONG and SIZEOF_LONG_LONG
    - by Modules/_multiprocessing/multiprocessing.h, in case SIZEOF_VOID_P is
      different from SIZEOF_LONG and SIZEOF_LONG_LONG
msg290487 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-25 16:21
I don't think there is a bug. There are several ways to get the integer representation of the pointer. PyLong_FromVoidPtr() is the most straightforward. From Python side id() uses it. printf("%p") is a way of getting string representation of the pointer, that string can be converted to integer. This way is used in object.__repr__(). Usually %p formats the pointer as hexadecimal, but the sign is not defined. Therefore when the pointer is converted to the integer it can be interpreted as either signed or unsigned integer. PyLong_AsVoidPtr() should accept both representations.
msg290586 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-03-27 11:17
"I am not sure whether such a platform exists, but on a platform where SIZEOF_VOID_P < SIZEOF_LONG, PyLong_AsVoidPtr ..."

I'm suite sure that every C function of CPython breaks if this assumption becomes wrong :-) Such platform doesn't exist.

You can add the build assertion if it makes you more confortable ;-)
msg290593 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-27 11:50
> I'm suite sure that every C function of CPython breaks if this assumption becomes wrong :-) Such platform doesn't exist.

Did you meant 64-bit Windows? ;-)
msg290594 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-03-27 11:55
> Did you meant 64-bit Windows? ;-)

On Windows 64-bit, sizeof(void*) > sizeof(long), not the opposite.
msg290599 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-27 12:04
Ah, you are right. I don't know whether Python supports such platform after dropping support of DOS.

But in any case I don't see any issue here.

Oren will be not active until August, so we should make the decision about closing the issue or changing the code without him.
msg290602 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-03-27 12:15
I closed the issue.

@Oren: Please reopen the issue with a pull request if you consider that the assertion is worth it.
History
Date User Action Args
2017-03-27 12:15:25vstinnersetstatus: open -> closed

messages: + msg290602
stage: resolved
2017-03-27 12:04:16serhiy.storchakasetmessages: + msg290599
2017-03-27 11:55:21vstinnersetmessages: + msg290594
2017-03-27 11:50:34serhiy.storchakasetmessages: + msg290593
2017-03-27 11:17:53vstinnersetnosy: + vstinner
messages: + msg290586
2017-03-25 16:21:49serhiy.storchakasetnosy: + mark.dickinson, serhiy.storchaka
resolution: not a bug
messages: + msg290487
2017-03-04 16:12:04Oren Milmancreate