Repository navigation
Conversation
Py_SETREF(), Py_XSETREF() and Py_CLEAR() now use memcpy() to set _tmp_old_dst/_tmp_old_op to fix a strict aliasing issue. Py_SETREF(), Py_XSETREF() and Py_CLEAR() now also cast _tmp_old_dst/_tmp_old_op to PyObject* when calling Py_DECREF()/Py_XDECREF() to avoid compiler warnings with the limited C API. Co-Authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Member
Author
Member
Author
|
To reproduce #161185 issue:
=> The test does crash (segfault) or fails with a C assertion error. Patch to force the memcpy() implementation and reintroduce the strict aliasing bug: Detailsdiff --git a/Include/refcount.h b/Include/refcount.h
index 2bc2e83ef1c..7e5e9e46117 100644
--- a/Include/refcount.h
+++ b/Include/refcount.h
@@ -482,7 +482,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
* On C++11 and newer, use "auto". On MSVC, check also _MSVC_LANG since
* __cplusplus is 199711L unless the /Zc:__cplusplus flag is used.
*/
-#ifdef _Py_TYPEOF
+#if 0 // def _Py_TYPEOF
#define Py_CLEAR(op) \
do { \
_Py_TYPEOF(&(op)) _tmp_op_ptr = &(op); \
@@ -492,7 +492,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
Py_DECREF(_PyObject_CAST(_tmp_old_op)); \
} \
} while (0)
-#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
+#elif 0 // defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
#define Py_CLEAR(op) \
do { \
auto _tmp_op_ptr = &(op); \
@@ -506,8 +506,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
#define Py_CLEAR(op) \
do { \
PyObject **_tmp_op_ptr = _Py_CAST(PyObject**, &(op)); \
- PyObject *_tmp_old_op; \
- memcpy(&_tmp_old_op, _tmp_op_ptr, sizeof(PyObject*)); \
+ PyObject *_tmp_old_op = (*_tmp_op_ptr); \
if (_tmp_old_op != _Py_NULL) { \
PyObject *_null_ptr = _Py_NULL; \
memcpy(_tmp_op_ptr, &_null_ptr, sizeof(PyObject*)); \
@@ -540,7 +539,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
* The memcpy() implementation does not emit a compiler warning if 'src' has
* not the same type than 'src': any pointer type is accepted for 'src'.
*/
-#ifdef _Py_TYPEOF
+#if 0 // def _Py_TYPEOF
#define Py_SETREF(dst, src) \
do { \
_Py_TYPEOF(&(dst)) _tmp_dst_ptr = &(dst); \
@@ -548,7 +547,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
*_tmp_dst_ptr = (src); \
Py_DECREF(_PyObject_CAST(_tmp_old_dst)); \
} while (0)
-#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
+#elif 0 // defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
#define Py_SETREF(dst, src) \
do { \
auto _tmp_dst_ptr = &(dst); \
@@ -560,8 +559,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
#define Py_SETREF(dst, src) \
do { \
PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
- PyObject *_tmp_old_dst; \
- memcpy(&_tmp_old_dst, _tmp_dst_ptr, sizeof(PyObject*)); \
+ PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \
PyObject *_tmp_src = _PyObject_CAST(src); \
memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject*)); \
Py_DECREF(_tmp_old_dst); \
@@ -571,7 +569,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
/* Py_XSETREF() is a variant of Py_SETREF() that uses Py_XDECREF() instead of
* Py_DECREF().
*/
-#ifdef _Py_TYPEOF
+#if 0 // def _Py_TYPEOF
#define Py_XSETREF(dst, src) \
do { \
_Py_TYPEOF(&(dst)) _tmp_dst_ptr = &(dst); \
@@ -579,7 +577,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
*_tmp_dst_ptr = (src); \
Py_XDECREF(_PyObject_CAST(_tmp_old_dst)); \
} while (0)
-#elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
+#elif 0 // defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L)
#define Py_XSETREF(dst, src) \
do { \
auto _tmp_dst_ptr = &(dst); \
@@ -591,8 +589,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op)
#define Py_XSETREF(dst, src) \
do { \
PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
- PyObject *_tmp_old_dst; \
- memcpy(&_tmp_old_dst, _tmp_dst_ptr, sizeof(PyObject*)); \
+ PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \
PyObject *_tmp_src = _PyObject_CAST(src); \
memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject*)); \
Py_XDECREF(_tmp_old_dst); \ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Py_SETREF(), Py_XSETREF() and Py_CLEAR() now use memcpy() to set _tmp_old_dst/_tmp_old_op to fix a strict aliasing issue.
Py_SETREF(), Py_XSETREF() and Py_CLEAR() now also cast _tmp_old_dst/_tmp_old_op to PyObject* when calling Py_DECREF()/Py_XDECREF() to avoid compiler warnings with the limited C API.