Skip to content

Commit d448889

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

6 files changed

Lines changed: 55 additions & 11 deletions

File tree

‎Include/cpython/object.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,15 @@ PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);
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'.
351351
*/
352-
#ifdef _Py_TYPEOF
352+
#if defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
353+
#define Py_SETREF(dst, src) \
354+
do { \
355+
auto _tmp_dst_ptr = &(dst); \
356+
auto _tmp_old_dst = (*_tmp_dst_ptr); \
357+
*_tmp_dst_ptr = (src); \
358+
Py_DECREF(_tmp_old_dst); \
359+
} while (0)
360+
#elif defined(_Py_TYPEOF)
353361
#define Py_SETREF(dst, src) \
354362
do { \
355363
_Py_TYPEOF(&(dst)) _tmp_dst_ptr = &(dst); \

‎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))
552550
# define _Py_TYPEOF(expr) __typeof__(expr)
553551
#endif
554552

‎Include/refcount.h‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,8 +478,21 @@ 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
*/
482-
#ifdef _Py_TYPEOF
485+
#if defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
486+
#define Py_CLEAR(op) \
487+
do { \
488+
auto _tmp_op_ptr = &(op); \
489+
auto _tmp_old_op = (*_tmp_op_ptr); \
490+
if (_tmp_old_op != _Py_NULL) { \
491+
*_tmp_op_ptr = _Py_NULL; \
492+
Py_DECREF(_tmp_old_op); \
493+
} \
494+
} while (0)
495+
#elif defined(_Py_TYPEOF)
483496
#define Py_CLEAR(op) \
484497
do { \
485498
_Py_TYPEOF(&(op)) _tmp_op_ptr = &(op); \

‎Lib/test/test_cext/extension.c‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ static int
8080
_testcext_exec(PyObject *module)
8181
{
8282
PyObject *result, *obj;
83+
PyObject *slots[1];
8384

8485
#ifdef __STDC_VERSION__
8586
if (PyModule_AddIntMacro(module, __STDC_VERSION__) < 0) {
@@ -105,11 +106,21 @@ _testcext_exec(PyObject *module)
105106
Py_CLEAR(obj);
106107
assert(obj == NULL);
107108

109+
// gh-157649: Test Py_CLEAR() on an array
110+
slots[0] = Py_None;
111+
Py_CLEAR(slots[0]);
112+
assert(slots[0] == _Py_NULL);
113+
108114
#ifndef Py_LIMITED_API
109115
// Test Py_SETREF(): use typeof()/__typeof__() if available, or memcpy()
110116
obj = Py_None;
111117
Py_SETREF(obj, NULL);
112118
assert(obj == NULL);
119+
120+
// gh-157649: Test Py_SETREF() on an array
121+
slots[0] = Py_None;
122+
Py_SETREF(slots[0], _Py_NULL);
123+
assert(slots[0] == _Py_NULL);
113124
#endif
114125

115126
// Test that Py_BEGIN_CRITICAL_SECTION is available

‎Lib/test/test_cppext/extension.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,16 +307,26 @@ _testcppext_exec(PyObject *module)
307307
assert(Py_MAX(5, 11) == 11);
308308
assert(Py_ABS(-5) == 5);
309309

310-
// Test Py_CLEAR(): use typeof()/__typeof__() if available, or memcpy()
310+
// Test Py_CLEAR(): use auto on C++11, or memcpy()
311311
PyObject *obj = Py_None;
312312
Py_CLEAR(obj);
313313
assert(obj == _Py_NULL);
314314

315+
// gh-157649: Test Py_CLEAR() on an array
316+
PyObject *slots[1] = {Py_None};
317+
Py_CLEAR(slots[0]);
318+
assert(slots[0] == _Py_NULL);
319+
315320
#ifndef Py_LIMITED_API
316-
// Test Py_SETREF(): use typeof()/__typeof__() if available, or memcpy()
321+
// Test Py_SETREF(): use auto on C++11, or memcpy()
317322
obj = Py_None;
318323
Py_SETREF(obj, _Py_NULL);
319324
assert(obj == _Py_NULL);
325+
326+
// gh-157649: Test Py_SETREF() on an array
327+
slots[0] = Py_None;
328+
Py_SETREF(slots[0], _Py_NULL);
329+
assert(slots[0] == _Py_NULL);
320330
#endif
321331

322332
// 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)