Repository navigation
Py_BUILD_ASSERT is broken on non-constant expression #118124
Description
Bug report
Bug description:
Macros Py_BUILD_ASSERT and Py_BUILD_ASSERT_EXPR defined in Include/pymacro.h are broken when cond is not a constant expression, because sizeof is allowed to apply on variable-length arrays(VLA) since C99 and with compiler extension since C89.
/* Assert a build-time dependency, as an expression.
Your compile will fail if the condition isn't true, or can't be evaluated
by the compiler. This can be used in an expression: its value is 0.
Example:
#define foo_to_char(foo) \
((char *)(foo) \
+ Py_BUILD_ASSERT_EXPR(offsetof(struct foo, string) == 0))
Written by Rusty Russell, public domain, http://ccodearchive.net/ */
#define Py_BUILD_ASSERT_EXPR(cond) \
(sizeof(char [1 - 2*!(cond)]) - 1)
#define Py_BUILD_ASSERT(cond) do { \
(void)Py_BUILD_ASSERT_EXPR(cond); \
} while(0)Following code compiles on Clang and GCC without error:
void foo(int param)
{
Py_BUILD_ASSERT(param > 0);
}Since CPython is now using C11 (PEP7) and static_assert is already used in other public internal headers(e.g. Include/internal/pycore_long.h), Py_BUILD_ASSERT should be deprecated and use static_assert instead.
Since they are defined in a public header of CPython without a "_Py" prefix, removing them might break third-party code.
Py_BUILD_ASSERT can be defined to static_assert in a way with deprecating warning message:
#define Py_BUILD_ASSERT(cond) do { \
Py_DEPRECATED(3.14) \
int _Py_BUILD_ASSERT_is_deprecated_use_static_assert_instead = 0; \
_Py_BUILD_ASSERT_is_deprecated_use_static_assert_instead; \
static_assert(cond, ""); \
} while(0)Py_BUILD_ASSERT_EXPR is used (and only used) in another macro Py_ARRAY_LENGTH, so it can't be deprecated yet.
Fixing Py_BUILD_ASSERT_EXPR is tricky, most because of old non-conformant MSVC1. And we need it also working in C++. The easiest fix is to change documentation so that it should be used only with constant expression or there's no assertion otherwise. But If we want to make it mandatory, here's a workaround:
#if defined(__cplusplus)
template<typename T>
struct _Py_BUILD_ASSERT_EXPR_prohibit_vla {
static_assert(sizeof(T) == 1,
"Py_BUILD_ASSERT_EXPR can only be used with constant "
"expression of value true");
};
# define Py_BUILD_ASSERT_EXPR(cond) \
(!sizeof(_Py_BUILD_ASSERT_EXPR_prohibit_vla<char[1 - 2 * !(cond)]>))
#elif defined(_MSC_VER)
# define Py_BUILD_ASSERT_EXPR(cond) \
(!sizeof( \
__pragma(warning(push)) \
__pragma(warning(suppress: 4116)) \
enum { \
Py_CONCAT(_Py_BUILD_ASSERT_EXPR_prohibit_vla_,__LINE__) = \
sizeof(char[1 - 2 * !(cond)]) \
} \
__pragma(warning(pop)) \
))
#else
# define Py_BUILD_ASSERT_EXPR(cond) \
(!sizeof( \
enum { \
Py_CONCAT(_Py_BUILD_ASSERT_EXPR_prohibit_vla_,__LINE__) = \
sizeof(char[1 - 2 * !(cond)]) \
} \
))
#endifYou can view and try the workarounds using the amazing conformance-viewer in amazing Compiler Explorer: for C and for C++.
CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs
- GH-118124: Fix Py_BUILD_ASSERT on non-constant expression #118125
- gh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11 #118398
- gh-118124: fix assert related C++ checks on Solaris/Illumos #121974
- [3.13] gh-118124: fix assert related C++ checks on Solaris/Illumos (GH-121974) #122108
- [3.12] gh-118124: fix assert related C++ checks on Solaris/Illumos (GH-121974) #122109
Footnotes
-
One of the unbelievable bugs is that MSVC prior to v19.21 accepts negative-length(implicitly extended to unsigned value) arrays when used as template argument, e.g.
A<char[-1]>. ↩
Activity
vstinner commentedon Apr 29, 2024
The Python C API should be usable with older C versions.
pythongh-118124: Use static_assert() in Py_BUILD_ASSERT()
pythongh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11
pythongh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11
vstinner commentedon Apr 29, 2024
I proposed PR gh-118398 to use static_assert() in Py_BUILD_ASSERT() and Py_BUILD_ASSERT_EXPR() on C11 and newer.
namniav commentedon Apr 29, 2024
I thought that
Include/internal/...is part of C API since I found it in installation location but it turns out I was wrong.pythongh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11
gh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11 (#118398)
vstinner commentedon Apr 30, 2024
Fixed in the main branch. It sounds too risky to backport the change.
pythongh-118124: Use static_assert() in Py_BUILD_ASSERT() on C11 (pyt…
kulikjak commentedon Jul 18, 2024
Hi - sorry, a little late to this, but this change broke
test_cppexton Solaris/Illumos. The issue seems to be that__STDC_VERSION__can be defined when compiling C++ as well (the standard doesn't forbid that) and it is defined on Solaris/Illumos [1][2].The check was already broken with 96e1901 before, but I only encountered it recently when trying to compile Python C++ extension that used
static_assert. After this change however, the Python tests won't pass without some modifications.I believe that
!defined(__cplusplus)should be used to check for C++ instead (the same was done in Mesa several years ago here [2]).[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57025
[2] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/2258/diffs?commit_id=78cd9c60eef40650bba1182d8bb51fc9beb938e2
(should I create a new issue considering that this one is closed for a long time?)
vstinner commentedon Jul 20, 2024
Do you want to propose a fix?
kulikjak commentedon Jul 21, 2024
It's already here: #121974 :)
gh-118124: fix assert related C++ checks on Solaris/Illumos (#121974)
[3.13] pythongh-118124: fix assert related C++ checks on Solaris/Illu…
[3.12] gh-118124: fix assert related C++ checks on Solaris/Illumos (G…
[3.13] gh-118124: fix assert related C++ checks on Solaris/Illumos (G…