Py_BUILD_ASSERT is broken on non-constant expression

[namniav]

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 MSVC¹. 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)]) \
            } \
        ))
#endif

You 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

1. 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]>. ↩

[vstinner]

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.

The Python C API should be usable with older C versions.

[vstinner]

I proposed PR gh-118398 to use static_assert() in Py_BUILD_ASSERT() and Py_BUILD_ASSERT_EXPR() on C11 and newer.

[namniav]

The Python C API should be usable with older C versions.

I thought that Include/internal/... is part of C API since I found it in installation location but it turns out I was wrong.

[vstinner]

Fixed in the main branch. It sounds too risky to backport the change.

[kulikjak]

Hi - sorry, a little late to this, but this change broke test_cppext on 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]

I believe that !defined(__cplusplus) should be used to check for C++ instead (the same was done in Mesa several years ago here [2]).

Do you want to propose a fix?

[kulikjak]

It's already here: #121974 :)