You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Reliance on C bit fields in C API is undefined behavior #89188
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=Noneclosed_at=Nonecreated_at=<Date2021-08-27.01:47:13.094>labels= ['type-bug', '3.8', '3.9', '3.10', '3.11', 'expert-C-API', '3.7']
title='Reliance on C bit fields in C API is undefined behavior'updated_at=<Date2021-08-31.22:02:29.674>user='https://github.com/indygreg'
At least the PyASCIIObject struct in Include/cpython/unicodeobject.h uses bit fields. Various preprocessor macros like PyUnicode_IS_ASCII() and PyUnicode_KIND() access this struct's bit field.
This is problematic because according to the C specification, the storage of bit fields is unspecified and may vary from compiler to compiler or architecture to architecture. Theoretically, a build of libpython with compiler A may not have the same storage layout of a bit field as a separate binary built with compiler B. These 2 binaries could be linked/loaded together, resulting in a crash or incorrect behavior at run-time.
To ensure bit field behavior is consistent, the same compiler must be used for all bit field interaction. Since it is effectively impossible to ensure this for programs like Python where multiple compilers are commonly at play (a 3rd party C extension will likely not be built on the same machine that built libpython), bit fields must not be exposed in the C API. If a bit field must exist, the bit field should not be declared in a public .h header and any APIs for accessing the bit field must be implemented as compiled functions such that only a single compiler will define the bit field storage layout.
In order to avoid undefined behavior, Python's C API should avoid all use of bit fields.
I don't think that PyUnicode_KIND() makes sense if CPython uses UTF-8 tomorrow.
If the internal representation os strings is switched to use masks and shifts instead of bitfields, PyUnicode_KIND (and others) can be adapted to the new details without breaking API compatibility.
PyUnicode_KIND() was exposed in the *public* C API because unicodeobject.h provides functions as macros for best performances, and these macros use PyUnicode_KIND() internally.
Macros like PyUnicode_READ(kind, data, index) are also designed for best performances with the exact PEP-393 implementation.
The public C API should only contain PyUnicode_READ_CHAR(unicode, index): this macro doesn't use "kind" or "data" which are (again) specific to the PEP-393.
In the CPython implementation, we should use the most efficient code, it's fine to use macros accessing directly structures.
But for the public C API, I would recommend to only provide abstractions, even if there are a little bit slower.
indygreg commented on Aug 30, 2021
indygregmannequin
MannequinAuthor
More actions
My use case for these low-level APIs is to write tests for low-level string/encoding handling in my custom use of the PyPreConfig and PyConfig structs. I wanted to verify that exact byte sequences were turned into specific representations inside of Python strings. This includes ensuring that certain byte sequences retain their appropriate "character" width in internal storage.
I know there are alternative ways of performing this testing. But testing against the actual data structure used internally by CPython seemed the most precise since it isolates problems to the "store in Python" side of the problem and not "what does Python do once the data is stored."
My use case for these low-level APIs is to write tests for low-level string/encoding handling in my custom use of the PyPreConfig and PyConfig structs. I wanted to verify that exact byte sequences were turned into specific representations inside of Python strings. This includes ensuring that certain byte sequences retain their appropriate "character" width in internal storage.
CPython contains many checks to ensure that a string always use the most effecient storage, especially in debug mode. The C API should not allow to create a string using an inefficient storage, unless you "abuse" the C API :-D
Activity
indygreg commentedon Aug 27, 2021
At least the PyASCIIObject struct in Include/cpython/unicodeobject.h uses bit fields. Various preprocessor macros like PyUnicode_IS_ASCII() and PyUnicode_KIND() access this struct's bit field.
This is problematic because according to the C specification, the storage of bit fields is unspecified and may vary from compiler to compiler or architecture to architecture. Theoretically, a build of libpython with compiler A may not have the same storage layout of a bit field as a separate binary built with compiler B. These 2 binaries could be linked/loaded together, resulting in a crash or incorrect behavior at run-time.
https://stackoverflow.com/questions/6043483/why-bit-endianness-is-an-issue-in-bitfields/6044223#6044223
To ensure bit field behavior is consistent, the same compiler must be used for all bit field interaction. Since it is effectively impossible to ensure this for programs like Python where multiple compilers are commonly at play (a 3rd party C extension will likely not be built on the same machine that built libpython), bit fields must not be exposed in the C API. If a bit field must exist, the bit field should not be declared in a public .h header and any APIs for accessing the bit field must be implemented as compiled functions such that only a single compiler will define the bit field storage layout.
In order to avoid undefined behavior, Python's C API should avoid all use of bit fields.
This issue is in response to PyO3/pyo3#1824.
7 remaining items
vstinner commentedon Aug 30, 2021
PyUnicode_KIND() is very specific to the exact PEP-393 implementation. Documentation of this field:
---
/* Character size:
PyUnicode_WCHAR_KIND (0):
platform)
PyUnicode_1BYTE_KIND (1):
(ASCII), otherwise at least one character is in the range
U+0080-U+00FF
PyUnicode_2BYTE_KIND (2):
PyUnicode_4BYTE_KIND (4):
I don't think that PyUnicode_KIND() makes sense if CPython uses UTF-8 tomorrow.
PyUnicode_KIND() was exposed in the *public* C API because unicodeobject.h provides functions as macros for best performances, and these macros use PyUnicode_KIND() internally.
Macros like PyUnicode_READ(kind, data, index) are also designed for best performances with the exact PEP-393 implementation.
The public C API should only contain PyUnicode_READ_CHAR(unicode, index): this macro doesn't use "kind" or "data" which are (again) specific to the PEP-393.
In the CPython implementation, we should use the most efficient code, it's fine to use macros accessing directly structures.
But for the public C API, I would recommend to only provide abstractions, even if there are a little bit slower.
indygreg commentedon Aug 30, 2021
My use case for these low-level APIs is to write tests for low-level string/encoding handling in my custom use of the PyPreConfig and PyConfig structs. I wanted to verify that exact byte sequences were turned into specific representations inside of Python strings. This includes ensuring that certain byte sequences retain their appropriate "character" width in internal storage.
I know there are alternative ways of performing this testing. But testing against the actual data structure used internally by CPython seemed the most precise since it isolates problems to the "store in Python" side of the problem and not "what does Python do once the data is stored."
vstinner commentedon Aug 31, 2021
CPython contains many checks to ensure that a string always use the most effecient storage, especially in debug mode. The C API should not allow to create a string using an inefficient storage, unless you "abuse" the C API :-D
I'm not sure what do you test.
pythongh-89188: replace bitfield with struct fields in PyASCIIObject
pythongh-89188: replace bitfield with struct fields in PyASCIIObject
pythongh-89188: add `PyUnicode_Data` and `PyUnicode_GetKind`
ericsnowcurrently commentedon Mar 1, 2024
(Also see the discussion between @davidhewitt and @encukou at https://www.youtube.com/watch?v=E_hczlgDqus.)
vstinner commentedon Mar 2, 2024
See also capi-workgroup/api-evolution#10
pythongh-89188: Implement PyUnicode_KIND() as a function
pythongh-89188: Implement PyUnicode_KIND() as a function
gh-89188: Implement PyUnicode_KIND() as a function (#129412)
vstinner commentedon Jan 30, 2025
Fixed by the change a810cb8.
pythongh-89188: Implement PyUnicode_KIND() as a function (python#129412)