From 4aaa382fb112b4ffad91101107c8716690d6b1b6 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Sun, 29 Dec 2019 19:14:14 +0900 Subject: [PATCH 1/4] bpo-38588: Fix segfaults when dict comparision with modifying operand --- Lib/test/test_dict.py | 10 ++++++++++ .../2019-12-29-19-13-54.bpo-38588.pgXnNS.rst | 1 + Objects/dictobject.c | 2 ++ 3 files changed, 13 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 5b513765f7b08a..aa595f1a2dde9a 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -1237,6 +1237,16 @@ def __hash__(self): dict_b = {X(): X()} self.assertTrue(dict_a == dict_b) + # test fix for seg fault reported in issue 38588 part 1. + class Y: + def __eq__(self, other): + dict_d.clear() + return True + + dict_c = {0: Y()} + dict_d = {0: set()} + self.assertTrue(dict_c == dict_d) + def test_fromkeys_operator_modifying_dict_operand(self): # test fix for seg fault reported in issue 27945 part 4a. class X(int): diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst new file mode 100644 index 00000000000000..c82b896dc45920 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst @@ -0,0 +1 @@ +Fix the segfault when dict comparision with modifying operand. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 4afa19c8a0a90b..87f88abbe53bd9 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2777,9 +2777,11 @@ dict_equal(PyDictObject *a, PyDictObject *b) return -1; return 0; } + Py_INCREF(bval); cmp = PyObject_RichCompareBool(aval, bval, Py_EQ); Py_DECREF(key); Py_DECREF(aval); + Py_DECREF(bval); if (cmp <= 0) /* error or not equal */ return cmp; } From 035a9d7f23d63dc529c5bf8bb322f970a86aac2a Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Dec 2019 13:31:57 +0900 Subject: [PATCH 2/4] bpo-38588: Fix list case --- Lib/test/test_list.py | 24 +++++++++++++++++++ .../2019-12-29-19-13-54.bpo-38588.pgXnNS.rst | 3 ++- Objects/listobject.c | 7 ++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index fe4b2cd365f088..cb3a0af8789751 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -163,6 +163,30 @@ class L(list): pass with self.assertRaises(TypeError): (3,) + L([1,2]) + def test_equal_operator_modifying_operand(self): + class X: + def __eq__(self,other) : + list2.clear() + return NotImplemented + + class Y: + def __eq__(self, other): + list1.clear() + return NotImplemented + + class Z: + def __eq__(self, other): + list3.clear() + return NotImplemented + + list1 = [X()] + list2 = [Y()] + self.assertTrue(list1 == list2) + + list3 = [Z()] + list4 = [1] + self.assertFalse(list3 == list4) + @cpython_only def test_preallocation(self): iterable = [0] * 10 diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst index c82b896dc45920..0b81085a89d254 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2019-12-29-19-13-54.bpo-38588.pgXnNS.rst @@ -1 +1,2 @@ -Fix the segfault when dict comparision with modifying operand. +Fix possible crashes in dict and list when calling +:c:func:`PyObject_RichCompareBool`. diff --git a/Objects/listobject.c b/Objects/listobject.c index 645742b801fac4..95a37bb7dd475c 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -2653,8 +2653,15 @@ list_richcompare(PyObject *v, PyObject *w, int op) /* Search for the first index where items are different */ for (i = 0; i < Py_SIZE(vl) && i < Py_SIZE(wl); i++) { + PyObject *vitem = vl->ob_item[i]; + PyObject *witem = wl->ob_item[i]; + + Py_INCREF(vitem); + Py_INCREF(witem); int k = PyObject_RichCompareBool(vl->ob_item[i], wl->ob_item[i], Py_EQ); + Py_DECREF(vitem); + Py_DECREF(witem); if (k < 0) return NULL; if (!k) From e809f7a17d12809db294ab6ec8ead9edb06d7862 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Dec 2019 13:54:42 +0900 Subject: [PATCH 3/4] bpo-38588: Add comment --- Lib/test/test_list.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index cb3a0af8789751..64e501844d396f 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -164,6 +164,7 @@ class L(list): pass (3,) + L([1,2]) def test_equal_operator_modifying_operand(self): + # test fix for seg fault reported in issue 38588 part 2. class X: def __eq__(self,other) : list2.clear() From 645e1fba7979555cb34a772f0653dd8fa4da36bf Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 31 Dec 2019 09:29:06 +0900 Subject: [PATCH 4/4] bpo-38588: Update comment --- Lib/test/test_dict.py | 4 ++-- Lib/test/test_list.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index aa595f1a2dde9a..de483ab552155a 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -1221,7 +1221,7 @@ def test_free_after_iterating(self): support.check_free_after_iterating(self, lambda d: iter(d.items()), dict) def test_equal_operator_modifying_operand(self): - # test fix for seg fault reported in issue 27945 part 3. + # test fix for seg fault reported in bpo-27945 part 3. class X(): def __del__(self): dict_b.clear() @@ -1237,7 +1237,7 @@ def __hash__(self): dict_b = {X(): X()} self.assertTrue(dict_a == dict_b) - # test fix for seg fault reported in issue 38588 part 1. + # test fix for seg fault reported in bpo-38588 part 1. class Y: def __eq__(self, other): dict_d.clear() diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 64e501844d396f..6254f3c6c35a8a 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -164,7 +164,7 @@ class L(list): pass (3,) + L([1,2]) def test_equal_operator_modifying_operand(self): - # test fix for seg fault reported in issue 38588 part 2. + # test fix for seg fault reported in bpo-38588 part 2. class X: def __eq__(self,other) : list2.clear()