PyTupleObject::ob_item[1] with out-of-bounds access => undefined behavior

[MaskRay]

Include/cpython/tupleobject.h has

typedef struct {
    PyObject_VAR_HEAD
    /* ob_item contains space for 'ob_size' elements.
       Items must normally not be NULL, except during construction when
       the tuple is not yet visible outside the function that builds it. */
    PyObject *ob_item[1];
} PyTupleObject;

CPython may allocate the object with trailing elements, then access it with something like
PyUnicode_InternInPlace(&_PyTuple_ITEMS(tuple)[i]); where i > 0.

This out-of-bounds access is UB. (https://stackoverflow.com/questions/44745677/flexible-array-members-can-lead-to-undefined-behavior mentioned that before C99 TC2 there was a non-normative example which suggested that [1] can be used. That was incorrect and was removed by TC2.)

The 2022-06-24 Clang -fstrict-flex-arrays commit (https://reviews.llvm.org/D126864 https://reviews.llvm.org/rG886715af962de2c92fac4bd37104450345711e4a) made -fsanitize=array-bounds stricter and would catch such UB. Note: the Clang patch appears non-comprehensive. It misses many similar UB cases but catches the CPython UB.

Reproduce (with clang compiled from latest llvm-project):

% mkdir -p out/bounds && cd out/bounds
% ../../configure CC=/tmp/RelA/bin/clang CXX=/tmp/RelA/bin/clang++ CFLAGS=-fsanitize=bounds LDFLAGS=-fsanitize=bounds
% make -j 60
 CC='/tmp/RelA/bin/clang' LDSHARED='/tmp/RelA/bin/clang -shared -fsanitize=bounds   ' OPT='-DNDEBUG -g -fwrapv -O3 -Wall'       ./python -E ../../setup.py  build
../../Objects/codeobject.c:49:34: runtime error: index 2 out of bounds for type 'PyObject *[1]' (aka 'struct _object *[1]')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../Objects/codeobject.c:49:34 in 
running build
running build_ext

See also https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html GCC appears to be more permissive, at least til now: "Although using one-element arrays this way is discouraged, GCC handles accesses to trailing one-element array members analogously to zero-length arrays."

There are multiple suspicious places in CPython:

% rg 'ob_.*\[1\]'
Include/memoryobject.h
65:    Py_ssize_t ob_array[1];       /* shape, strides, suboffsets */

Tools/gdb/libpython.py
876:               digit ob_digit[1];

Include/cpython/tupleobject.h
10:    PyObject *ob_item[1];

Include/cpython/longintrepr.h
81:    digit ob_digit[1];

Include/cpython/bytesobject.h
8:    char ob_sval[1];

[MojoVampire]

This usage of a length 1 array as the pseudo-flexible array member was to allow compiling the code as either C or C++; C++ has never had a concept of a flexible array member, and while yes, it's undefined behavior, it's one of those weird cases of "technically undefined but actually works everywhere" behavior; it worked in C90, C99 and every C++ compiler available when the code was written, while using [] or [0] is not even valid code in C++ (See: https://stackoverflow.com/a/4413035 ). WINAPI relies on this feature too (having been designed before C supported FAM as well). PEP7 states "The public C API should be compatible with C++.", and while these structures aren't part of the limited API, they remain part of the public API so any other approach violates that rule.

I can't say whether this decision needs revisiting, but it would be a major change that would break huge swathes of third-party extension modules written in C++ rather than C.

[MaskRay]

While P1039 I got you, FAM: Flexible Array Members for C++ has been rejected, GCC and Clang seem to support T[] for a very long time (at least GCC 4.1 and Clang 3.0) in C++ mode. According to godbolt, recent MSVC versions support T[] as well. The only downside may be just a -pedantic warning ( which can be ignored with a pragma).

[arhadthedev]

GCC and Clang seem to support T[] for a very long time (at least GCC 4.1 and Clang 3.0)

We need to account for way more compilers in existance (Visual Studio's msvc, Intel C++, Oracle C++ - that's what I've worked with while my parallel systems university course; IBM XLC++, and more).

[kumaraditya303]

This is a duplicate of #84301. The discussion can be continued there.