Skip to content

Commit 3330712

Browse files
authored
gh-157649: Fix Py_CLEAR()/Py_SETREF() on C++ (#158070)
On C++, do not use decltype() in _Py_TYPEOF since it produces invalid code in Py_CLEAR() and Py_SETREF(). Instead, implement Py_CLEAR(), Py_SETREF() and Py_XSETREF() using "auto" on C++11 and newer. Add Py_CLEAR(), Py_SETREF() and Py_XSETREF() tests on an array.
1 parent 6e32490 commit 3330712

5 files changed

Lines changed: 57 additions & 10 deletions

File tree

‎Include/cpython/object.h‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,9 @@ PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);
342342
* `dst` points to a valid object.
343343
*
344344
* Temporary variables are used to only evaluate macro arguments once and so
345-
* avoid the duplication of side effects. _Py_TYPEOF() or memcpy() is used to
346-
* avoid a miscompilation caused by type punning. See Py_CLEAR() comment for
347-
* implementation details about type punning.
345+
* avoid the duplication of side effects. _Py_TYPEOF(), C++ auto, or memcpy()
346+
* is used to avoid a miscompilation caused by type punning. See Py_CLEAR()
347+
* comment for implementation details about type punning.
348348
*
349349
* The memcpy() implementation does not emit a compiler warning if 'src' has
350350
* not the same type than 'src': any pointer type is accepted for 'src'.
@@ -357,6 +357,14 @@ PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);
357357
*_tmp_dst_ptr = (src); \
358358
Py_DECREF(_tmp_old_dst); \
359359
} while (0)
360+
#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
361+
#define Py_SETREF(dst, src) \
362+
do { \
363+
auto _tmp_dst_ptr = &(dst); \
364+
auto _tmp_old_dst = (*_tmp_dst_ptr); \
365+
*_tmp_dst_ptr = (src); \
366+
Py_DECREF(_tmp_old_dst); \
367+
} while (0)
360368
#else
361369
#define Py_SETREF(dst, src) \
362370
do { \
@@ -379,6 +387,14 @@ PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);
379387
*_tmp_dst_ptr = (src); \
380388
Py_XDECREF(_tmp_old_dst); \
381389
} while (0)
390+
#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
391+
#define Py_XSETREF(dst, src) \
392+
do { \
393+
auto _tmp_dst_ptr = &(dst); \
394+
auto _tmp_old_dst = (*_tmp_dst_ptr); \
395+
*_tmp_dst_ptr = (src); \
396+
Py_XDECREF(_tmp_old_dst); \
397+
} while (0)
382398
#else
383399
#define Py_XSETREF(dst, src) \
384400
do { \

‎Include/pyport.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -538,17 +538,15 @@ extern "C" {
538538
//
539539
// Example: _Py_TYPEOF(x) x_copy = (x);
540540
//
541-
// On C23, use typeof(). On C++11, use decltype(). Otherwise, use __typeof__()
541+
// On C23, use typeof(). Otherwise, use __typeof__()
542542
// if on GCC, clang or MSVC 17.9 and newer.
543543
//
544-
// On MSVC, check also _MSVC_LANG since __cplusplus is 199711L unless
545-
// the /Zc:__cplusplus flag is used.
544+
// gh-157649: Do not use decltype() on C++, since it produces invalid code in
545+
// Py_CLEAR()/Py_SETREF().
546546
#if defined (__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
547547
# define _Py_TYPEOF(expr) typeof(expr)
548-
#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
549-
# define _Py_TYPEOF(expr) decltype(expr)
550-
#elif defined(__GNUC__) || defined(__clang__) || \
551-
(defined(_MSC_VER) && _MSC_VER >= 1939)
548+
#elif (defined(__GNUC__) || defined(__clang__) \
549+
|| (defined(_MSC_VER) && _MSC_VER >= 1939 && !defined(__cplusplus)))
552550
# define _Py_TYPEOF(expr) __typeof__(expr)
553551
#endif
554552

‎Include/refcount.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,9 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
478478
* and so avoid type punning. Otherwise, use memcpy() which causes type erasure
479479
* and so prevents the compiler to reuse an old cached 'op' value after
480480
* Py_CLEAR().
481+
*
482+
* On C++11 and newer, use "auto". On MSVC, check also _MSVC_LANG since
483+
* __cplusplus is 199711L unless the /Zc:__cplusplus flag is used.
481484
*/
482485
#ifdef _Py_TYPEOF
483486
#define Py_CLEAR(op) \
@@ -489,6 +492,16 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
489492
Py_DECREF(_tmp_old_op); \
490493
} \
491494
} while (0)
495+
#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
496+
#define Py_CLEAR(op) \
497+
do { \
498+
auto _tmp_op_ptr = &(op); \
499+
auto _tmp_old_op = (*_tmp_op_ptr); \
500+
if (_tmp_old_op != _Py_NULL) { \
501+
*_tmp_op_ptr = _Py_NULL; \
502+
Py_DECREF(_tmp_old_op); \
503+
} \
504+
} while (0)
492505
#else
493506
#define Py_CLEAR(op) \
494507
do { \

‎Lib/test/test_cext/extension.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ static PyObject *
8282
test_macros(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
8383
{
8484
PyObject *obj, *dict;
85+
PyObject *slots[1];
8586

8687
// test Py_BUILD_ASSERT() and Py_BUILD_ASSERT_EXPR()
8788
Py_BUILD_ASSERT(sizeof(int) == sizeof(unsigned int));
@@ -97,16 +98,31 @@ test_macros(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
9798
Py_CLEAR(obj);
9899
assert(obj == _Py_NULL);
99100

101+
// gh-157649: Test Py_CLEAR() on an array
102+
slots[0] = Py_None;
103+
Py_CLEAR(slots[0]);
104+
assert(slots[0] == _Py_NULL);
105+
100106
#ifndef Py_LIMITED_API
101107
// Test Py_SETREF(): use typeof()/__typeof__() if available, or memcpy()
102108
obj = Py_None;
103109
Py_SETREF(obj, _Py_NULL);
104110
assert(obj == _Py_NULL);
105111

112+
// gh-157649: Test Py_SETREF() on an array
113+
slots[0] = Py_None;
114+
Py_SETREF(slots[0], _Py_NULL);
115+
assert(slots[0] == _Py_NULL);
116+
106117
// Test Py_XSETREF(): use typeof()/__typeof__() if available, or memcpy()
107118
obj = Py_None;
108119
Py_XSETREF(obj, _Py_NULL);
109120
assert(obj == _Py_NULL);
121+
122+
// gh-157649: Test Py_XSETREF() on an array
123+
slots[0] = Py_None;
124+
Py_XSETREF(slots[0], _Py_NULL);
125+
assert(slots[0] == _Py_NULL);
110126
#endif
111127

112128
// Test that Py_BEGIN_CRITICAL_SECTION is available
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :c:macro:`Py_CLEAR` and :c:macro:`Py_SETREF` macros on C++: implement
2+
them using ``auto`` instead of ``decltype()``. Using ``decltype()``
3+
produced invalid code when clearing/setting an array item. Patch by Victor
4+
Stinner.

0 commit comments

Comments
 (0)