python / cpython

The Python programming language
https://www.python.org/
Other
61.32k stars 29.56k forks source link

PyModule_AddIntConstant() wraps >=2^31 values when long is 4 bytes #72402

Open c1dfce53-4c86-41aa-8be2-81288cc14748 opened 7 years ago

c1dfce53-4c86-41aa-8be2-81288cc14748 commented 7 years ago
BPO 28215
Nosy @vstinner, @tiran, @altendky

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: ```python assignee = None closed_at = None created_at = labels = ['type-bug'] title = 'PyModule_AddIntConstant() wraps >=2^31 values when long is 4 bytes' updated_at = user = 'https://github.com/altendky' ``` bugs.python.org fields: ```python activity = actor = 'altendky' assignee = 'none' closed = False closed_date = None closer = None components = [] creation = creator = 'altendky' dependencies = [] files = [] hgrepos = [] issue_num = 28215 keywords = [] message_count = 5.0 messages = ['277036', '277044', '277051', '277052', '277055'] nosy_count = 3.0 nosy_names = ['vstinner', 'christian.heimes', 'altendky'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue28215' versions = ['Python 3.5'] ```

c1dfce53-4c86-41aa-8be2-81288cc14748 commented 7 years ago

I am cross compiling Python 3.5.2 for use on a 32-bit ARM processor with Linux. I use socket.CAN_EFF_FLAG and noticed that it is negative on the target despite being positive on my host (64-bit Intel Linux).

Host:

altendky@tp:~$ uname -a Linux tp 4.4.0-31-generic #50-Ubuntu SMP Wed Jul 13 00:07:12 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux altendky@tp:~$ python3 --version Python 3.5.2 altendky@tp:~$ python3 -c 'import socket; print(socket.CAN_EFF_FLAG)' 2147483648

^^ Is expected

Target:

root@rosi \~$ uname -a Linux rosi 3.0.101-rt130-opusa3-2.1.0-2 #1 PREEMPT Tue Apr 12 13:49:26 CEST 2016 armv6l GNU/Linux root@rosi \~$ /opt/epc/bin/python3 --version Python 3.5.2 root@rosi \~$ /opt/epc/bin/python3 -c 'import socket; print(socket.CAN_EFF_FLAG)' -2147483648

^^ Is not expected to be negative

Only CAN_EFF_FLAG reference in my source used to cross build Python:

Modules/socketmodule.c: PyModule_AddIntMacro(m, CAN_EFF_FLAG);

Definition in cross compiler include:

altendky@tp:/opt/freescale/usr/local/gcc-4.6.2-glibc-2.13-linaro-multilib-2011.12/fsl-linaro-toolchain/arm-fsl-linux-gnueabi/multi-libs/default/usr/include$ grep -r CAN_EFF_FLAG linux/can.h:#define CAN_EFF_FLAG 0x80000000U / EFF/SFF is set in the MSB \/

For reference, here it is on the host system (looks the same to me):

altendky@tp:/usr/include$ grep -r CAN_EFF_FLAG linux/can.h:#define CAN_EFF_FLAG 0x80000000U / EFF/SFF is set in the MSB \/

But perhaps this long type for the value is the issue? If signed and only 4-bytes as is the case on my target then this will misinterpret the 0x80000000U literal resulting in the above observed -2147483648.

PyModule_AddIntConstant(PyObject *m, const char *name, long value)

On my target system, printf("%d", sizeof(long)) yields 4.

For now I just work around it in my application by reassigning it to be it's absolute value.

socket.CAN_EFF_FLAG = abs(socket.CAN_EFF_FLAG)
tiran commented 7 years ago

The constant is an unsigned long but PyModule_AddIntConstant() takes a signed long. You have to write your own function that uses PyLong_FromUnsignedLong() and PyModule_AddObject().

Do you get a compiler warning because 0x80000000U is larger than (1\<\<31)-1?

tiran commented 7 years ago

Oh sorry, I missed the point that you are talking about an existing constant in the socket module. At first I thought that you were referring to a constant that you have added.

It sounds like a bug for constants >= 2**31.

c1dfce53-4c86-41aa-8be2-81288cc14748 commented 7 years ago

I do not seem to be getting a compiler warning.

arm-fsl-linux-gnueabi-gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Werror=declaration-after-statement -I. -IInclude -I./Include -I"/epc/t/262/misc-build-arm-fsl-linux-gnueabi/sysroot/root/all//opt/epc/include" -DPy_BUILD_CORE -c ./Modules/socketmodule.c -o Modules/socketmodule.o

To really encompass all cases I think you are correct that both a signed and an unsigned handler are needed. Though, I have an idea for something nifty, I'll share if it works.

Regardless, shouldn't they use intmax_t and uintmax_t from stdtypes.h to make sure they handle anything that could be defined in the referenced C code? In my case simply switching from long to intmax_t would be sufficient.

Note that I am not worried about getting this fixed for my one case. My workaround is fine for my application.

I also will hopefully be correcting the subject to >=2**31 if this change does what I think. Good ol' off-by-one.

c1dfce53-4c86-41aa-8be2-81288cc14748 commented 7 years ago

A little macro funny business gets a function the ability to know if the type passed to its wrapping macro is signed or not.

http://ideone.com/NZYs7u

<snippet>
  // http://stackoverflow.com/questions/7469915
  #define IS_UNSIGNED(v) (v >= 0 && ~v >= 0)
  #define F(v) f(IS_UNSIGNED(v), v, v)
  void f (bool is_unsigned, intmax_t s, uintmax_t u)

Looking in [Objects/longobject.c](https://github.com/python/cpython/blob/main/Objects/longobject.c) suggests that perhaps the two functions that could be chosen from would be PyLong_From[Unsigned]LongLong() to avoid truncation. Is there some reason not to use these? I don't know the habits of CPython developers to know if there's a significant optimization going on here.

Just to throw it out there, in the case of macros, PyLong_FromString() might even be usable...

Included for quick reference:

    int PyModule_AddIntConstant(PyObject *m, const char *name, long value)
  https://hg.python.org/cpython/file/tip/Python/modsupport.c#l566

    PyModule_AddIntMacro(m, CAN_EFF_FLAG);
  https://hg.python.org/cpython/file/tip/Modules/socketmodule.c#l7098
PyObject * PyLong_FromLong(long ival)

https://hg.python.org/cpython/file/tip/Objects/longobject.c#l231

    #define PyModule_AddIntMacro(m, c) PyModule_AddIntConstant(m, #c, c)
  https://hg.python.org/cpython/file/tip/Include/modsupport.h#l80