From da5eb5a31c31d0ba18a6b5f36be6ee40a01a2f2c Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Mon, 27 May 2013 14:46:14 -0700 Subject: [PATCH] don't expand the operand to Py_XINCREF/XDECREF/CLEAR/DECREF multiple times (closes #17206) A patch from Illia Polosukhin. --- Include/object.h | 34 +++++++++++++++++------------ Misc/ACKS | 1 + Misc/NEWS | 4 ++++ Modules/_testcapimodule.c | 46 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/Include/object.h b/Include/object.h index ad58450fb91..221b4a20fd7 100644 --- a/Include/object.h +++ b/Include/object.h @@ -680,12 +680,6 @@ is not considered to be a reference to the type object, to save complications in the deallocation function. (This is actually a decision that's up to the implementer of each new type so if you want, you can count such references to the type object.) - -*** WARNING*** The Py_DECREF macro must have a side-effect-free argument -since it may evaluate its argument multiple times. (The alternative -would be to mace it a proper function or assign it to a global temporary -variable first, both of which are slower; and in a multi-threaded -environment the global variable trick is not safe.) */ /* First define a pile of simple helper macros, one set per special @@ -764,15 +758,16 @@ PyAPI_FUNC(void) _Py_Dealloc(PyObject *); #define Py_INCREF(op) ( \ _Py_INC_REFTOTAL _Py_REF_DEBUG_COMMA \ - ((PyObject*)(op))->ob_refcnt++) + ((PyObject *)(op))->ob_refcnt++) #define Py_DECREF(op) \ do { \ + PyObject *_py_decref_tmp = (PyObject *)(op); \ if (_Py_DEC_REFTOTAL _Py_REF_DEBUG_COMMA \ - --((PyObject*)(op))->ob_refcnt != 0) \ - _Py_CHECK_REFCNT(op) \ + --(_py_decref_tmp)->ob_refcnt != 0) \ + _Py_CHECK_REFCNT(_py_decref_tmp) \ else \ - _Py_Dealloc((PyObject *)(op)); \ + _Py_Dealloc(_py_decref_tmp); \ } while (0) /* Safely decref `op` and set `op` to NULL, especially useful in tp_clear @@ -811,16 +806,27 @@ PyAPI_FUNC(void) _Py_Dealloc(PyObject *); */ #define Py_CLEAR(op) \ do { \ - if (op) { \ - PyObject *_py_tmp = (PyObject *)(op); \ + PyObject *_py_tmp = (PyObject *)(op); \ + if (_py_tmp != NULL) { \ (op) = NULL; \ Py_DECREF(_py_tmp); \ } \ } while (0) /* Macros to use in case the object pointer may be NULL: */ -#define Py_XINCREF(op) do { if ((op) == NULL) ; else Py_INCREF(op); } while (0) -#define Py_XDECREF(op) do { if ((op) == NULL) ; else Py_DECREF(op); } while (0) +#define Py_XINCREF(op) \ + do { \ + PyObject *_py_xincref_tmp = (PyObject *)(op); \ + if (_py_xincref_tmp != NULL) \ + Py_INCREF(_py_xincref_tmp); \ + } while (0) + +#define Py_XDECREF(op) \ + do { \ + PyObject *_py_xdecref_tmp = (PyObject *)(op); \ + if (_py_xdecref_tmp != NULL) \ + Py_DECREF(_py_xdecref_tmp); \ + } while (0) /* These are provided as conveniences to Python runtime embedders, so that diff --git a/Misc/ACKS b/Misc/ACKS index 578c59df9e5..3c9e24e53a5 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -983,6 +983,7 @@ Oleg Plakhotnyuk Remi Pointel Ariel Poliak Guilherme Polo +Illia Polosukhin Michael Pomraning Iustin Pop Claudiu Popa diff --git a/Misc/NEWS b/Misc/NEWS index 7dabb2e4dfa..737672a2004 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,10 @@ What's New in Python 3.4.0 Alpha 1? Core and Builtins ----------------- +- Issue #17206: Py_CLEAR(), Py_DECREF(), Py_XINCREF() and Py_XDECREF() now + expands their arguments once instead of multiple times. + Patch written by Illia Polosukhin. + - Issue #17937: Try harder to collect cyclic garbage at shutdown. - Issue #12370: Prevent class bodies from interfering with the __class__ diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9a14fa2b790..b3ed0474b0a 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2468,6 +2468,48 @@ test_pytime_object_to_timespec(PyObject *self, PyObject *args) return Py_BuildValue("Nl", _PyLong_FromTime_t(sec), nsec); } +static PyObject * +_test_incref(PyObject *ob) +{ + Py_INCREF(ob); + return ob; +} + +static PyObject * +test_xincref_doesnt_leak(PyObject *ob) +{ + PyObject *obj = PyLong_FromLong(0); + Py_XINCREF(_test_incref(obj)); + Py_DECREF(obj); + Py_DECREF(obj); + Py_DECREF(obj); + Py_RETURN_NONE; +} + +static PyObject * +test_incref_doesnt_leak(PyObject *ob) +{ + PyObject *obj = PyLong_FromLong(0); + Py_INCREF(_test_incref(obj)); + Py_DECREF(obj); + Py_DECREF(obj); + Py_DECREF(obj); + Py_RETURN_NONE; +} + +static PyObject * +test_xdecref_doesnt_leak(PyObject *ob) +{ + Py_XDECREF(PyLong_FromLong(0)); + Py_RETURN_NONE; +} + +static PyObject * +test_decref_doesnt_leak(PyObject *ob) +{ + Py_DECREF(PyLong_FromLong(0)); + Py_RETURN_NONE; +} static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, @@ -2478,6 +2520,10 @@ static PyMethodDef TestMethods[] = { {"test_dict_iteration", (PyCFunction)test_dict_iteration,METH_NOARGS}, {"test_lazy_hash_inheritance", (PyCFunction)test_lazy_hash_inheritance,METH_NOARGS}, {"test_long_api", (PyCFunction)test_long_api, METH_NOARGS}, + {"test_xincref_doesnt_leak",(PyCFunction)test_xincref_doesnt_leak, METH_NOARGS}, + {"test_incref_doesnt_leak", (PyCFunction)test_incref_doesnt_leak, METH_NOARGS}, + {"test_xdecref_doesnt_leak",(PyCFunction)test_xdecref_doesnt_leak, METH_NOARGS}, + {"test_decref_doesnt_leak", (PyCFunction)test_decref_doesnt_leak, METH_NOARGS}, {"test_long_and_overflow", (PyCFunction)test_long_and_overflow, METH_NOARGS}, {"test_long_as_double", (PyCFunction)test_long_as_double,METH_NOARGS},