diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1108,11 +1108,10 @@ @no_tracing def test_bad_getattr(self): + # Issue #3514: crash when there is an infinite loop in __getattr__ x = BadGetattr() - for proto in 0, 1: + for proto in protocols: self.assertRaises(RuntimeError, self.dumps, x, proto) - # protocol 2 don't raise a RuntimeError. - d = self.dumps(x, 2) def test_reduce_bad_iterator(self): # Issue4176: crash when 4th and 5th items of __reduce__() diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -3197,15 +3197,15 @@ class C4(C4classic, object): # mixed inheritance pass - for bin in 0, 1: + for proto in range(pickle.HIGHEST_PROTOCOL + 1): for cls in C, C1, C2: - s = pickle.dumps(cls, bin) + s = pickle.dumps(cls, proto) cls2 = pickle.loads(s) self.assertTrue(cls2 is cls) a = C1(1, 2); a.append(42); a.append(24) b = C2("hello", "world", 42) - s = pickle.dumps((a, b), bin) + s = pickle.dumps((a, b), proto) x, y = pickle.loads(s) self.assertEqual(x.__class__, a.__class__) self.assertEqual(sorteditems(x.__dict__), sorteditems(a.__dict__)) @@ -3215,14 +3215,14 @@ self.assertEqual(repr(y), repr(b)) # Test for __getstate__ and __setstate__ on new style class u = C3(42) - s = pickle.dumps(u, bin) + s = pickle.dumps(u, proto) v = pickle.loads(s) self.assertEqual(u.__class__, v.__class__) self.assertEqual(u.foo, v.foo) # Test for picklability of hybrid class u = C4() u.foo = 42 - s = pickle.dumps(u, bin) + s = pickle.dumps(u, proto) v = pickle.loads(s) self.assertEqual(u.__class__, v.__class__) self.assertEqual(u.foo, v.foo) @@ -3289,26 +3289,27 @@ class D(C): pass # Now it should work - x = C() - y = pickle.loads(pickle.dumps(x)) - self.assertEqual(hasattr(y, 'a'), 0) - x.a = 42 - y = pickle.loads(pickle.dumps(x)) - self.assertEqual(y.a, 42) - x = D() - x.a = 42 - x.b = 100 - y = pickle.loads(pickle.dumps(x)) - self.assertEqual(y.a + y.b, 142) - # A subclass that adds a slot should also work - class E(C): - __slots__ = ['b'] - x = E() - x.a = 42 - x.b = "foo" - y = pickle.loads(pickle.dumps(x)) - self.assertEqual(y.a, x.a) - self.assertEqual(y.b, x.b) + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + x = C() + y = pickle.loads(pickle.dumps(x, proto)) + self.assertEqual(hasattr(y, 'a'), 0) + x.a = 42 + y = pickle.loads(pickle.dumps(x, proto)) + self.assertEqual(y.a, 42) + x = D() + x.a = 42 + x.b = 100 + y = pickle.loads(pickle.dumps(x, proto)) + self.assertEqual(y.a + y.b, 142) + # A subclass that adds a slot should also work + class E(C): + __slots__ = ['b'] + x = E() + x.a = 42 + x.b = "foo" + y = pickle.loads(pickle.dumps(x, proto)) + self.assertEqual(y.a, x.a) + self.assertEqual(y.b, x.b) def test_binary_operator_override(self): # Testing overrides of binary operations... diff --git a/Objects/typeobject.c b/Objects/typeobject.c --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3354,56 +3354,184 @@ return mod_copyreg; } -static PyObject * -slotnames(PyObject *cls) -{ - PyObject *clsdict; +Py_LOCAL(PyObject *) +_PyType_GetSlotNames(PyTypeObject *cls) +{ PyObject *copyreg; PyObject *slotnames; _Py_IDENTIFIER(__slotnames__); _Py_IDENTIFIER(_slotnames); - clsdict = ((PyTypeObject *)cls)->tp_dict; - slotnames = _PyDict_GetItemId(clsdict, &PyId___slotnames__); - if (slotnames != NULL && PyList_Check(slotnames)) { + assert(PyType_Check(cls)); + + /* Get the slot names from the cache in the class if possible. */ + slotnames = _PyDict_GetItemIdWithError(cls->tp_dict, &PyId___slotnames__); + if (slotnames != NULL) { + if (slotnames != Py_None && !PyList_Check(slotnames)) { + PyErr_Format(PyExc_TypeError, + "%.200s.__slotnames__ should be a list or None, " + "not %.200s", + cls->tp_name, Py_TYPE(slotnames)->tp_name); + return NULL; + } Py_INCREF(slotnames); return slotnames; } + else { + if (PyErr_Occurred()) { + return NULL; + } + /* The class does not have the slot names cached yet. */ + } copyreg = import_copyreg(); if (copyreg == NULL) return NULL; - slotnames = _PyObject_CallMethodId(copyreg, &PyId__slotnames, "O", cls); + /* Use _slotnames function from the copyreg module to find the slots + by this class and its bases. This function will cache the result + in __slotnames__. */ + slotnames = _PyObject_CallMethodObjIdArgs(copyreg, &PyId__slotnames, + cls, NULL); Py_DECREF(copyreg); - if (slotnames != NULL && - slotnames != Py_None && - !PyList_Check(slotnames)) - { + if (slotnames == NULL) + return NULL; + + if (slotnames != Py_None && !PyList_Check(slotnames)) { PyErr_SetString(PyExc_TypeError, - "copyreg._slotnames didn't return a list or None"); + "copyreg._slotnames didn't return a list or None"); Py_DECREF(slotnames); - slotnames = NULL; + return NULL; } return slotnames; } +Py_LOCAL(PyObject *) +_PyObject_GetState(PyObject *obj) +{ + PyObject *state; + PyObject *getstate; + _Py_IDENTIFIER(__getstate__); + + getstate = _PyObject_GetAttrId(obj, &PyId___getstate__); + if (getstate == NULL) { + PyObject *slotnames; + + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return NULL; + } + PyErr_Clear(); + + { + PyObject **dict; + dict = _PyObject_GetDictPtr(obj); + if (dict != NULL && *dict != NULL) { + state = *dict; + } + else { + state = Py_None; + } + Py_INCREF(state); + } + + slotnames = _PyType_GetSlotNames(Py_TYPE(obj)); + if (slotnames == NULL) { + Py_DECREF(state); + return NULL; + } + + assert(slotnames == Py_None || PyList_Check(slotnames)); + if (slotnames != Py_None && Py_SIZE(slotnames) > 0) { + PyObject *slots; + Py_ssize_t slotnames_size, i; + + slots = PyDict_New(); + if (slots == NULL) { + Py_DECREF(slotnames); + Py_DECREF(state); + return NULL; + } + + slotnames_size = Py_SIZE(slotnames); + for (i = 0; i < slotnames_size; i++) { + PyObject *name, *value; + + name = PyList_GET_ITEM(slotnames, i); + value = PyObject_GetAttr(obj, name); + if (value == NULL) { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + goto error; + } + /* It is not an error if the attribute is not present. */ + PyErr_Clear(); + } + else { + int err = PyDict_SetItem(slots, name, value); + Py_DECREF(value); + if (err) { + goto error; + } + } + + /* The list is stored on the class so it may mutates while we + iterate over it */ + if (slotnames_size != Py_SIZE(slotnames)) { + PyErr_Format(PyExc_RuntimeError, + "__slotsname__ changed size during iteration"); + goto error; + } + + /* We handle errors within the loop here. */ + if (0) { + error: + Py_DECREF(slotnames); + Py_DECREF(slots); + Py_DECREF(state); + return NULL; + } + } + + /* If we found some slot attributes, pack them in a tuple along + the orginal attribute dictionary. */ + if (PyDict_Size(slots) > 0) { + PyObject *state2; + + state2 = PyTuple_Pack(2, state, slots); + Py_DECREF(state); + if (state2 == NULL) { + Py_DECREF(slotnames); + Py_DECREF(slots); + return NULL; + } + state = state2; + } + Py_DECREF(slots); + } + Py_DECREF(slotnames); + } + else { /* getstate != NULL */ + state = PyObject_CallObject(getstate, NULL); + Py_DECREF(getstate); + if (state == NULL) + return NULL; + } + + return state; +} + static PyObject * reduce_2(PyObject *obj) { PyObject *cls, *getnewargs; PyObject *args = NULL, *args2 = NULL; - PyObject *getstate = NULL, *state = NULL, *names = NULL; + PyObject *state = NULL, *names = NULL; PyObject *slots = NULL, *listitems = NULL, *dictitems = NULL; PyObject *copyreg = NULL, *newobj = NULL, *res = NULL; Py_ssize_t i, n; _Py_IDENTIFIER(__getnewargs__); - _Py_IDENTIFIER(__getstate__); _Py_IDENTIFIER(__newobj__); - cls = (PyObject *) Py_TYPE(obj); - getnewargs = _PyObject_GetAttrId(obj, &PyId___getnewargs__); if (getnewargs != NULL) { args = PyObject_CallObject(getnewargs, NULL); @@ -3422,56 +3550,9 @@ if (args == NULL) goto end; - getstate = _PyObject_GetAttrId(obj, &PyId___getstate__); - if (getstate != NULL) { - state = PyObject_CallObject(getstate, NULL); - Py_DECREF(getstate); - if (state == NULL) - goto end; - } - else { - PyObject **dict; - PyErr_Clear(); - dict = _PyObject_GetDictPtr(obj); - if (dict && *dict) - state = *dict; - else - state = Py_None; - Py_INCREF(state); - names = slotnames(cls); - if (names == NULL) - goto end; - if (names != Py_None && PyList_GET_SIZE(names) > 0) { - assert(PyList_Check(names)); - slots = PyDict_New(); - if (slots == NULL) - goto end; - n = 0; - /* Can't pre-compute the list size; the list - is stored on the class so accessible to other - threads, which may be run by DECREF */ - for (i = 0; i < PyList_GET_SIZE(names); i++) { - PyObject *name, *value; - name = PyList_GET_ITEM(names, i); - value = PyObject_GetAttr(obj, name); - if (value == NULL) - PyErr_Clear(); - else { - int err = PyDict_SetItem(slots, name, - value); - Py_DECREF(value); - if (err) - goto end; - n++; - } - } - if (n) { - state = Py_BuildValue("(NO)", state, slots); - if (state == NULL) - goto end; - } - } - } + state = _PyObject_GetState(obj); + if (state == NULL) + goto end; if (!PyList_Check(obj)) { listitems = Py_None; @@ -3509,6 +3590,7 @@ args2 = PyTuple_New(n+1); if (args2 == NULL) goto end; + cls = (PyObject *) Py_TYPE(obj); Py_INCREF(cls); PyTuple_SET_ITEM(args2, 0, cls); for (i = 0; i < n; i++) {